feat(nitro,nuxt): add suggestions + trace to import protection#34454
feat(nitro,nuxt): add suggestions + trace to import protection#34454
Conversation
|
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdded 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/nuxt/src/core/plugins/import-protection.ts`:
- Around line 67-70: The matcher for server imports (the entry using
serverRelative and the anonymous function in patterns) incorrectly matches any
path segments named "server/..." (e.g., node_modules/foo/server/api) because id
is tested raw; fix by normalising the import path against the project root
before testing: compute a path for the id relative to nuxt.options.rootDir
(normalising separators and resolving symlinks if needed) and run the RegExp
test against that relative path instead of the raw id, keeping the existing
serverRelative pattern building (escapeRE(relative(... resolve(...)))); update
the anonymous matcher to use this normalised relative id and add a regression
test covering a root-level server/ layout (and a node_modules package containing
/server/api) to ensure only project-root server/* matches are protected.
- Around line 30-34: The suggestion for `useAppConfig()` is currently
unconditional in the patterns.push([...]) entry for `nuxt.config` and can point
to an unavailable API; change import-protection.ts to build the hints array
dynamically instead of hardcoding the third message: always include the
`useRuntimeConfig()` hint, and only append the `useAppConfig()` hint when in the
Nuxt app context or when the experimental flag is enabled (check the
runtime/context flag equivalent to experimental.serverAppConfig or an isNuxtApp
indicator available to this plugin), otherwise omit that hint or replace it with
a neutral message about using a module for app config; specifically modify the
patterns.push([...]) call to compute the third element as a conditional array
based on that flag/context so `useAppConfig()` is only suggested when it
actually exists.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 77837335-f48e-469e-93c3-7eec882e3577
⛔ Files ignored due to path filters (3)
packages/nitro-server/package.jsonis excluded by!**/package.jsonpackages/nuxt/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (4)
packages/nitro-server/src/index.tspackages/nuxt/src/core/nuxt.tspackages/nuxt/src/core/plugins/import-protection.tspackages/nuxt/test/import-protection.test.ts
| patterns.push([ | ||
| /^((~|~~|@|@@)?\/)?nuxt\.config(\.|$)/, | ||
| 'Importing directly from a `nuxt.config` file is not allowed. Instead, use runtime config or a module.', | ||
| 'Importing directly from a `nuxt.config` file is not allowed.', | ||
| ['Use `useRuntimeConfig()` to access runtime config in your app.', 'Use `useAppConfig()` to access config that doesn\'t need to be changed at runtime.', 'Use a Nuxt module to access build-time configuration.'], | ||
| ]) |
There was a problem hiding this comment.
Make the nuxt.config guidance context-aware.
Line 33 now suggests useAppConfig() unconditionally. That helper is only auto-imported on the Nitro side when experimental.serverAppConfig === true in packages/nitro-server/src/index.ts (Lines 354-361), and it is removed again when the flag is off (Lines 668-675). nitro-app and shared users can therefore be pointed at an API that is unavailable in their current context. Please gate that hint by context/flag, or drop it outside nuxt-app.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nuxt/src/core/plugins/import-protection.ts` around lines 30 - 34,
The suggestion for `useAppConfig()` is currently unconditional in the
patterns.push([...]) entry for `nuxt.config` and can point to an unavailable
API; change import-protection.ts to build the hints array dynamically instead of
hardcoding the third message: always include the `useRuntimeConfig()` hint, and
only append the `useAppConfig()` hint when in the Nuxt app context or when the
experimental flag is enabled (check the runtime/context flag equivalent to
experimental.serverAppConfig or an isNuxtApp indicator available to this
plugin), otherwise omit that hint or replace it with a neutral message about
using a module for app config; specifically modify the patterns.push([...]) call
to compute the third element as a conditional array based on that flag/context
so `useAppConfig()` is only suggested when it actually exists.
| const serverRelative = escapeRE(relative(nuxt.options.rootDir, resolve(nuxt.options.srcDir, nuxt.options.serverDir || 'server'))) | ||
| patterns.push([ | ||
| new RegExp(escapeRE(relative(nuxt.options.srcDir, resolve(nuxt.options.srcDir, nuxt.options.serverDir || 'server'))) + '\\/(api|routes|middleware|plugins)\\/'), | ||
| (id: string, importer: string) => new RegExp(serverRelative + '\\/(api|routes|middleware|plugins)\\/').test(id) | ||
| && `Importing from server is not allowed in ${context} [from \`${importer}\`].`, |
There was a problem hiding this comment.
Anchor the server-import matcher to the project root.
With a root-level server/ directory, Line 67 computes serverRelative as just server, so Line 69 will also match paths like node_modules/foo/server/api/... or any package subpath containing /server/api/. That turns legitimate third-party imports into protected ones. Please normalise id against rootDir before testing, and add a regression case for the root-level server/ layout.
Based on learnings, in Nuxt v4, the app/ and server/ directories are siblings at the root level, while in Nuxt v3, server/ is inside srcDir. This structural difference affects watch path logic in the builder.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nuxt/src/core/plugins/import-protection.ts` around lines 67 - 70,
The matcher for server imports (the entry using serverRelative and the anonymous
function in patterns) incorrectly matches any path segments named "server/..."
(e.g., node_modules/foo/server/api) because id is tested raw; fix by normalising
the import path against the project root before testing: compute a path for the
id relative to nuxt.options.rootDir (normalising separators and resolving
symlinks if needed) and run the RegExp test against that relative path instead
of the raw id, keeping the existing serverRelative pattern building
(escapeRE(relative(... resolve(...)))); update the anonymous matcher to use this
normalised relative id and add a regression test covering a root-level server/
layout (and a node_modules package containing /server/api) to ensure only
project-root server/* matches are protected.
Merging this PR will not alter performance
Comparing 
Footnotes
|
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/nitro-server/src/index.ts (1)
576-589:⚠️ Potential issue | 🟠 MajorSkip Nitro import protection during test runs as well.
This block still registers both
ImpoundPlugin.rollup(...)instances whennuxt.options.testis true.packages/nuxt/src/core/nuxt.tsnow skips the app-side protection in that mode, so shared/Nitro test bundles can still fail on protected imports unless this registration is guarded too.💡 Proposed fix
- nitroConfig.rollupConfig!.plugins!.push( - ImpoundPlugin.rollup({ - cwd: nuxt.options.rootDir, - trace: true, - include: sharedPatterns, - patterns: createImportProtectionPatterns(nuxt, { context: 'shared' }), - }), - ImpoundPlugin.rollup({ - cwd: nuxt.options.rootDir, - trace: true, - patterns: createImportProtectionPatterns(nuxt, { context: 'nitro-app' }), - exclude: [/node_modules[\\/]nitro(?:pack)?(?:-nightly)?[\\/]|(packages|@nuxt)[\\/]nitro-server(?:-nightly)?[\\/](src|dist)[\\/]runtime[\\/]/, ...sharedPatterns], - }), - ) + if (!nuxt.options.test) { + nitroConfig.rollupConfig!.plugins!.push( + ImpoundPlugin.rollup({ + cwd: nuxt.options.rootDir, + trace: true, + include: sharedPatterns, + patterns: createImportProtectionPatterns(nuxt, { context: 'shared' }), + }), + ImpoundPlugin.rollup({ + cwd: nuxt.options.rootDir, + trace: true, + patterns: createImportProtectionPatterns(nuxt, { context: 'nitro-app' }), + exclude: [/node_modules[\\/]nitro(?:pack)?(?:-nightly)?[\\/]|(packages|@nuxt)[\\/]nitro-server(?:-nightly)?[\\/](src|dist)[\\/]runtime[\\/]/, ...sharedPatterns], + }), + ) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nitro-server/src/index.ts` around lines 576 - 589, The ImpoundPlugin.rollup registrations are always pushed even during test runs, causing import protection to run for shared/Nitro test bundles; wrap the call to nitroConfig.rollupConfig!.plugins!.push(...) in a guard that checks nuxt.options.test and skip registering both ImpoundPlugin.rollup instances when nuxt.options.test is true (i.e., only push the plugins when !nuxt.options.test) so the import protection is not applied during test mode; reference the existing nitroConfig.rollupConfig!.plugins!.push invocation and the ImpoundPlugin.rollup(...) entries to locate where to add the guard.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/nitro-server/src/index.ts`:
- Around line 576-589: The ImpoundPlugin.rollup registrations are always pushed
even during test runs, causing import protection to run for shared/Nitro test
bundles; wrap the call to nitroConfig.rollupConfig!.plugins!.push(...) in a
guard that checks nuxt.options.test and skip registering both
ImpoundPlugin.rollup instances when nuxt.options.test is true (i.e., only push
the plugins when !nuxt.options.test) so the import protection is not applied
during test mode; reference the existing nitroConfig.rollupConfig!.plugins!.push
invocation and the ImpoundPlugin.rollup(...) entries to locate where to add the
guard.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: de269376-ce20-498d-adba-7380cf27b403
⛔ Files ignored due to path filters (3)
packages/nitro-server/package.jsonis excluded by!**/package.jsonpackages/nuxt/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (2)
packages/nitro-server/src/index.tspackages/nuxt/src/core/nuxt.ts
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/nuxt/src/core/nuxt.ts`:
- Around line 428-442: The code currently treats SSR-only (envOptions.client ===
false) as non-error by setting error = false and adding warn: 'once', causing
Vite to be less strict than webpack/rspack; change this to consistent behavior
by making error the same for all bundlers (e.g., set error = true
unconditionally) and remove the special-case that injects warn: 'once'. Update
the ImpoundPlugin.vite call (using nuxtProtectionConfig and error) and the
construction that merges warn so that addVitePlugin(...) receives the same
strict config for both envOptions variations.
- Around line 399-409: sharedProtectionConfig currently enables trace: true and
is registered via addBuildPlugin with prepend: true, causing the Vite trace
transform to be prepended and run before SFC compilation; mirror the nuxt-app
split used for nuxtProtectionConfig by splitting the Vite registration for
sharedProtectionConfig into a main resolver plugin and a trace transform plugin
(use ImpoundPlugin.vite.main and ImpoundPlugin.vite.trace or equivalent entry
points) so the trace transform runs after SFC compilation, and register the pair
in addBuildPlugin in the same order and options as nuxtProtectionConfig to
ensure correct plugin ordering for shared import protection (referencing
sharedProtectionConfig, ImpoundPlugin.vite, addBuildPlugin, and
nuxtProtectionConfig).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 7874346b-6276-47a3-a2a1-c23e8c950837
⛔ Files ignored due to path filters (3)
packages/nitro-server/package.jsonis excluded by!**/package.jsonpackages/nuxt/package.jsonis excluded by!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (1)
packages/nuxt/src/core/nuxt.ts
| const sharedProtectionConfig = { | ||
| cwd: nuxt.options.rootDir, | ||
| trace: true, | ||
| include: sharedPatterns, | ||
| patterns: createImportProtectionPatterns(nuxt, { context: 'shared' }), | ||
| } | ||
| addBuildPlugin({ | ||
| vite: () => ImpoundPlugin.vite(sharedProtectionConfig), | ||
| webpack: () => ImpoundPlugin.webpack(sharedProtectionConfig), | ||
| rspack: () => ImpoundPlugin.rspack(sharedProtectionConfig), | ||
| }, { server: false, prepend: true }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/nuxt && sed -n '395,444p' src/core/nuxt.tsRepository: nuxt/nuxt
Length of output: 2609
🏁 Script executed:
cd packages/nuxt && sed -n '380,450p' src/core/nuxt.tsRepository: nuxt/nuxt
Length of output: 3507
🏁 Script executed:
cd packages/nuxt && grep -n "nuxtProtectionConfig\|sharedProtectionConfig" src/core/nuxt.ts | head -20Repository: nuxt/nuxt
Length of output: 639
🏁 Script executed:
find . -name "*.ts" -o -name "*.js" | xargs grep -l "ImpoundPlugin" | grep -v node_modules | head -10Repository: nuxt/nuxt
Length of output: 169
🏁 Script executed:
rg "class ImpoundPlugin|export.*ImpoundPlugin" --type ts --type jsRepository: nuxt/nuxt
Length of output: 35
🏁 Script executed:
rg "import.*ImpoundPlugin" packages/nuxt/src/core/nuxt.ts -A 2 -B 2Repository: nuxt/nuxt
Length of output: 249
🏁 Script executed:
cat packages/nuxt/test/import-protection.test.tsRepository: nuxt/nuxt
Length of output: 2545
🏁 Script executed:
rg "sharedProtectionConfig|shared.*trace" packages/nuxt --type tsRepository: nuxt/nuxt
Length of output: 417
🏁 Script executed:
cd packages/nuxt && sed -n '412,445p' src/core/nuxt.ts | cat -nRepository: nuxt/nuxt
Length of output: 1941
🏁 Script executed:
cd packages/nuxt && git log --oneline --all -S "sharedProtectionConfig" | head -10Repository: nuxt/nuxt
Length of output: 139
Mirror the Vite main/trace plugin split for sharedProtectionConfig.
Both sharedProtectionConfig and nuxtProtectionConfig enable trace: true, but only the nuxt-app path (lines 428–442) splits the Vite plugins into main resolvers and trace transforms. The code comment at lines 424–427 documents why: the trace plugin must run after SFC compilation so that es-module-lexer can parse accurate code snippets. Registering sharedProtectionConfig via addBuildPlugin(..., { prepend: true }) prepends all plugins together, which will place the trace transform before compilation rather than after, breaking the intended tracing behaviour for shared folder violations.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nuxt/src/core/nuxt.ts` around lines 399 - 409,
sharedProtectionConfig currently enables trace: true and is registered via
addBuildPlugin with prepend: true, causing the Vite trace transform to be
prepended and run before SFC compilation; mirror the nuxt-app split used for
nuxtProtectionConfig by splitting the Vite registration for
sharedProtectionConfig into a main resolver plugin and a trace transform plugin
(use ImpoundPlugin.vite.main and ImpoundPlugin.vite.trace or equivalent entry
points) so the trace transform runs after SFC compilation, and register the pair
in addBuildPlugin in the same order and options as nuxtProtectionConfig to
ensure correct plugin ordering for shared import protection (referencing
sharedProtectionConfig, ImpoundPlugin.vite, addBuildPlugin, and
nuxtProtectionConfig).
| for (const envOptions of [ | ||
| { client: false } as const, | ||
| { server: false } as const, | ||
| ]) { | ||
| const error = envOptions.client === false ? false : true | ||
| const vitePlugins = [ImpoundPlugin.vite({ ...nuxtProtectionConfig, error, ...(error === false && { warn: 'once' as const }) })].flat() | ||
| .map(p => Object.assign(p, { name: `nuxt:import-protection:${p.name}` })) | ||
| const mainPlugins = vitePlugins.filter(p => !p.name.includes('trace')) | ||
| const tracePlugins = vitePlugins.filter(p => p.name.includes('trace')) | ||
| if (mainPlugins.length) { | ||
| addVitePlugin(() => mainPlugins, { ...envOptions, prepend: true }) | ||
| } | ||
| if (tracePlugins.length) { | ||
| addVitePlugin(() => tracePlugins, envOptions) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's look at the full context around the code section
cat -n packages/nuxt/src/core/nuxt.ts | sed -n '420,450p'Repository: nuxt/nuxt
Length of output: 1756
🏁 Script executed:
# Search for any related comments about import protection or ImpoundPlugin
rg -A 5 -B 5 "ImpoundPlugin" packages/nuxt/src/core/nuxt.tsRepository: nuxt/nuxt
Length of output: 2394
🏁 Script executed:
# Look for documentation or comments about error handling in this area
rg -B 10 "envOptions.client === false" packages/nuxt/src/core/nuxt.tsRepository: nuxt/nuxt
Length of output: 575
🏁 Script executed:
# Check if there are related tests that explain the expected behaviour
fd -t f "*.test.ts" -o "*.spec.ts" | xargs rg -l "import-protection\|ImpoundPlugin" 2>/dev/null | head -5Repository: nuxt/nuxt
Length of output: 190
🏁 Script executed:
# Let's check the webpack and rspack configurations for comparison
cat -n packages/nuxt/src/core/nuxt.ts | sed -n '400,445p'Repository: nuxt/nuxt
Length of output: 2501
🏁 Script executed:
# Search for ImpoundPlugin documentation or any related issues/PRs
rg -B 20 "nuxtProtectionConfig" packages/nuxt/src/core/nuxt.ts | head -60Repository: nuxt/nuxt
Length of output: 2177
🏁 Script executed:
# Check if there are any git history or comments about SSR-only error handling decisions
rg "SSR-only\|server-only" packages/nuxt/src/core/nuxt.tsRepository: nuxt/nuxt
Length of output: 35
🏁 Script executed:
# Look for any test files that might explain the expected behaviour
find . -name "*.test.ts" -o -name "*.spec.ts" | xargs grep -l "import-protection\|ImpoundPlugin" 2>/dev/null | head -3Repository: nuxt/nuxt
Length of output: 99
🏁 Script executed:
# Read the test file to understand expected behaviour
cat packages/nuxt/test/import-protection.test.tsRepository: nuxt/nuxt
Length of output: 2545
🏁 Script executed:
# Check if there are any recent PRs or commits related to this logic
git log --oneline -n 20 -- packages/nuxt/src/core/nuxt.ts | head -20Repository: nuxt/nuxt
Length of output: 88
🏁 Script executed:
# Search for any documentation or comments about the distinction between Vite and webpack/rspack
rg "webpack\|rspack" packages/nuxt/src/core/nuxt.ts -B 2 -A 2Repository: nuxt/nuxt
Length of output: 35
🏁 Script executed:
# Search for any related issues or PRs discussing SSR-only import protection
rg "SSR\|server-only\|client.*false" packages/nuxt --type ts -B 3 -A 3 | grep -i "import\|protection\|error\|warn" | head -30Repository: nuxt/nuxt
Length of output: 35
🏁 Script executed:
# Check the impound plugin documentation or comments about error vs warn
rg "error.*false\|warn.*once" packages/nuxt -B 5 -A 5 | head -50Repository: nuxt/nuxt
Length of output: 35
🏁 Script executed:
# Search for any GitHub issues or PR discussions (check if there are related comments in the file)
git log --all --grep="import.*protection\|SSR.*import" --oneline 2>/dev/null | head -10Repository: nuxt/nuxt
Length of output: 286
🏁 Script executed:
# Look for any environment variable or configuration that controls this behaviour
rg "NUXT.*IMPORT\|IMPORT.*PROTECTION" packages/nuxtRepository: nuxt/nuxt
Length of output: 35
🏁 Script executed:
# Search for the commit that introduced this error = false logic
git log -p --all -- packages/nuxt/src/core/nuxt.ts | grep -B 20 -A 20 "error = envOptions.client === false" | head -60Repository: nuxt/nuxt
Length of output: 35
🏁 Script executed:
# Check if there are any related PRs or issues mentioning this
git log --all --oneline -- packages/nuxt/src/core/nuxt.ts | grep -i "vite\|import\|protect\|error\|warn" | head -20Repository: nuxt/nuxt
Length of output: 1598
🏁 Script executed:
# Look for any documentation about import protection behaviour
find . -name "*.md" -o -name "*.txt" | xargs grep -l "import.*protection\|ImpoundPlugin" 2>/dev/nullRepository: nuxt/nuxt
Length of output: 37
🏁 Script executed:
# Check the exact line that sets error to understand the ternary logic better
ast-grep --pattern 'const error = $$$'Repository: nuxt/nuxt
Length of output: 3396
Ensure consistent error handling across bundlers for import protection.
The SSR-only configuration (client: false) downgrades import protection violations to warnings with warn: 'once', while webpack and rspack receive the default strict configuration. This makes Vite less strict than webpack/rspack for the same protection rule and violates the coding guideline to "Use error handling patterns consistently". Either all bundlers should warn, or all should error—the behaviour should be uniform.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/nuxt/src/core/nuxt.ts` around lines 428 - 442, The code currently
treats SSR-only (envOptions.client === false) as non-error by setting error =
false and adding warn: 'once', causing Vite to be less strict than
webpack/rspack; change this to consistent behavior by making error the same for
all bundlers (e.g., set error = true unconditionally) and remove the
special-case that injects warn: 'once'. Update the ImpoundPlugin.vite call
(using nuxtProtectionConfig and error) and the construction that merges warn so
that addVitePlugin(...) receives the same strict config for both envOptions
variations.
🔗 Linked issue
📚 Description
inspired by some of the nice features in tanstack start, this backports some of them to https://github.com/unjs/impound, our bundler/framework-agnostic solution for import protection
in particular, we now show suggestions + show a 'trace' of where the import came from
🚧 TODO