Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork5.5k
perf(nuxt,vite): add more filters to internal plugins#33898
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
|
| return{ | ||
| code:[ | ||
| 'import { defineAsyncComponent } from "vue"', | ||
| `${exportWording} defineAsyncComponent(() => import(${JSON.stringify(bare)}).then(r => r[${JSON.stringify(componentExport)}] || r.default || r))`, |
Check warning
Code scanning / CodeQL
Improper code sanitization Medium
improperly sanitized value
Uh oh!
There was an error while loading.Please reload this page.
Show autofix suggestionHide autofix suggestion
Copilot Autofix
AI 1 day ago
To fix this, we should perform additional escaping for the string inserted into the JavaScript code beyond justJSON.stringify. Specifically, we should escape the following characters even after stringification:<,>,/,\u2028, and\u2029, as well as any others recommended for this context. This requires defining an escaping function (e.g.,escapeUnsafeChars) and wrapping the stringified values ofbare and any other dynamic identifiers (such ascomponentExport if required, but the main vulnerability is with the file path) before interpolating them into template literals that form JavaScript code.
- Import or define a function (can be placed inside the file) that does this escaping.
- Replace
JSON.stringify(bare)withescapeUnsafeChars(JSON.stringify(bare))(and optionally do the same for other applicable dynamic values). - No change is needed to functionality, only additional sanitization is performed prior to code generation.
- The new
escapeUnsafeCharsfunction should be placed above or near the code block that uses it for clarity.
| @@ -11,6 +11,25 @@ | ||
| importtype{getComponentsT}from'../module.ts' | ||
| importtype{Nuxt}from'nuxt/schema' | ||
| // Utility to safely escape potentially dangerous characters in strings inserted into JS source. | ||
| constcharMap={ | ||
| '<':'\\u003C', | ||
| '>':'\\u003E', | ||
| '/':'\\u002F', | ||
| '\\':'\\\\', | ||
| '\b':'\\b', | ||
| '\f':'\\f', | ||
| '\n':'\\n', | ||
| '\r':'\\r', | ||
| '\t':'\\t', | ||
| '\0':'\\0', | ||
| '\u2028':'\\u2028', | ||
| '\u2029':'\\u2029' | ||
| } | ||
| functionescapeUnsafeChars(str:string){ | ||
| returnstr.replace(/[<>\/\\\b\f\n\r\t\0\u2028\u2029]/g,x=>charMap[xaskeyoftypeofcharMap]) | ||
| } | ||
| constCOMPONENT_QUERY_RE=/[?&]nuxt_component=/ | ||
| interfaceTransformPluginOptions{ | ||
| @@ -80,7 +99,7 @@ | ||
| return{ | ||
| code:[ | ||
| 'import { defineAsyncComponent } from "vue"', | ||
| `${exportWording} defineAsyncComponent(() => import(${JSON.stringify(bare)}).then(r => r[${JSON.stringify(componentExport)}] || r.default || r))`, | ||
| `${exportWording} defineAsyncComponent(() => import(${escapeUnsafeChars(JSON.stringify(bare))}).then(r => r[${JSON.stringify(componentExport)}] || r.default || r))`, | ||
| ].join('\n'), | ||
| map:null, | ||
| } |
| return{ | ||
| code:[ | ||
| 'import { defineAsyncComponent } from "vue"', | ||
| `${exportWording} defineAsyncComponent(() => import(${JSON.stringify(bare)}).then(r => r[${JSON.stringify(componentExport)}] || r.default || r))`, |
Check warning
Code scanning / CodeQL
Improper code sanitization Medium
improperly sanitized value
Uh oh!
There was an error while loading.Please reload this page.
Show autofix suggestionHide autofix suggestion
Copilot Autofix
AI 1 day ago
To fix the problem, we need to ensure that any dynamic content inserted into generated JavaScript code—especially content derived from query parameters—is sanitized so as not to introduce injection vulnerabilities. This involves escaping potentially dangerous characters in the output ofJSON.stringify(componentExport) before interpolating it in the constructed code string. The fix should include a utility function (akin toescapeUnsafeChars in the background) that replaces such characters with Unicode escape sequences. Specifically, we will:
- Add a function to escape unsafe characters in strings used in code generation (in the same file).
- Replace direct use of
JSON.stringify(componentExport)with a call to the escape function on its result (only in places where user-controlled values are interpolated in code, such as line 83). - Similarly, for line 110 (
JSON.stringify(name)), which comes from query params, use the escaping function there too.
We will add theescapeUnsafeChars function at the top of the file and use it at both vulnerable sites.
| @@ -11,6 +11,25 @@ | ||
| importtype{getComponentsT}from'../module.ts' | ||
| importtype{Nuxt}from'nuxt/schema' | ||
| // Escape unsafe characters from user-controlled input inserted into generated JS code | ||
| constcharMap={ | ||
| '<':'\\u003C', | ||
| '>':'\\u003E', | ||
| '/':'\\u002F', | ||
| '\\':'\\\\', | ||
| '\b':'\\b', | ||
| '\f':'\\f', | ||
| '\n':'\\n', | ||
| '\r':'\\r', | ||
| '\t':'\\t', | ||
| '\0':'\\0', | ||
| '\u2028':'\\u2028', | ||
| '\u2029':'\\u2029' | ||
| }; | ||
| functionescapeUnsafeChars(str:string):string{ | ||
| returnstr.replace(/[<>\b\f\n\r\t\0\u2028\u2029\\/]/g,x=>charMap[x]||x) | ||
| } | ||
| constCOMPONENT_QUERY_RE=/[?&]nuxt_component=/ | ||
| interfaceTransformPluginOptions{ | ||
| @@ -80,7 +99,7 @@ | ||
| return{ | ||
| code:[ | ||
| 'import { defineAsyncComponent } from "vue"', | ||
| `${exportWording} defineAsyncComponent(() => import(${JSON.stringify(bare)}).then(r => r[${JSON.stringify(componentExport)}] || r.default || r))`, | ||
| `${exportWording} defineAsyncComponent(() => import(${JSON.stringify(bare)}).then(r => r[${escapeUnsafeChars(JSON.stringify(componentExport))}] || r.default || r))`, | ||
| ].join('\n'), | ||
| map:null, | ||
| } | ||
| @@ -107,7 +126,7 @@ | ||
| return{ | ||
| code:[ | ||
| `import { createServerComponent } from${JSON.stringify(options.serverComponentRuntime)}`, | ||
| `${exportWording} createServerComponent(${JSON.stringify(name)})`, | ||
| `${exportWording} createServerComponent(${escapeUnsafeChars(JSON.stringify(name))})`, | ||
| ].join('\n'), | ||
| map:null, | ||
| } |
| code:[ | ||
| 'import { defineAsyncComponent } from "vue"', | ||
| 'import { createClientOnly } from "#app/components/client-only"', | ||
| `${exportWording} defineAsyncComponent(() => import(${JSON.stringify(bare)}).then(r => createClientOnly(r[${JSON.stringify(componentExport)}] || r.default || r)))`, |
Check warning
Code scanning / CodeQL
Improper code sanitization Medium
improperly sanitized value
Uh oh!
There was an error while loading.Please reload this page.
Show autofix suggestionHide autofix suggestion
Copilot Autofix
AI 1 day ago
To fix the problem, we need to ensure that any variable interpolated into JavaScript code as a string (particularly intoimport() statements) is further sanitized to escape potentially dangerous characters even afterJSON.stringify. The recommended way is to define anescapeUnsafeChars function as described in the example, which replaces dangerous characters (such as<,>,/,\b,\f,\n,\r,\t,\0,\u2028, and\u2029) with their Unicode escapes so that they cannot break out of a<script> block or similar dangerous context.
We will implement this function at the top of the file (after imports), and then on line 101, wrap bothJSON.stringify(bare) andJSON.stringify(componentExport) withescapeUnsafeChars(...). This is the minimum change needed to preserve existing functionality and harden against injection. The other cases on lines 83 and 110 could benefit from the same, but the prompt asks for a fix targeted at the code on 101.
| @@ -11,6 +11,25 @@ | ||
| importtype{getComponentsT}from'../module.ts' | ||
| importtype{Nuxt}from'nuxt/schema' | ||
| // Escape characters that could break out of a script or enable injection | ||
| constcharMap:Record<string,string>={ | ||
| '<':'\\u003C', | ||
| '>':'\\u003E', | ||
| '/':'\\u002F', | ||
| '\\':'\\\\', | ||
| '\b':'\\b', | ||
| '\f':'\\f', | ||
| '\n':'\\n', | ||
| '\r':'\\r', | ||
| '\t':'\\t', | ||
| '\0':'\\0', | ||
| '\u2028':'\\u2028', | ||
| '\u2029':'\\u2029' | ||
| } | ||
| functionescapeUnsafeChars(str:string){ | ||
| returnstr.replace(/[<>\/\\\b\f\n\r\t\0\u2028\u2029]/g,x=>charMap[x]||x) | ||
| } | ||
| constCOMPONENT_QUERY_RE=/[?&]nuxt_component=/ | ||
| interfaceTransformPluginOptions{ | ||
| @@ -98,7 +117,7 @@ | ||
| code:[ | ||
| 'import { defineAsyncComponent } from "vue"', | ||
| 'import { createClientOnly } from "#app/components/client-only"', | ||
| `${exportWording} defineAsyncComponent(() => import(${JSON.stringify(bare)}).then(r => createClientOnly(r[${JSON.stringify(componentExport)}] || r.default || r)))`, | ||
| `${exportWording} defineAsyncComponent(() => import(${escapeUnsafeChars(JSON.stringify(bare))}).then(r => createClientOnly(r[${escapeUnsafeChars(JSON.stringify(componentExport))}] || r.default || r)))`, | ||
| ].join('\n'), | ||
| map:null, | ||
| } |
| code:[ | ||
| 'import { defineAsyncComponent } from "vue"', | ||
| 'import { createClientOnly } from "#app/components/client-only"', | ||
| `${exportWording} defineAsyncComponent(() => import(${JSON.stringify(bare)}).then(r => createClientOnly(r[${JSON.stringify(componentExport)}] || r.default || r)))`, |
Check warning
Code scanning / CodeQL
Improper code sanitization Medium
improperly sanitized value
Uh oh!
There was an error while loading.Please reload this page.
Show autofix suggestionHide autofix suggestion
Copilot Autofix
AI 1 day ago
The fix is to apply additional escaping to the value output byJSON.stringify(componentExport), ensuring that any HTML context breaking characters (<,>,/,\,\b,\f,\n,\r,\t,\0,\u2028,\u2029) are replaced with safe Unicode escape sequences. We'll define anescapeUnsafeChars function (using the standard mapping from the example), and wrap all relevantJSON.stringify() output for identifiers included in the generated code with this function. Inpackages/nuxt/src/components/plugins/transform.ts, this applies directly to line 101 (and also line 83 and 110 if you want complete coverage) — any place where a user-controllable string is inserted into generated code viaJSON.stringify.
We'll insert the utility function at the top of the file after imports. Then, we'll update line 101 to wrapJSON.stringify(componentExport) withescapeUnsafeChars(). For consistency and defense-in-depth, do the same for line 83, 110, and 93/102/111 if they output user-controllable values, but CodeQL flagged specifically line 101.
| @@ -11,6 +11,26 @@ | ||
| importtype{getComponentsT}from'../module.ts' | ||
| importtype{Nuxt}from'nuxt/schema' | ||
| // Escape unsafe characters in JSON stringified output used in code construction | ||
| constcharMap={ | ||
| '<':'\\u003C', | ||
| '>':'\\u003E', | ||
| '/':'\\u002F', | ||
| '\\':'\\\\', | ||
| '\b':'\\b', | ||
| '\f':'\\f', | ||
| '\n':'\\n', | ||
| '\r':'\\r', | ||
| '\t':'\\t', | ||
| '\0':'\\0', | ||
| '\u2028':'\\u2028', | ||
| '\u2029':'\\u2029' | ||
| }; | ||
| functionescapeUnsafeChars(str:string):string{ | ||
| returnstr.replace(/[<>\b\f\n\r\t\0\u2028\u2029/\\]/g,x=>charMap[xaskeyoftypeofcharMap]||x); | ||
| } | ||
| constCOMPONENT_QUERY_RE=/[?&]nuxt_component=/ | ||
| interfaceTransformPluginOptions{ | ||
| @@ -80,7 +100,7 @@ | ||
| return{ | ||
| code:[ | ||
| 'import { defineAsyncComponent } from "vue"', | ||
| `${exportWording} defineAsyncComponent(() => import(${JSON.stringify(bare)}).then(r => r[${JSON.stringify(componentExport)}] || r.default || r))`, | ||
| `${exportWording} defineAsyncComponent(() => import(${JSON.stringify(bare)}).then(r => r[${escapeUnsafeChars(JSON.stringify(componentExport))}] || r.default || r))`, | ||
| ].join('\n'), | ||
| map:null, | ||
| } | ||
| @@ -98,7 +118,7 @@ | ||
| code:[ | ||
| 'import { defineAsyncComponent } from "vue"', | ||
| 'import { createClientOnly } from "#app/components/client-only"', | ||
| `${exportWording} defineAsyncComponent(() => import(${JSON.stringify(bare)}).then(r => createClientOnly(r[${JSON.stringify(componentExport)}] || r.default || r)))`, | ||
| `${exportWording} defineAsyncComponent(() => import(${JSON.stringify(bare)}).then(r => createClientOnly(r[${escapeUnsafeChars(JSON.stringify(componentExport))}] || r.default || r)))`, | ||
| ].join('\n'), | ||
| map:null, | ||
| } | ||
| @@ -107,7 +127,7 @@ | ||
| return{ | ||
| code:[ | ||
| `import { createServerComponent } from${JSON.stringify(options.serverComponentRuntime)}`, | ||
| `${exportWording} createServerComponent(${JSON.stringify(name)})`, | ||
| `${exportWording} createServerComponent(${escapeUnsafeChars(JSON.stringify(name))})`, | ||
| ].join('\n'), | ||
| map:null, | ||
| } |
pkg-pr-newbot commentedDec 16, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@nuxt/kit@nuxt/nitro-servernuxt@nuxt/rspack-builder@nuxt/schema@nuxt/vite-builder@nuxt/webpack-buildercommit: |
coderabbitaibot commentedDec 16, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
WalkthroughThis PR refactors many plugin hooks from plain functions to structured hook objects with Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for usingCodeRabbit! 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/vite/src/plugins/layer-dep-optimize.ts (1)
28-38:Consider restoring thedirs.lengthearly exit for performance.The removed
dirs.lengthcheck means the handler continues to execute even whendirsis empty (all layers have been processed). While functionally correct (findIndex returns -1), restoring the check would avoid unnecessarynormalizecalls:resolveId: { async handler (source, _importer) {- if (!_importer) { return }+ if (!_importer || !dirs.length) { return } const importer = normalize(_importer)packages/nuxt/src/pages/plugins/page-meta.ts (2)
62-69:Complex filter patterns may benefit from a clarifying comment.The fourth pattern at line 67 uses triple negative lookaheads to match files that don't already contain the expected output (
__nuxt_page_meta,export { default }) or the macro call. This acts as a catch-all for files needing the empty/dev fallback. A brief comment explaining this intent would help future maintainers.code: { include: [ HAS_MACRO_RE, /\bfrom\s+["'][^"'?]*\?[^"']*type=script[^"']*["']/, /export\s+\{\s*default\s*\}\s+from\s+["'][^"'?]*\?[^"']*type=script[^"']*["']/,+ // Catch-all for files without expected output patterns - triggers empty/dev fallback /^(?!.*__nuxt_page_meta)(?!.*export\s+\{\s*default\s*\})(?!.*\bdefinePageMeta\s*\()[\s\S]*$/, ], },
235-321:Consider early exit for multiple definePageMeta detection.The error for multiple
definePageMetacalls is thrown at line 320 after the walk completes, meaning both calls are processed before the error. For the error case, this results in wasted work. An early exit could improve this, though it's an edge case.walk(ast, { scopeTracker, enter: (node) => { if (node.type !== 'CallExpression' || node.callee.type !== 'Identifier') { return } if (!('name' in node.callee) || node.callee.name !== 'definePageMeta') { return }- instances+++ if (instances++ > 0) {+ throw new Error('Multiple `definePageMeta` calls are not supported. File: ' + id.replace(/\?.+$/, ''))+ } const meta = node.arguments[0] // ... rest of processing
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
packages/nuxt/src/components/plugins/transform.ts(1 hunks)packages/nuxt/src/core/plugins/resolved-externals.ts(2 hunks)packages/nuxt/src/core/plugins/unctx.ts(2 hunks)packages/nuxt/src/core/plugins/virtual.ts(3 hunks)packages/nuxt/src/imports/module.ts(1 hunks)packages/nuxt/src/imports/transform.ts(1 hunks)packages/nuxt/src/pages/plugins/page-meta.ts(2 hunks)packages/vite/src/plugins/layer-dep-optimize.ts(1 hunks)packages/vite/src/plugins/sourcemap-preserver.ts(2 hunks)packages/vite/src/plugins/ssr-styles.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/imports/transform.tspackages/nuxt/src/core/plugins/unctx.tspackages/nuxt/src/components/plugins/transform.tspackages/vite/src/plugins/sourcemap-preserver.tspackages/vite/src/plugins/layer-dep-optimize.tspackages/nuxt/src/core/plugins/virtual.tspackages/nuxt/src/core/plugins/resolved-externals.tspackages/nuxt/src/imports/module.tspackages/vite/src/plugins/ssr-styles.tspackages/nuxt/src/pages/plugins/page-meta.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,vue}: Use clear, descriptive variable and function names
Add comments only to explain complex logic or non-obvious implementations
Keep functions focused and manageable (generally under 50 lines), and extract complex logic into separate domain-specific files
Remove code that is not used or needed
Use error handling patterns consistently
Files:
packages/nuxt/src/imports/transform.tspackages/nuxt/src/core/plugins/unctx.tspackages/nuxt/src/components/plugins/transform.tspackages/vite/src/plugins/sourcemap-preserver.tspackages/vite/src/plugins/layer-dep-optimize.tspackages/nuxt/src/core/plugins/virtual.tspackages/nuxt/src/core/plugins/resolved-externals.tspackages/nuxt/src/imports/module.tspackages/vite/src/plugins/ssr-styles.tspackages/nuxt/src/pages/plugins/page-meta.ts
🧠 Learnings (7)
📓 Common learnings
Learnt from: GalacticHypernovaRepo: nuxt/nuxt PR: 26468File: packages/nuxt/src/components/plugins/loader.ts:24-24Timestamp: 2024-11-05T15:22:54.759ZLearning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.Learnt from: huang-julienRepo: nuxt/nuxt PR: 29366File: packages/nuxt/src/app/components/nuxt-root.vue:16-19Timestamp: 2024-12-12T12:36:34.871ZLearning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:```jsconst IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null```instead of wrapping the import in a computed property or importing the component unconditionally.📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernovaRepo: nuxt/nuxt PR: 26468File: packages/nuxt/src/components/plugins/loader.ts:24-24Timestamp: 2024-11-05T15:22:54.759ZLearning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.Applied to files:
packages/nuxt/src/imports/transform.tspackages/nuxt/src/core/plugins/unctx.tspackages/nuxt/src/components/plugins/transform.tspackages/vite/src/plugins/sourcemap-preserver.tspackages/vite/src/plugins/layer-dep-optimize.tspackages/nuxt/src/core/plugins/virtual.tspackages/nuxt/src/core/plugins/resolved-externals.tspackages/nuxt/src/imports/module.tspackages/nuxt/src/pages/plugins/page-meta.ts
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julienRepo: nuxt/nuxt PR: 29366File: packages/nuxt/src/app/components/nuxt-root.vue:16-19Timestamp: 2024-12-12T12:36:34.871ZLearning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:```jsconst IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null```instead of wrapping the import in a computed property or importing the component unconditionally.Applied to files:
packages/nuxt/src/components/plugins/transform.tspackages/vite/src/plugins/layer-dep-optimize.tspackages/nuxt/src/core/plugins/resolved-externals.tspackages/nuxt/src/imports/module.tspackages/nuxt/src/pages/plugins/page-meta.ts
📚 Learning: 2025-04-18T18:33:41.753Z
Learnt from: TheAlexLichterRepo: nuxt/nuxt PR: 31812File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202Timestamp: 2025-04-18T18:33:41.753ZLearning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.Applied to files:
packages/vite/src/plugins/sourcemap-preserver.ts
📚 Learning: 2024-11-28T21:22:40.496Z
Learnt from: GalacticHypernovaRepo: nuxt/nuxt PR: 29661File: packages/kit/src/template.ts:227-229Timestamp: 2024-11-28T21:22:40.496ZLearning: In `packages/kit/src/template.ts`, when updating the `EXTENSION_RE` regular expression for TypeScript configuration, avoid using patterns like `(\.\w+)+$` as they can result in catastrophic backtracking.Applied to files:
packages/vite/src/plugins/ssr-styles.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CRRepo: nuxt/nuxt PR: 0File: .github/copilot-instructions.md:0-0Timestamp: 2025-11-25T11:42:16.132ZLearning: Applies to **/*.{ts,tsx,js,jsx,vue} : Use error handling patterns consistentlyApplied to files:
packages/vite/src/plugins/ssr-styles.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CRRepo: nuxt/nuxt PR: 0File: .github/copilot-instructions.md:0-0Timestamp: 2025-11-25T11:42:16.132ZLearning: Applies to **/*.{ts,tsx,js,jsx,vue} : Remove code that is not used or neededApplied to files:
packages/nuxt/src/pages/plugins/page-meta.ts
🧬 Code graph analysis (3)
packages/nuxt/src/imports/transform.ts (1)
packages/nuxt/src/components/plugins/transform.ts (1)
TransformPlugin(22-159)
packages/nuxt/src/components/plugins/transform.ts (1)
packages/kit/src/ignore.ts (1)
isIgnored(15-35)
packages/nuxt/src/pages/plugins/page-meta.ts (2)
packages/nuxt/src/utils.ts (1)
logger(14-14)packages/nuxt/src/pages/utils.ts (1)
isSerializable(670-716)
🪛 ast-grep (0.40.0)
packages/nuxt/src/core/plugins/unctx.ts
[warning] 28-28: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(\\b(${keys.join('|')})\\()
Note: [CWE-1333] Inefficient Regular Expression Complexity [REFERENCES]
-https://owasp.org/www-community/attacks/Regular_expression_Denial_of_Service_-_ReDoS
-https://cwe.mitre.org/data/definitions/1333.html
(regexp-from-variable)
🪛 GitHub Check: CodeQL
packages/nuxt/src/components/plugins/transform.ts
[warning] 83-83: Improper code sanitization
Code construction depends on animproperly sanitized value.
[warning] 83-83: Improper code sanitization
Code construction depends on animproperly sanitized value.
[warning] 101-101: Improper code sanitization
Code construction depends on animproperly sanitized value.
[warning] 101-101: Improper code sanitization
Code construction depends on animproperly sanitized value.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: code
🔇 Additional comments (18)
packages/nuxt/src/imports/module.ts (1)
125-131:LGTM! Consider simplifying the ctx wrapper.The restructured call aligns with the new
TransformPluginOptionsinterface. The wrapper function(code, id, options) => ctx.injectImports(code, id, options)is functionally a pass-through. If thectxvariable's type is compatible, you could potentially pass{ injectImports: ctx.injectImports.bind(ctx) }or similar, though this current approach is clear and explicit.packages/nuxt/src/core/plugins/virtual.ts (1)
133-136:LGTM! Good cross-platform path handling.The
escapeDirectoryhelper correctly escapes special regex characters and normalises path separators to match both Unix (/) and Windows (\) styles.packages/vite/src/plugins/ssr-styles.ts (3)
221-250:LGTM! Client-side transform logic is well-structured.The handler properly checks for global CSS, handles resolution failures with cached warnings, and only returns a result when changes have been made.
252-320:LGTM! Server-side transform logic maintains existing behaviour.The refactored handler preserves the original logic: early exits for unsupported files, proper island path handling, CSS chunk emission, and static import processing with deduplication.
211-220:Hook timing assumption is incorrect for current Vite.The applyToEnvironment hook is called at config time, currently after configResolved, so the
entryvariable will be set before the filter is constructed. The suggested guard is unnecessary for current Vite versions.Environment plugins resolution may be moved before configResolved in the future, but this is not a current concern requiring action.
Likely an incorrect or invalid review comment.
packages/nuxt/src/imports/transform.ts (1)
13-19:LGTM! Clean interface extraction.The
TransformPluginOptionsinterface provides clear documentation of the plugin's requirements. UsingPick<Unimport, 'injectImports'>appropriately constrains thectxtype to only the method actually used.packages/nuxt/src/core/plugins/unctx.ts (2)
17-29:Good early exit optimisation; ReDoS risk is low given controlled input.The empty keys early exit avoids unnecessary transformer creation. Regarding the static analysis ReDoS warning on line 29: the
keysarray originates fromtransformerOptionswhich is framework-controlled configuration (function names likewithAsyncContext), not user input. The regex pattern\b(key1|key2)\(uses simple alternation without nested quantifiers, making catastrophic backtracking unlikely.If stricter guarantees are desired, you could validate that keys contain only safe identifier characters:
+const SAFE_IDENTIFIER_RE = /^[\w$]+$/+const safeKeys = keys.filter(k => SAFE_IDENTIFIER_RE.test(k))+if (safeKeys.length === 0) {+ return { name: 'unctx:transform' }+}-const INCLUDE_RE = new RegExp(`\\b(${keys.join('|')})\\(`)+const INCLUDE_RE = new RegExp(`\\b(${safeKeys.join('|')})\\(`)
37-56:LGTM! Efficient filter structure with safety net.The
include/excludefilter provides efficient pre-filtering before the handler executes. The retainedshouldTransformcheck (line 46) serves as a webpack compatibility safety net, which is appropriate given the TODO comment.packages/nuxt/src/core/plugins/resolved-externals.ts (1)
43-62:LGTM! Well-structured filter-based resolveId pattern.The conversion to the filter/handler pattern is clean. The use of
escapeStringRegexpensures special characters in dependency names (like@nuxt/kit) are properly escaped for the regex matching.packages/nuxt/src/components/plugins/transform.ts (3)
59-66:Good separation of concerns with the plugin split.Splitting into
imports-wrapperandimports-aliasplugins improves maintainability. ThetransformIncludelogic correctly gates processing to virtual files, build directory files, and non-ignored files.
71-116:Acknowledge static analysis but context is trusted.The CodeQL warnings about "improper code sanitization" at lines 83 and 101 relate to
bareandcomponentExportbeing used in code generation. However:
- These values originate from Nuxt's internal component registration (via
getComponentsImports)JSON.stringifyprovides escaping for the import paths- The fallback error at line 115 catches unexpected modes
This is acceptable for internal build-time code generation.
140-154:Clearing and repopulating imports ensures fresh component state.The pattern of clearing
imports.length = 0before pushing fresh components ensures the import list reflects the current component state. The conditional sourcemap generation based onnuxt.options.sourcemapsettings is appropriate.packages/nuxt/src/pages/plugins/page-meta.ts (3)
167-185:Cycle-safety check is well-implemented.The
declaration !== nodeguard at line 181 prevents infinite recursion when processing self-referential declarations. The inline comment clearly explains the edge case being handled.
191-224:Solid AST traversal logic for declaration processing.The separation of Variable and Function declaration handling is clean. The await detection at lines 199-202 provides a clear error message. Arrow functions correctly defer to variable handling since they're assigned to variables.
373-378:Clean helper functions for node position resolution.The
resolveStartandresolveEndhelpers elegantly handle the difference between function nodes (which havefnNode) and regular nodes.packages/vite/src/plugins/sourcemap-preserver.ts (3)
5-5:LGTM!The import of
escapeStringRegexpis appropriate for constructing the filter regex safely.
18-18:LGTM!The factory function pattern appropriately delays plugin initialization until Nitro's build phase, ensuring
outputDiris set before the filter regex is constructed.
24-42:LGTM!The handler implementation correctly:
- Resolves the id to an absolute path
- Performs an early exit check against the
idsSet for performance- Reads code and sourcemap files in parallel with appropriate error handling
- Returns the expected
{code, map}formatThe custom
.map.jsonextension is consistent with thewriteBundlehook implementation.
Uh oh!
There was an error while loading.Please reload this page.
| load:{ | ||
| filter:{ | ||
| id:newRegExp('^(\\w:)?'+escapeStringRegexp(outputDir).replace(/\//g,'[\\\\/]')), | ||
| }, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash# Examine the sourcemap-preserver.ts filecd /tmpgit clone --depth=1 https://github.com/nuxt/nuxt.git2>/dev/null||trueif [-f nuxt/packages/vite/src/plugins/sourcemap-preserver.ts ];then head -50 nuxt/packages/vite/src/plugins/sourcemap-preserver.tsfi
Repository: nuxt/nuxt
Length of output: 1370
🏁 Script executed:
#!/bin/bash# Search for the sourcemap-preserver.ts filefind. -name"sourcemap-preserver.ts" -type f2>/dev/null| head -5
Repository: nuxt/nuxt
Length of output: 103
🏁 Script executed:
#!/bin/bash# Test the regex pattern matching without needing the actual filenode<< 'EOF'// Simulate escapeStringRegexp behaviorfunction escapeStringRegexp(str) { return str .replace(/[|\\{}()[\]^$+*?.]/g, '\\$&') .replace(/-/g, '\\x2d');}// Test casesfunction testFilter(outputDir, testPaths) { const pattern = '^(\\w:)?' + escapeStringRegexp(outputDir).replace(/\//g, '[\\\\\\\/]'); const regex = new RegExp(pattern); console.log(`\nOutput dir: ${outputDir}`); console.log(`Regex pattern: ${pattern}`); console.log(`Compiled regex: ${regex}`); testPaths.forEach(path => { const matches = regex.test(path); console.log(` "${path}": ${matches ? 'MATCH' : 'NO MATCH'}`); });}// Test 1: Unix-style pathstestFilter('/project/dist', [ '/project/dist/file.js', '/project/dist-backup/file.js', '/project/dist2/file.js', '\\\\project\\\\dist\\\\file.js',]);// Test 2: Windows-style pathstestFilter('C:\\project\\dist', [ 'C:\\project\\dist\\file.js', 'D:\\project\\dist\\file.js', 'C:C:\\project\\dist\\file.js',]);// Test 3: Mixed forward slashes (common in Windows)testFilter('C:/project/dist', [ 'C:/project/dist/file.js', 'D:/project/dist/file.js', 'D:C:/project/dist/file.js', 'C:/project/dist-backup/file.js',]);// Test 4: Verify the regex doesn't need word boundarytestFilter('/app/output', [ '/app/output/bundle.js', '/app/output-cache/bundle.js', '/app/output_old/bundle.js',]);EOF
Repository: nuxt/nuxt
Length of output: 1047
🏁 Script executed:
#!/bin/bashnode<<'NODEEOF'// Simulate escapeStringRegexp behavior (sindresorhus implementation)function escapeStringRegexp(str) { return str.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&').replace(/-/g, '\\x2d');}// Test the actual pattern constructionfunction testRegexBehaviour(outputDir, testPaths) { const escaped = escapeStringRegexp(outputDir); const slashReplaced = escaped.replace(/\//g, '[\\\\\\\/]'); const pattern = '^(\\w:)?' + slashReplaced; const regex = new RegExp(pattern); console.log(`\n=== Testing: ${outputDir} ===`); console.log(`Escaped: "${escaped}"`); console.log(`Pattern: "${pattern}"`); testPaths.forEach(path => { const result = regex.test(path); console.log(` "${path}": ${result ? 'MATCH' : 'no match'}`); });}// Test case 1: Unix absolute pathtestRegexBehaviour('/project/dist', [ '/project/dist/file.js', '/project/dist-backup/file.js', 'W:/project/dist/file.js',]);// Test case 2: Windows path with backslashestestRegexBehaviour('C:\\\\project\\\\dist', [ 'C:\\\\project\\\\dist\\\\file.js', 'C:/project/dist/file.js', 'D:\\\\project\\\\dist\\\\file.js', 'D:C:\\\\project\\\\dist\\\\file.js',]);// Test case 3: Windows path with forward slashestestRegexBehaviour('C:/project/dist', [ 'C:/project/dist/file.js', 'D:/project/dist/file.js', 'C:/project/dist-backup/file.js',]);NODEEOF
Repository: nuxt/nuxt
Length of output: 745
🏁 Script executed:
#!/bin/bash# Find and examine the filefd sourcemap-preserver.ts2>/dev/null| head -3
Repository: nuxt/nuxt
Length of output: 101
🏁 Script executed:
#!/bin/bashnode<<'NODEEOF'// Simulate escapeStringRegexp (sindresorhus/escape-string-regexp)function escapeStringRegexp(str) { return str.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&').replace(/-/g, '\\x2d');}// Test the exact regex pattern from the codefunction testRegex(outputDir, testPaths) { const escaped = escapeStringRegexp(outputDir); const slashReplaced = escaped.replace(/\//g, '[\\\\\\\/]'); const pattern = '^(\\w:)?' + slashReplaced; const regex = new RegExp(pattern); console.log(`\n=== OUTPUT DIR: ${outputDir} ===`); console.log(`Pattern: ${pattern}`); testPaths.forEach(path => { const result = regex.test(path); console.log(` "${path}": ${result ? 'MATCH' : 'no match'}`); });}// Test case 1: Unix absolute pathconsole.log('TEST 1: Unix paths');testRegex('/project/dist', [ '/project/dist/file.js', '/project/dist-backup/file.js', '/project/dist2/file.js', '/project/dist_old/file.js',]);// Test case 2: Windows paths with backslashesconsole.log('\nTEST 2: Windows paths (backslashes)');testRegex('C:\\project\\dist', [ 'C:\\project\\dist\\file.js', 'C:/project/dist/file.js', 'D:\\project\\dist\\file.js', 'D:C:\\project\\dist\\file.js',]);// Test case 3: Windows paths with forward slashesconsole.log('\nTEST 3: Windows paths (forward slashes)');testRegex('C:/project/dist', [ 'C:/project/dist/file.js', 'D:/project/dist/file.js', 'C:/project/dist-backup/file.js', 'C:/project/dist_old/file.js',]);// Test case 4: Path with special regex charactersconsole.log('\nTEST 4: Path with special characters');testRegex('/project/[dist]', [ '/project/[dist]/file.js', '/project/dist/file.js',]);NODEEOF
Repository: nuxt/nuxt
Length of output: 1010
🏁 Script executed:
node<<'EOF'// Simulate escapeStringRegexp (sindresorhus)function escapeStringRegexp(str) { return str.replace(/[|\\{}()[\]^$+*?.]/g, '\\$&').replace(/-/g, '\\x2d');}// Test exact regex from codefunction test(outputDir, paths) { const escaped = escapeStringRegexp(outputDir); const slashReplaced = escaped.replace(/\//g, '[\\\\\\\/]'); const pattern = '^(\\w:)?' + slashReplaced; const regex = new RegExp(pattern); console.log(`\n=== ${outputDir} ===\nPattern: ${pattern}\n`); paths.forEach(p => { console.log(` ${regex.test(p) ? 'YES' : 'NO '} "${p}"`); });}test('/project/dist', [ '/project/dist/file.js', '/project/dist-backup/file.js', '/project/dist2/file.js']);test('C:/project/dist', [ 'C:/project/dist/file.js', 'D:/project/dist/file.js', 'C:/project/dist-backup/file.js', 'D:C:/project/dist/file.js']);test('C:\\project\\dist', [ 'C:\\project\\dist\\file.js', 'D:\\project\\dist\\file.js', 'D:C:\\project\\dist\\file.js']);EOF
Repository: nuxt/nuxt
Length of output: 577
Refine the filter regex to match only the intended output directory path.
The current regex pattern matches overly broadly:
- Paths like "/project/dist-backup" match the filter for "/project/dist"
- On Windows, "D:C:\project\dist" matches when outputDir is "C:\project\dist"
Add a path separator or end-of-string anchor after the escaped outputDir to ensure the filter matches only the exact directory path, improving the performance optimisation without changing the safety guarantees provided by theids.has() check.
🤖 Prompt for AI Agents
In packages/vite/src/plugins/sourcemap-preserver.ts around lines 20 to 23, thecurrent regex used for filtering outputDir is too permissive (it matchesprefixes like "/project/dist-backup" or malformed Windows paths); update thepattern so that after the escaped outputDir you require either a path separatoror the end of string. Concretely, append a non-capturing group that matcheseither [\/] or end-of-string (e.g. '(?:[\\\\/]|$)') to the escaped outputDirreplacement so the filter only matches the exact directory path or its subpathswhile preserving the existing '^(\\w:)?' prefix and overall behavior.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/nuxt/test/page-metadata.test.ts (1)
392-392:Refine the type annotation for better type safety.The type annotation has the following issues:
- The return type is incomplete: it should include the
mapproperty (e.g.,{ code: string, map?: any }) andundefinedin the union (based on the handler implementation in page-meta.ts).- The
as anytype assertion for the second parameter bypasses type checking.Consider this diff to improve type accuracy:
- const transformPlugin = PageMetaPlugin({ extractedKeys: ['extracted'] }).raw({}, {} as any) as { transform: { handler: (code: string, id: string) => { code: string } | null } }+ const transformPlugin = PageMetaPlugin({ extractedKeys: ['extracted'] }).raw({}, {} as any) as { transform: { handler: (code: string, id: string) => { code: string, map?: any } | null | undefined } }Alternatively, if the map property is not relevant for these tests, you could add a comment explaining why it's omitted.
packages/nuxt/test/components-transform.test.ts (1)
116-121:Consider improving type safety for the plugins array.The multi-pass transformation logic correctly threads results through each plugin's handler sequentially. However, the
as any[]assertion on line 117 bypasses type checking.Consider typing the plugins array properly:
- for (const plugin of plugins as any[]) {+ for (const plugin of plugins) { const result = await plugin.transform.handler(transformResult || code, id)Alternatively, if TypeScript inference isn't working, explicitly type the variable on line 109:
constplugins=TransformPlugin(stubNuxt,{// ...}).vite()asArray<{transform:{handler:(code:string,id:string)=>Promise<{code:string}|string|undefined>}}>Note: The test directly invokes handlers without checking the transform filters (
filter.id,filter.code), which differs from production Vite behavior. This is acceptable for unit testing since the handlers contain internal guards, but be aware the execution path differs from the actual plugin pipeline.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/nuxt/test/components-transform.test.ts(1 hunks)packages/nuxt/test/page-metadata.test.ts(13 hunks)packages/nuxt/test/virtual.test.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/test/components-transform.test.tspackages/nuxt/test/page-metadata.test.tspackages/nuxt/test/virtual.test.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,vue}: Use clear, descriptive variable and function names
Add comments only to explain complex logic or non-obvious implementations
Keep functions focused and manageable (generally under 50 lines), and extract complex logic into separate domain-specific files
Remove code that is not used or needed
Use error handling patterns consistently
Files:
packages/nuxt/test/components-transform.test.tspackages/nuxt/test/page-metadata.test.tspackages/nuxt/test/virtual.test.ts
**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Write unit tests for core functionality using
vitest
Files:
packages/nuxt/test/components-transform.test.tspackages/nuxt/test/page-metadata.test.tspackages/nuxt/test/virtual.test.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: GalacticHypernovaRepo: nuxt/nuxt PR: 26468File: packages/nuxt/src/components/plugins/loader.ts:24-24Timestamp: 2024-11-05T15:22:54.759ZLearning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.Learnt from: huang-julienRepo: nuxt/nuxt PR: 29366File: packages/nuxt/src/app/components/nuxt-root.vue:16-19Timestamp: 2024-12-12T12:36:34.871ZLearning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:```jsconst IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null```instead of wrapping the import in a computed property or importing the component unconditionally.📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernovaRepo: nuxt/nuxt PR: 26468File: packages/nuxt/src/components/plugins/loader.ts:24-24Timestamp: 2024-11-05T15:22:54.759ZLearning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.Applied to files:
packages/nuxt/test/components-transform.test.tspackages/nuxt/test/page-metadata.test.tspackages/nuxt/test/virtual.test.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CRRepo: nuxt/nuxt PR: 0File: .github/copilot-instructions.md:0-0Timestamp: 2025-11-25T11:42:16.132ZLearning: Applies to **/*.{test,spec}.{ts,tsx,js} : Write unit tests for core functionality using `vitest`Applied to files:
packages/nuxt/test/components-transform.test.tspackages/nuxt/test/virtual.test.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CRRepo: nuxt/nuxt PR: 0File: .github/copilot-instructions.md:0-0Timestamp: 2025-11-25T11:42:16.132ZLearning: Applies to **/e2e/**/*.{ts,tsx,js} : Write end-to-end tests using Playwright and `nuxt/test-utils`Applied to files:
packages/nuxt/test/components-transform.test.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CRRepo: nuxt/nuxt PR: 0File: .github/copilot-instructions.md:0-0Timestamp: 2025-11-25T11:42:16.132ZLearning: Applies to **/*.vue : Use `<script setup lang="ts">` and the composition API when creating Vue componentsApplied to files:
packages/nuxt/test/page-metadata.test.ts
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julienRepo: nuxt/nuxt PR: 29366File: packages/nuxt/src/app/components/nuxt-root.vue:16-19Timestamp: 2024-12-12T12:36:34.871ZLearning: In `packages/nuxt/src/app/components/nuxt-root.vue`, when optimizing bundle size by conditionally importing components based on route metadata, prefer using inline conditional imports like:```jsconst IsolatedPage = route?.meta?.isolate ? defineAsyncComponent(() => import('#build/isolated-page.mjs')) : null```instead of wrapping the import in a computed property or importing the component unconditionally.Applied to files:
packages/nuxt/test/page-metadata.test.ts
🧬 Code graph analysis (2)
packages/nuxt/test/components-transform.test.ts (1)
packages/nuxt/src/components/plugins/transform.ts (1)
TransformPlugin(22-159)
packages/nuxt/test/page-metadata.test.ts (1)
packages/nuxt/src/pages/plugins/page-meta.ts (1)
PageMetaPlugin(50-347)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: code
- GitHub Check: check-provenance
🔇 Additional comments (3)
packages/nuxt/test/virtual.test.ts (1)
55-55:LGTM! Test stub now explicitly includes buildDir.The addition of
buildDir: '/.nuxt'makes the stub Nuxt configuration more complete and aligns with the refactored VirtualFSPlugin's expectations. The value correctly matches the#buildalias mapping.packages/nuxt/test/page-metadata.test.ts (1)
407-840:LGTM! Consistent adaptation to the new transform API.All test cases have been correctly updated to use
transform.handler(...)instead of the previous directtransform(...)pattern. The changes maintain test coverage and correctly validate the plugin's behaviour across error cases, code extraction, import handling, shadowing logic, and metadata stripping.packages/nuxt/test/components-transform.test.ts (1)
109-109:Variable naming accurately reflects multi-plugin architecture.The renaming from
plugintopluginscorrectly indicates thatTransformPluginnow returns an array of plugins, aligning with the new split-plugin design.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/nuxt/src/core/plugins/virtual.ts (1)
84-84:Fix regex alternation precedence issue.The current pattern
'^' + Array.from(relevantAliases).join('|') + '([\\\\/]|$)'produces incorrect precedence. Due to how alternation works in regex, this becomes(^alias1)|(alias2)|(alias3([\\/]|$)), meaning only the last alternative has the path separator check applied.Apply this diff to wrap alternatives in a non-capturing group:
- ...relevantAliases.size ? [new RegExp('^' + Array.from(relevantAliases).join('|') + '([\\\\/]|$)')] : [],+ ...relevantAliases.size ? [new RegExp('^(?:' + Array.from(relevantAliases).join('|') + ')([\\\\/]|$)')] : [],Based on the past review comments, this ensures all alternatives are anchored at the start and followed by a path separator or end-of-string.
🧹 Nitpick comments (1)
packages/vite/src/plugins/sourcemap-preserver.ts (1)
33-35:Consider including the file id in the warning for easier debugging.The warning message would be more actionable if it included which file failed to load.
if (!code) {- this.warn('Failed loading file')+ this.warn(`Failed loading file: ${id}`) return null }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/nuxt/src/core/plugins/virtual.ts(3 hunks)packages/vite/src/plugins/sourcemap-preserver.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/core/plugins/virtual.tspackages/vite/src/plugins/sourcemap-preserver.ts
**/*.{ts,tsx,js,jsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx,js,jsx,vue}: Use clear, descriptive variable and function names
Add comments only to explain complex logic or non-obvious implementations
Keep functions focused and manageable (generally under 50 lines), and extract complex logic into separate domain-specific files
Remove code that is not used or needed
Use error handling patterns consistently
Files:
packages/nuxt/src/core/plugins/virtual.tspackages/vite/src/plugins/sourcemap-preserver.ts
🧠 Learnings (3)
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernovaRepo: nuxt/nuxt PR: 26468File: packages/nuxt/src/components/plugins/loader.ts:24-24Timestamp: 2024-11-05T15:22:54.759ZLearning: In `packages/nuxt/src/components/plugins/loader.ts`, the references to `resolve` and `distDir` are legacy code from before Nuxt used the new unplugin VFS and will be removed.Applied to files:
packages/nuxt/src/core/plugins/virtual.tspackages/vite/src/plugins/sourcemap-preserver.ts
📚 Learning: 2024-11-28T21:22:40.496Z
Learnt from: GalacticHypernovaRepo: nuxt/nuxt PR: 29661File: packages/kit/src/template.ts:227-229Timestamp: 2024-11-28T21:22:40.496ZLearning: In `packages/kit/src/template.ts`, when updating the `EXTENSION_RE` regular expression for TypeScript configuration, avoid using patterns like `(\.\w+)+$` as they can result in catastrophic backtracking.Applied to files:
packages/nuxt/src/core/plugins/virtual.tspackages/vite/src/plugins/sourcemap-preserver.ts
📚 Learning: 2025-04-18T18:33:41.753Z
Learnt from: TheAlexLichterRepo: nuxt/nuxt PR: 31812File: packages/nuxt/src/components/plugins/islands-transform.ts:202-202Timestamp: 2025-04-18T18:33:41.753ZLearning: In Nuxt, using `rolldownVersion` (not `rollupVersion`) is intentional when detecting if rolldown-vite is being used, even though TypeScript may show an error because the property isn't in standard type definitions yet.Applied to files:
packages/vite/src/plugins/sourcemap-preserver.ts
🔇 Additional comments (8)
packages/vite/src/plugins/sourcemap-preserver.ts (2)
18-23:The trailing-slash normalisation addresses the previous regex concern.The addition of
.replace(/\/?$/, '/')before escaping ensures the filter requires a path separator after the output directory, preventing false matches on paths like/project/dist-backupwhenoutputDiris/project/dist. The factory function pattern correctly capturesoutputDirvia closure at plugin instantiation time.
46-50:LGTM!The factory function is correctly passed to the plugins array, ensuring
outputDiris captured at the appropriate time when Rollup instantiates the plugin.packages/nuxt/src/core/plugins/virtual.ts (6)
62-68:LGTM!The logic correctly identifies aliases that point to virtual file system paths and escapes them for safe regex usage.
70-75:LGTM!The logic correctly collects VFS entry directories whilst excluding build-related paths, and properly escapes them for regex usage.
91-91:LGTM!The conditional integration of the filter into the
resolveIdhook is correct and properly avoids duplication with the Vite-specific hook.
96-96:LGTM!The filter is correctly integrated into the Vite-specific
resolveIdhook, maintaining consistency with the generic hook.
134-136:LGTM!The
escapeDirectoryhelper correctly escapes paths for regex usage and handles cross-platform path separators by converting forward slashes to a character class that matches both forward and backslashes.
6-6: The import statement on line 6 is correct and uses the standard ESM import pattern for theescape-string-regexppackage, which is already declared as a dependency in the Nuxt framework's package.json. No action required.
Uh oh!
There was an error while loading.Please reload this page.
CodSpeed Performance ReportMerging#33898 willnot alter performanceComparing Summary
|
| { | ||
| name:'nuxt:components:imports-wrapper', | ||
| enforce:'post', | ||
| transformInclude(id){ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
could we use parts of thetransformInclude in the hook filter as well?
| import{tryImportModule,useNitro}from'@nuxt/kit' | ||
| importtype{Nuxt}from'@nuxt/schema' | ||
| import{resolveModulePath}from'exsolve' | ||
| importescapeStringRegexpfrom'escape-string-regexp' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Might be worth usinghttps://github.com/rolldown/rolldown/tree/main/packages/pluginutils instead?
🔗 Linked issue
📚 Description
this follows up on a number of other PRs to add more regexp-based filters to our plugins, which should improve performance when using rolldown-vite