perf(nuxt,vite,webpack): use string parsing for module ids#34451
perf(nuxt,vite,webpack): use string parsing for module ids#34451
Conversation
|
|
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughA new 🚥 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: 1
🧹 Nitpick comments (1)
packages/vite/src/plugins/ssr-styles.ts (1)
16-18: Consider centralising duplicated regex constants.
STYLE_QUERY_REis defined identically here and inruntime-paths.ts. Additionally,packages/nuxt/src/core/utils/plugins.tsalready exports similar constants (MACRO_RE,NUXT_COMPONENT_RE) that could be reused instead of redefiningMACRO_QUERY_REandNUXT_COMPONENT_QUERY_RE.Consider exporting
STYLE_QUERY_REfrom the core utils alongside the existing patterns and importing them where needed to reduce duplication.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/vite/src/plugins/ssr-styles.ts` around lines 16 - 18, Duplicate regex constants are being defined; export STYLE_QUERY_RE from the central core utils module that already exports MACRO_RE and NUXT_COMPONENT_RE, then remove the local definitions and import the shared constants instead of redefining MACRO_QUERY_RE, NUXT_COMPONENT_QUERY_RE and STYLE_QUERY_RE; also delete the duplicate STYLE_QUERY_RE in the other module (runtime-paths.ts) and update its usages to import the shared STYLE_QUERY_RE so all files use the single exported regex constant.
🤖 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/pages/plugins/page-meta.ts`:
- Around line 380-387: The macro detection is order-sensitive because it checks
id.includes('?macro=true') instead of reading the parsed query; update the logic
in the block around parseModuleId and params so you set query.macro from
params.get('macro') (e.g., query.macro = params.get('macro') ?? undefined or
query.macro = params.get('macro') === 'true' ? 'true' : undefined) rather than
using id.includes('?macro=true'), ensuring
parseModuleId(id.replace(/\?macro=true$/,'')) still runs and query.macro is
populated whenever macro is present anywhere in the query string.
---
Nitpick comments:
In `@packages/vite/src/plugins/ssr-styles.ts`:
- Around line 16-18: Duplicate regex constants are being defined; export
STYLE_QUERY_RE from the central core utils module that already exports MACRO_RE
and NUXT_COMPONENT_RE, then remove the local definitions and import the shared
constants instead of redefining MACRO_QUERY_RE, NUXT_COMPONENT_QUERY_RE and
STYLE_QUERY_RE; also delete the duplicate STYLE_QUERY_RE in the other module
(runtime-paths.ts) and update its usages to import the shared STYLE_QUERY_RE so
all files use the single exported regex constant.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 27179682-4049-4069-96c1-fe30345f8cff
📒 Files selected for processing (12)
packages/nuxt/src/components/plugins/islands-transform.tspackages/nuxt/src/components/plugins/transform.tspackages/nuxt/src/components/plugins/tree-shake.tspackages/nuxt/src/core/plugins/extract-async-data-handlers.tspackages/nuxt/src/core/plugins/keyed-functions.tspackages/nuxt/src/core/utils/index.tspackages/nuxt/src/core/utils/plugins.tspackages/nuxt/src/pages/plugins/page-meta.tspackages/vite/src/plugins/runtime-paths.tspackages/vite/src/plugins/ssr-styles.tspackages/vite/src/utils/index.tspackages/webpack/src/plugins/ssr-styles.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/pages/plugins/page-meta.ts`:
- Around line 383-388: The regexes TYPE_PARAM_RE, LANG_PARAM_RE and
MACRO_QUERY_RE are being executed against the full id which can include path
segments that look like query tokens; restrict these checks to the actual query
string instead (e.g., extract the search/query portion from id before testing)
so that query.type, query.lang and query.macro are derived only from the URL
query, not the pathname; update the code that builds query (the variable query
and uses of TYPE_PARAM_RE, LANG_PARAM_RE, MACRO_QUERY_RE on id) to run those
regexes against the isolated search/query substring (or parsed search via URL
parsing) before assigning to query.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 25522a06-cf93-4c54-bfcc-5f6d40ee99d5
📒 Files selected for processing (1)
packages/nuxt/src/pages/plugins/page-meta.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/nuxt/src/core/plugins/keyed-functions.ts (1)
30-35: Consider extracting shared constants to reduce duplication.The constants
SUPPORTED_EXT_RE,STYLE_QUERY_RE, andMACRO_QUERY_REare defined identically inpackages/nuxt/src/core/plugins/extract-async-data-handlers.ts(lines 10-13). Extracting these to a shared location (e.g., a constants file incore/utils/) would reduce duplication and ensure consistency if patterns need updating in future.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/nuxt/src/core/plugins/keyed-functions.ts` around lines 30 - 35, Extract the duplicated regex constants SUPPORTED_EXT_RE, STYLE_QUERY_RE, and MACRO_QUERY_RE into a shared module (e.g., export them from a new core/utils/constants or core/utils/regexes file), replace the local declarations in keyed-functions.ts (remove the SUPPORTED_EXT_RE, STYLE_QUERY_RE, MACRO_QUERY_RE definitions) with imports from that shared module, and update the other file (extract-async-data-handlers.ts) to import the same symbols; ensure the exported names match exactly (SUPPORTED_EXT_RE, STYLE_QUERY_RE, MACRO_QUERY_RE) and run a quick search to update any other usages to import from the new shared file.
🤖 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/pages/plugins/page-meta.ts`:
- Around line 54-55: The include regex /[?&]macro=true/ is too loose and matches
values like macro=truex; update the include used in page-meta plugin to use the
same precise pattern as MACRO_QUERY_RE (i.e., require a parameter boundary after
"true" such as & or end/hash/encoded equivalents) so only exact macro=true
matches pass; replace the current include regex with the MACRO_QUERY_RE or an
equivalent boundary-aware regex and keep the existing exclude entry unchanged.
---
Nitpick comments:
In `@packages/nuxt/src/core/plugins/keyed-functions.ts`:
- Around line 30-35: Extract the duplicated regex constants SUPPORTED_EXT_RE,
STYLE_QUERY_RE, and MACRO_QUERY_RE into a shared module (e.g., export them from
a new core/utils/constants or core/utils/regexes file), replace the local
declarations in keyed-functions.ts (remove the SUPPORTED_EXT_RE, STYLE_QUERY_RE,
MACRO_QUERY_RE definitions) with imports from that shared module, and update the
other file (extract-async-data-handlers.ts) to import the same symbols; ensure
the exported names match exactly (SUPPORTED_EXT_RE, STYLE_QUERY_RE,
MACRO_QUERY_RE) and run a quick search to update any other usages to import from
the new shared file.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 02c7c174-8e0b-404b-a6f9-477f6c4ec736
📒 Files selected for processing (5)
packages/nuxt/src/components/plugins/tree-shake.tspackages/nuxt/src/core/plugins/extract-async-data-handlers.tspackages/nuxt/src/core/plugins/keyed-functions.tspackages/nuxt/src/core/utils/plugins.tspackages/nuxt/src/pages/plugins/page-meta.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/nuxt/src/core/plugins/extract-async-data-handlers.ts
🔗 Linked issue
📚 Description
this is a performance-focused PR to improve id resolution speed - in some situations this is 14 THOUSAND times faster.
did some quick benchmarks with
mitata- obviously this is a micro-benchmark but within a plugin, this is called often and seems worthwhile ...previously we were doing a
new URL()construction viapathToFileURL,decodeURIComponent, then 3 regex matches insideparseURL/parsePathfromufo.The new parseModuleId is just a single
indexOf('?')+ at most two slice calls — both O(1) in V8 as they create string slices without copying ...