refactor(kit,nuxt,ui-templates,vite): address deprecations + improve regexp perf#33093
refactor(kit,nuxt,ui-templates,vite): address deprecations + improve regexp perf#33093
Conversation
|
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
|
Warning Rate limit exceeded@danielroe has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 12 minutes and 6 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (2)
WalkthroughThe PR adds TypeScript-aware ESLint configuration (imports @typescript-eslint/parser and a TS lint block) and inserts numerous eslint-disable comments for deprecated rules. It systematically refactors many regular expressions to use non-capturing groups, converts some String.match() usages to RegExp.test(), and removes/adjusts global flags in split/replace patterns. Functional changes include helper start/end resolvers in page-meta, regex/splitting adjustments in ui-templates, a preset swap from presetUno to presetWind3, and extensive test updates (regex changes and flaky-test retries). Most edits are stylistic or lint-related; a few files have non-trivial logic rearrangements around parsing and template handling. Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
packages/nuxt/src/core/runtime/nitro/handlers/island.ts (1)
76-86: Array merge is inadvertently overwritten in islandHead accumulationAfter pushing into the existing array, the subsequent assignment overwrites it with
value, discarding previous entries. Guard by merging only for arrays and otherwise assigning.Apply this diff:
- for (const [key, value] of Object.entries(resolveUnrefHeadInput(entry.input as any) as SerializableHead)) { - const currentValue = islandHead[key as keyof SerializableHead] - if (Array.isArray(currentValue)) { - currentValue.push(...value) - } - islandHead[key as keyof SerializableHead] = value - } + for (const [key, value] of Object.entries(resolveUnrefHeadInput(entry.input as any) as SerializableHead)) { + const k = key as keyof SerializableHead + const curr = islandHead[k] as unknown + if (Array.isArray(value)) { + if (Array.isArray(curr)) { + (curr as unknown[]).push(...value) + } else { + islandHead[k] = value as any + } + } else { + islandHead[k] = value as any + } + }packages/nuxt/src/core/nuxt.ts (1)
851-851: Regex likely won’t match absolute paths; restart trigger may never fire.RESTART_RE is anchored to start/end. It is tested against an absolute path (Line 701), so it won’t match “.../app.vue”. Make it segment-aware.
Apply:
-const RESTART_RE = /^(?:app|error|app\.config)\.(?:js|ts|mjs|jsx|tsx|vue)$/i +const RESTART_RE = /(?:^|[\\/])(?:app|error|app\.config)\.(?:js|ts|mjs|jsx|tsx|vue)$/iAlternatively, test the basename at use-site.
packages/nuxt/src/components/plugins/islands-transform.ts (1)
59-65: Avoid lastIndex pitfalls from using a global regex with .test()
SCRIPT_REis global; calling.test()mutateslastIndexand can affect subsequent uses. Use a non-global literal for the check.- if (!SCRIPT_RE.test(code)) { + if (!/<script[^>]*>/i.test(code)) { s.prepend('<script setup>' + IMPORT_CODE + '</script>') } else { s.replace(SCRIPT_RE, (full) => { return full + IMPORT_CODE }) }packages/ui-templates/lib/render.ts (1)
61-66: Regression: SVG URLs in CSS are no longer captured/inlinedDropping the capture from the
url(...)branch meanssrcisundefinedfor CSS URLs, so.svgassets won’t be inlined anymore.- for (const [_, src] of html.matchAll(/src="([^"]+)"|url\([^)]+\)/g)) { - if (src?.match(/\.svg$/)) { - svgSources.push(src) - } - } + for (const match of html.matchAll(/src="(?<src>[^"]+)"|url\((?<url>[^)]+)\)/g)) { + const src = (match.groups?.src || match.groups?.url)?.replace(/^['"]|['"]$/g, '') + if (src?.endsWith('.svg')) { + svgSources.push(src) + } + }
🧹 Nitpick comments (9)
packages/nuxt/src/core/nitro.ts (1)
269-277: Scoped deprecation suppression is appropriateInline disabling for this one check keeps lint noise down without broad suppression. Consider adding a TODO with the Nuxt v5 removal plan next to it.
packages/kit/src/plugin.ts (1)
45-52: Targeted suppression around legacyplugin.ssrlooks goodThis preserves existing behaviour while silencing the warning. Optionally emit a dev-time deprecation warning when
ssris present to help users migrate tomode: 'server'or filename suffixes.packages/nuxt/src/core/templates.ts (1)
61-61: Non-capturing group improves regex perf with no behaviour changeGood switch to
(?:...). Minor nit:/(?:4[5-7])/is a tad shorter and a brief comment noting these map to- . /(ASCII 45–47) would aid readability.Also applies to: 71-72, 91-92
packages/nuxt/src/components/plugins/lazy-hydration-transform.ts (1)
84-84: Avoid regex for hot-path prefix strippingTiny perf/readability win: use string prefix checks/slices instead of a regex inside the AST walk.
- const pascalName = pascalCase(node.name.replace(/^(?:Lazy|lazy-)/, '')) + const name = node.name + const base = name.startsWith('Lazy') ? name.slice(4) : (name.startsWith('lazy-') ? name.slice(5) : name) + const pascalName = pascalCase(base)packages/vite/src/utils/warmup.ts (1)
27-27: Safer removal ofimportquery preserves delimitersCurrent replace can collapse adjacent params (e.g.
?a=1&import&b=2→?a=1b=2). Keep the preceding delimiter when the match is followed by&.- url = url.replace(/[?&]import=?(?:&|$)/, '').replace(/[?&]$/, '') + url = url + .replace(/([?&])import=?(?:&|$)/, (m, p1) => m.endsWith('&') ? p1 : '') + .replace(/[?&]$/, '')packages/nuxt/src/core/plugins/plugin-metadata.ts (1)
41-49: Cache the negative path to avoid repeated scansWhen returning early with
{}, consider caching it to prevent repeated checks on identical content.- if (!/(?:defineNuxtPlugin|definePayloadPlugin)\s*\([\w(]/.test(code)) { - return {} - } + if (!/(?:defineNuxtPlugin|definePayloadPlugin)\s*\([\w(]/.test(code)) { + metaCache[code] = {} + return metaCache[code] + }packages/nuxt/src/dirs.ts (1)
5-5: Match directory name, not just suffix.Using a suffix regex can misfire on directories like "my-shared". Prefer checking the last path segment.
Apply:
-import { dirname, resolve } from 'pathe' +import { basename, dirname, resolve } from 'pathe' -let _distDir = dirname(fileURLToPath(import.meta.url)) -if (/(?:chunks|shared)$/.test(_distDir)) { _distDir = dirname(_distDir) } +let _distDir = dirname(fileURLToPath(import.meta.url)) +if (['chunks', 'shared'].includes(basename(_distDir))) { _distDir = dirname(_distDir) }eslint.config.mjs (1)
99-113: Scope TS parser/rule to TS files to avoid unnecessary JS/MJS overheadApplying the TS parser and
@typescript-eslint/no-deprecatedacross.js/.mjscan slow linting and cause noise. Consider narrowing to TypeScript-only globs.- files: ['packages/**/*.{mjs,js,ts}', '**/*.{spec,test}.{mjs,js,ts}'], + files: ['packages/**/*.{ts,mts,cts}', '**/*.{spec,test}.{ts,mts,cts}'],test/basic.test.ts (1)
2135-2139: Minor: normalise CSS url(...) values to avoid quote-related false negatives
url(...)may be quoted. Normalising improves robustness of the startsWith checks.- for (const match of html.matchAll(/(?:href|src)="(.*?)"|url\(([^)]*)\)/g)) { - const url = match[1] || match[2]! + for (const match of html.matchAll(/(?:href|src)="(.*?)"|url\(([^)]*)\)/g)) { + const url = (match[1] || match[2]!)?.replace(/^['"]|['"]$/g, '') expect(url.startsWith('/_nuxt/') || isPublicFile('/', url)).toBeTruthy() }(Apply the same normalisation in the other loops.)
Also applies to: 2166-2170, 2181-2187, 2211-2215
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (2)
package.jsonis excluded by!package.json,!**/package.jsonpnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (25)
eslint.config.mjs(3 hunks)packages/kit/src/context.ts(2 hunks)packages/kit/src/internal/esm.ts(3 hunks)packages/kit/src/plugin.ts(1 hunks)packages/nuxt/src/components/plugins/islands-transform.ts(2 hunks)packages/nuxt/src/components/plugins/lazy-hydration-transform.ts(1 hunks)packages/nuxt/src/components/scan.ts(1 hunks)packages/nuxt/src/core/builder.ts(1 hunks)packages/nuxt/src/core/nitro.ts(1 hunks)packages/nuxt/src/core/nuxt.ts(1 hunks)packages/nuxt/src/core/plugins/plugin-metadata.ts(1 hunks)packages/nuxt/src/core/runtime/nitro/handlers/island.ts(2 hunks)packages/nuxt/src/core/templates.ts(1 hunks)packages/nuxt/src/dirs.ts(1 hunks)packages/nuxt/src/head/runtime/composables.ts(3 hunks)packages/nuxt/src/pages/plugins/page-meta.ts(4 hunks)packages/nuxt/test/pages.test.ts(1 hunks)packages/ui-templates/lib/dev.ts(1 hunks)packages/ui-templates/lib/render.ts(3 hunks)packages/ui-templates/uno.config.ts(1 hunks)packages/vite/src/dirs.ts(1 hunks)packages/vite/src/runtime/vite-node.mjs(1 hunks)packages/vite/src/utils/warmup.ts(1 hunks)test/basic.test.ts(14 hunks)test/nuxt/nuxt-time.test.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/core/builder.tspackages/nuxt/src/core/nitro.tspackages/vite/src/utils/warmup.tspackages/kit/src/plugin.tspackages/nuxt/src/pages/plugins/page-meta.tspackages/nuxt/test/pages.test.tspackages/nuxt/src/core/plugins/plugin-metadata.tspackages/nuxt/src/components/plugins/lazy-hydration-transform.tspackages/nuxt/src/core/nuxt.tspackages/nuxt/src/head/runtime/composables.tspackages/nuxt/src/core/templates.tspackages/nuxt/src/core/runtime/nitro/handlers/island.tspackages/ui-templates/uno.config.tspackages/kit/src/context.tstest/basic.test.tspackages/vite/src/dirs.tspackages/nuxt/src/dirs.tspackages/nuxt/src/components/scan.tstest/nuxt/nuxt-time.test.tspackages/kit/src/internal/esm.tspackages/nuxt/src/components/plugins/islands-transform.tspackages/ui-templates/lib/dev.tspackages/ui-templates/lib/render.ts
**/*.{test,spec}.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Write unit tests for core functionality using
vitest
Files:
packages/nuxt/test/pages.test.tstest/basic.test.tstest/nuxt/nuxt-time.test.ts
🧠 Learnings (6)
📓 Common learnings
Learnt from: GalacticHypernova
PR: nuxt/nuxt#29661
File: packages/kit/src/template.ts:227-229
Timestamp: 2024-11-28T21:22:40.496Z
Learning: 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.
📚 Learning: 2024-11-28T21:22:40.496Z
Learnt from: GalacticHypernova
PR: nuxt/nuxt#29661
File: packages/kit/src/template.ts:227-229
Timestamp: 2024-11-28T21:22:40.496Z
Learning: 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/builder.tspackages/nuxt/src/core/templates.tstest/basic.test.tspackages/vite/src/dirs.tspackages/nuxt/src/components/scan.tspackages/nuxt/src/components/plugins/islands-transform.tspackages/ui-templates/lib/render.ts
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julien
PR: nuxt/nuxt#29366
File: packages/nuxt/src/app/components/nuxt-root.vue:16-19
Timestamp: 2024-12-12T12:36:34.871Z
Learning: 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:
```js
const 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/core/builder.ts
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
PR: nuxt/nuxt#26468
File: packages/nuxt/src/components/plugins/loader.ts:24-24
Timestamp: 2024-11-05T15:22:54.759Z
Learning: 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/builder.tspackages/nuxt/src/core/nuxt.tspackages/nuxt/src/dirs.ts
📚 Learning: 2025-07-18T16:46:07.446Z
Learnt from: CR
PR: nuxt/nuxt#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T16:46:07.446Z
Learning: Applies to **/*.{test,spec}.{ts,tsx,js,jsx} : Write unit tests for core functionality using `vitest`
Applied to files:
test/nuxt/nuxt-time.test.ts
📚 Learning: 2025-07-18T16:46:07.446Z
Learnt from: CR
PR: nuxt/nuxt#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T16:46:07.446Z
Learning: Applies to **/e2e/**/*.{ts,js} : Write end-to-end tests using Playwright and `nuxt/test-utils`
Applied to files:
test/nuxt/nuxt-time.test.ts
🧬 Code graph analysis (2)
packages/vite/src/utils/warmup.ts (1)
packages/nuxt/src/app/components/test-component-wrapper.ts (1)
url(8-26)
packages/nuxt/src/core/nuxt.ts (1)
packages/kit/src/context.ts (1)
nuxtCtx(9-9)
⏰ 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: build
🔇 Additional comments (16)
packages/ui-templates/uno.config.ts (1)
1-1: Ensure UnoCSS is v66.0.0 or later for presetWind3
No changes to the import are required, aspresetWind3has been included inunocsssince v66.0.0.packages/vite/src/runtime/vite-node.mjs (1)
18-18: Expanded eslint-disable is fine and remains scoped to a single lineKeeps the workaround local while you migrate off
process.serverin future.packages/nuxt/test/pages.test.ts (1)
44-44: LGTM: non-capturing group is correct hereSemantics unchanged; avoids creating an unnecessary capture.
packages/nuxt/src/core/builder.ts (1)
29-32: LGTM: use RegExp.test for boolean checksThis removes unnecessary array allocation from String.match while preserving behaviour.
Also applies to: 33-36
packages/kit/src/context.ts (1)
32-33: LGTM: targeted deprecation suppressionsLocalised disables keep noise down until unctx exposes the replacement.
Also applies to: 53-55
packages/kit/src/internal/esm.ts (1)
36-37: Scoped deprecation suppressions look goodLint disables are narrowly scoped and do not alter runtime behaviour. No further action required.
Also applies to: 77-79, 86-87
packages/nuxt/src/head/runtime/composables.ts (1)
55-56: Deprecation suppressions are appropriateThe added
no-deprecatedsuppressions are minimal and local to the deprecated paths. Safe change.Also applies to: 64-66, 73-75
packages/nuxt/src/core/runtime/nitro/handlers/island.ts (1)
17-17: Regex non-capturing change is correctSwitching to a non-capturing group is semantically identical and avoids unnecessary captures.
packages/vite/src/dirs.ts (1)
5-5: LGTMSwitching to
RegExp.testwith a non-capturing group is clearer and avoids creating match arrays.packages/ui-templates/lib/dev.ts (1)
31-31: LGTM: split without the global flag is equivalent.In JS, RegExp flags don’t affect String.prototype.split’s global matching. Safe change.
packages/nuxt/src/core/nuxt.ts (1)
127-135: Temporary deprecation suppressions are acceptable.The targeted eslint disables keep compatibility without runtime impact. Clear TODO for Nuxt v5.
packages/nuxt/src/pages/plugins/page-meta.ts (1)
149-153: Good: use fnNode bounds to de-dup and slice declarations correctly.Switching to resolveStart/resolveEnd avoids misidentifying/overlapping function declarations and fixes ordering checks within the same scope.
Also applies to: 281-282, 298-300, 365-370
packages/nuxt/src/components/scan.ts (1)
14-15: LGTM: remove unnecessary captures.Non-capturing groups keep semantics (mode still from [1]) and reduce regex overhead.
eslint.config.mjs (2)
8-8: TS parser import looks goodImporting
@typescript-eslint/parseris appropriate for enabling TS-aware linting.
18-19: Enabling TypeScript tooling in the Nuxt config is appropriateTurning on
features.typescript: truealigns with the rest of this PR.test/nuxt/nuxt-time.test.ts (1)
106-112: Good move to vi.spyOn for DOM selectionSwitching to
vi.spyOn(document, 'querySelectorAll')keeps the original API surface and is cleaner than monkey-patching.
CodSpeed Performance ReportMerging #33093 will not alter performanceComparing Summary
|
1 participant
🔗 Linked issue
📚 Description
this adds typescript config and prevents use of deprecated methods (or explicitly allows them internally)
it also fixes some issues of regexp performance.