Conversation
📝 WalkthroughWalkthroughThis PR removes several external dependencies (destr, dotenv, git-url-parse, is-docker, package-manager-detector, pathe) and replaces them with internal implementations and Node.js built-ins. It adds a local Docker detection utility, custom environment parsing helpers (isTruthy, parseDotenv), a lightweight Git URL parser, and package manager detection via lockfile/package.json. Path resolution uses node:path. The Node engine requirement is lowered to >=18.12.0, citty is bumped to ^0.2.0, and the Context type nuxtMajorVersion is widened from 2 | 3 to number. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 75-77: The package.json engines entry currently pins "node" to
">=18.12.0" which targets an EOL Node release; update the engines field so it
requires a supported LTS (for example change the "node" value to ">=22.0.0") to
ensure maintained security updates and compatibility; modify the "engines" →
"node" value in package.json accordingly and run CI/test to verify no runtime
breakages.
In `@src/context.ts`:
- Line 21: The assignment to nuxtMajorVersion may yield undefined if
nuxt.options._majorVersion is missing; update the nuxtMajorVersion
initialization (the expression using isNuxtMajorVersion and
nuxt.options._majorVersion) to coerce/guard that value and provide a numeric
fallback (e.g., default to 2 or another safe number) so it always satisfies the
Context interface's required number type; reference isNuxtMajorVersion,
nuxtMajorVersion and nuxt.options._majorVersion and ensure the result is
explicitly a number.
🧹 Nitpick comments (3)
src/cli.ts (1)
20-36:parseDotenvdoesn't handle theexportprefix.Some
.envfiles useexport KEY=valuesyntax (common when.envfiles are also sourced by shell scripts). The current parser would produce a key ofexport KEYinstead ofKEY. This is a minor gap since the only consumer checks a specific key name.Proposed fix
const key = trimmed.slice(0, eqIndex).trim() + // strip optional `export` prefix + const cleanKey = key.startsWith('export ') ? key.slice(7).trim() : key let value = trimmed.slice(eqIndex + 1).trim() // Remove surrounding quotes if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith('\'') && value.endsWith('\''))) { value = value.slice(1, -1) } - result[key] = value + result[cleanKey] = valuesrc/context.ts (2)
105-136: Usepath.joinfor constructing file paths.String template concatenation (
${rootDir}/${file}) works in practice because Node'sfsAPIs normalize separators on Windows, butpath.joinis the idiomatic and safer approach. Since the PR already removedpathe, consider importingjoinfromnode:path.Proposed fix
Add to imports:
import { join } from 'node:path'Then in
detectPackageManagerand when readingpackage.json:- if (existsSync(`${rootDir}/${file}`)) { + if (existsSync(join(rootDir, file))) {- const pkgJson = JSON.parse(readFileSync(`${rootDir}/package.json`, 'utf8')) + const pkgJson = JSON.parse(readFileSync(join(rootDir, 'package.json'), 'utf8'))
38-38: Redundant fallback:detectPackageManageralready returns'unknown'.
detectPackageManagerreturns'unknown'at Line 135 as its final fallback, so the|| 'unknown'here is unreachable. Harmless, but slightly misleading.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/cli.ts`:
- Around line 16-18: The isTruthy function currently only matches exact
case-sensitive values and misses variants like "TRUE" or "True"; update isTruthy
to normalize string inputs (e.g., convert to lowercase) before comparison and
handle numeric values consistently: for non-boolean inputs coerce to string,
.toLowerCase() it, and compare against 'true' and '1', while still accepting
boolean true and numeric 1; modify the isTruthy function so it first checks
typeof val and uses a lowercase string comparison for case-insensitive matches.
- Around line 20-36: The parseDotenv function fails to handle lines prefixed
with "export" and inline comments; update parseDotenv so for each non-empty,
non-# line it first removes a leading "export " token from the start of the
trimmed line before splitting on '=', then when extracting the value strip an
inline comment only if the value is not enclosed in matching single or double
quotes (i.e., remove any unquoted " #..." suffix after trimming), then remove
surrounding quotes and unescape any escaped quotes; ensure the key is the
cleaned left-hand side and the value is the cleaned right-hand side so isTruthy
checks work as expected.
| function isTruthy(val: unknown): boolean { | ||
| return val === true || val === 'true' || val === '1' || val === 1 | ||
| } |
There was a problem hiding this comment.
isTruthy doesn't handle case-insensitive variants like 'TRUE' or 'True'.
Environment variables are commonly set with varying cases. A user writing NUXT_TELEMETRY_DISABLED=TRUE would bypass this check. Consider normalizing:
Suggested fix
function isTruthy(val: unknown): boolean {
- return val === true || val === 'true' || val === '1' || val === 1
+ return val === true || val === 1 || (typeof val === 'string' && (val.toLowerCase() === 'true' || val === '1'))
}🤖 Prompt for AI Agents
In `@src/cli.ts` around lines 16 - 18, The isTruthy function currently only
matches exact case-sensitive values and misses variants like "TRUE" or "True";
update isTruthy to normalize string inputs (e.g., convert to lowercase) before
comparison and handle numeric values consistently: for non-boolean inputs coerce
to string, .toLowerCase() it, and compare against 'true' and '1', while still
accepting boolean true and numeric 1; modify the isTruthy function so it first
checks typeof val and uses a lowercase string comparison for case-insensitive
matches.
| function parseDotenv(src: string): Record<string, string> { | ||
| const result: Record<string, string> = {} | ||
| for (const line of src.split('\n')) { | ||
| const trimmed = line.trim() | ||
| if (!trimmed || trimmed.startsWith('#')) continue | ||
| const eqIndex = trimmed.indexOf('=') | ||
| if (eqIndex === -1) continue | ||
| const key = trimmed.slice(0, eqIndex).trim() | ||
| let value = trimmed.slice(eqIndex + 1).trim() | ||
| // Remove surrounding quotes | ||
| if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith('\'') && value.endsWith('\''))) { | ||
| value = value.slice(1, -1) | ||
| } | ||
| result[key] = value | ||
| } | ||
| return result | ||
| } |
There was a problem hiding this comment.
parseDotenv doesn't handle export prefix or inline comments.
Two common .env patterns supported by the dotenv library this replaces are missing:
exportprefix:export NUXT_TELEMETRY_DISABLED=truewould produce the keyexport NUXT_TELEMETRY_DISABLED, never matching the expected key.- Inline comments:
NUXT_TELEMETRY_DISABLED=true # enable thiswould store the value astrue # enable this, which would fail theisTruthycheck.
Since this replaces a well-tested library, it's worth covering these edge cases to avoid subtle regressions.
Suggested fix
function parseDotenv(src: string): Record<string, string> {
const result: Record<string, string> = {}
for (const line of src.split('\n')) {
- const trimmed = line.trim()
+ let trimmed = line.trim()
if (!trimmed || trimmed.startsWith('#')) continue
+ // Strip optional `export` prefix
+ if (trimmed.startsWith('export ')) {
+ trimmed = trimmed.slice(7).trim()
+ }
const eqIndex = trimmed.indexOf('=')
if (eqIndex === -1) continue
const key = trimmed.slice(0, eqIndex).trim()
let value = trimmed.slice(eqIndex + 1).trim()
// Remove surrounding quotes
if ((value.startsWith('"') && value.endsWith('"')) || (value.startsWith('\'') && value.endsWith('\''))) {
value = value.slice(1, -1)
}
+ else {
+ // Strip inline comments for unquoted values
+ const hashIndex = value.indexOf(' #')
+ if (hashIndex !== -1) {
+ value = value.slice(0, hashIndex).trim()
+ }
+ }
result[key] = value
}
return result
}🤖 Prompt for AI Agents
In `@src/cli.ts` around lines 20 - 36, The parseDotenv function fails to handle
lines prefixed with "export" and inline comments; update parseDotenv so for each
non-empty, non-# line it first removes a leading "export " token from the start
of the trimmed line before splitting on '=', then when extracting the value
strip an inline comment only if the value is not enclosed in matching single or
double quotes (i.e., remove any unquoted " #..." suffix after trimming), then
remove surrounding quotes and unescape any escaped quotes; ensure the key is the
cleaned left-hand side and the value is the cleaned right-hand side so isTruthy
checks work as expected.
1 participant
🔗 Linked issue
closes #340
closes #276
❓ Type of change
📚 Description
this significantly cleans up our dependency tree 🔥
we can do more when we release a new version without support for nuxt/bridge