feat(nuxi): defer subcommands to the project's @nuxt/cli - #1394
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
📝 WalkthroughWalkthroughThe CLI entrypoint now loads Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
commit: |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/nuxi/src/launcher.ts`:
- Around line 65-80: Update the package-selection logic around findPackage and
MIN_PROJECT_CLI so a required minimum version cannot be bypassed when pkg or
pkg.version is unavailable. Fail closed by skipping the candidate whenever
minimum is configured but the package version cannot be determined; retain the
existing isAtLeast check for known versions and allow candidates without a
configured minimum as before.
In `@packages/nuxt-cli/src/run-command.ts`:
- Around line 12-15: Update the argument handling in the run-command function so
adding --no-clear does not mutate the caller-owned argv array. Create a new
array containing the existing arguments and the flag, and use that array for
subsequent command execution while preserving current behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 819a0d99-e064-4e0f-a361-1a91bda6b043
📒 Files selected for processing (17)
packages/nuxi/bin/nuxi.mjspackages/nuxi/src/cli.tspackages/nuxi/src/index.tspackages/nuxi/src/launcher.tspackages/nuxi/src/main.tspackages/nuxi/src/run.tspackages/nuxi/test/launcher.spec.tspackages/nuxi/tsdown.config.tspackages/nuxt-cli/src/commands/init.tspackages/nuxt-cli/src/commands/module/add.tspackages/nuxt-cli/src/commands/module/remove.tspackages/nuxt-cli/src/run-command.tspackages/nuxt-cli/src/run.tspackages/nuxt-cli/test/unit/commands/add.spec.tspackages/nuxt-cli/test/unit/commands/module/add.spec.tspackages/nuxt-cli/test/unit/commands/module/remove.spec.tspackages/nuxt-cli/test/unit/commands/network-failures.spec.ts
💤 Files with no reviewable changes (1)
- packages/nuxt-cli/src/run.ts
| const pkg = findPackage(entry, name) | ||
| const minimum = MIN_PROJECT_CLI[name] | ||
| if (pkg?.version && minimum && !isAtLeast(pkg.version, minimum)) { | ||
| continue | ||
| } | ||
| const devEntry = pkg && join(pkg.root, 'dist/dev/index.mjs') | ||
| return { | ||
| name, | ||
| version: pkg?.version, | ||
| entry, | ||
| devEntry: devEntry && existsSync(devEntry) ? devEntry : undefined, | ||
| } | ||
| } | ||
| } | ||
| return null | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Version floor is silently skipped when the package can't be located.
isAtLeast is only checked when pkg?.version is truthy (pkg?.version && minimum && !isAtLeast(...)). If findPackage fails to find a matching package.json within 5 parent directories (e.g. unusual nesting, Yarn PnP virtual FS, or a symlink structure that doesn't expose a plain directory tree), pkg is null, minimum is bypassed, and the code proceeds to hand off to a CLI whose version — and thus compatibility — was never verified. The blast radius is limited by loadDelegate's runMain-shape check, but that's a much weaker guarantee than the explicit MIN_PROJECT_CLI floor this code is trying to enforce.
Proposed fix: fail closed instead of open when the version can't be determined
const pkg = findPackage(entry, name)
const minimum = MIN_PROJECT_CLI[name]
- if (pkg?.version && minimum && !isAtLeast(pkg.version, minimum)) {
+ if (minimum && !(pkg?.version && isAtLeast(pkg.version, minimum))) {
continue
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const pkg = findPackage(entry, name) | |
| const minimum = MIN_PROJECT_CLI[name] | |
| if (pkg?.version && minimum && !isAtLeast(pkg.version, minimum)) { | |
| continue | |
| } | |
| const devEntry = pkg && join(pkg.root, 'dist/dev/index.mjs') | |
| return { | |
| name, | |
| version: pkg?.version, | |
| entry, | |
| devEntry: devEntry && existsSync(devEntry) ? devEntry : undefined, | |
| } | |
| } | |
| } | |
| return null | |
| } | |
| const pkg = findPackage(entry, name) | |
| const minimum = MIN_PROJECT_CLI[name] | |
| if (minimum && !(pkg?.version && isAtLeast(pkg.version, minimum))) { | |
| continue | |
| } | |
| const devEntry = pkg && join(pkg.root, 'dist/dev/index.mjs') | |
| return { | |
| name, | |
| version: pkg?.version, | |
| entry, | |
| devEntry: devEntry && existsSync(devEntry) ? devEntry : undefined, | |
| } | |
| } | |
| } | |
| return null | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/nuxi/src/launcher.ts` around lines 65 - 80, Update the
package-selection logic around findPackage and MIN_PROJECT_CLI so a required
minimum version cannot be bypassed when pkg or pkg.version is unavailable. Fail
closed by skipping the candidate whenever minimum is configured but the package
version cannot be determined; retain the existing isAtLeast check for known
versions and allow candidates without a configured minimum as before.
| argv: string[] = process.argv.slice(2), | ||
| data: { overrides?: Record<string, any> } = {}, | ||
| ): Promise<{ result: unknown }> { | ||
| argv.push('--no-clear') // Dev |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Avoid mutating caller-owned argv.
argv.push('--no-clear') modifies the array supplied by the caller. Repeated calls can accumulate flags, and callers may observe their arguments changed after execution. Create a new array instead.
Suggested fix
- argv.push('--no-clear') // Dev
+ const rawArgs = [...argv, '--no-clear'] // Dev
...
- rawArgs: argv,
+ rawArgs,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| argv: string[] = process.argv.slice(2), | |
| data: { overrides?: Record<string, any> } = {}, | |
| ): Promise<{ result: unknown }> { | |
| argv.push('--no-clear') // Dev | |
| const rawArgs = [...argv, '--no-clear'] // Dev |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/nuxt-cli/src/run-command.ts` around lines 12 - 15, Update the
argument handling in the run-command function so adding --no-clear does not
mutate the caller-owned argv array. Create a new array containing the existing
arguments and the flag, and use that array for subsequent command execution
while preserving current behavior.
1 participant
🔗 Linked issue
#648
📚 Description
this defers to
@nuxt/cliif installed when runningnuxiglobally.this also ships two unrelated bundle size improvements (skipping inlining all of
@nuxt/schema's types in nuxi 😱 and pulling in unrelated commands intocreate-nuxt) 🔥