perf(nuxt): skip running lazy hydration transform with filter #33213
Conversation
|
|
@nuxt/kit
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
WalkthroughThe transform now runs only when a new template-matching regex detects likely lazy-hydration usage. Template parsing precedes script analysis; scope tracking is introduced via ScopeTracker and parseAndWalk to avoid rewriting in-scope declarations. MagicString is initialised after template parsing. The transformation iterates template AST nodes, detects hydration strategies from attributes and auto-imported components, and rewrites tags to Lazy with precise location updates for both opening and closing tags. If a strategy exists but the element is not a Lazy component, a dev/test warning is logged and the node is skipped. Error handling remains, with change detection moved inside the main try. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/nuxt/src/components/plugins/lazy-hydration-transform.ts (2)
91-100: Supportv-bind:prefix for strategy propsTemplates may use
v-bind:hydrate-on-idle. Only:is handled now.- for (const attr in node.attributes) { - const isDynamic = attr.startsWith(':') - const prop = camelCase(isDynamic ? attr.slice(1) : attr) + for (const attr in node.attributes) { + const isDynamic = attr.startsWith(':') || attr.startsWith('v-bind:') + const raw = isDynamic ? attr.replace(/^v-bind:|^:/, '') : attr + const prop = camelCase(raw)
116-123: Guard against failed tag matches to prevent runtime errors
startingChunk!assumes a match; nested same-named tags or unusual whitespace could break it.- const { 0: startingChunk, index: startingPoint = 0 } = chunk.match(new RegExp(`<${node.name}[^>]*>`)) || {} - s.overwrite(startingPoint + chunkOffset, startingPoint + chunkOffset + startingChunk!.length, startingChunk!.replace(node.name, newName)) + const mOpen = chunk.match(new RegExp(`<${node.name}[^>]*>`)) + if (!mOpen) { return } + const { 0: startingChunk, index: startingPoint = 0 } = mOpen + (s ??= new MagicString(code)).overwrite( + startingPoint + chunkOffset, + startingPoint + chunkOffset + startingChunk.length, + startingChunk.replace(node.name, newName) + )
🧹 Nitpick comments (2)
packages/nuxt/src/components/plugins/lazy-hydration-transform.ts (2)
71-71: Lazily instantiate MagicString to avoid unnecessary allocationsCreate
MagicStringonly when the first overwrite occurs.- const s = new MagicString(code) + let s: MagicString | undefined- s.overwrite(startingPoint + chunkOffset, startingPoint + chunkOffset + startingChunk!.length, startingChunk!.replace(node.name, newName)) + (s ??= new MagicString(code)).overwrite( + startingPoint + chunkOffset, + startingPoint + chunkOffset + startingChunk!.length, + startingChunk!.replace(node.name, newName) + )- if (endingChunk && endingPoint) { - s.overwrite(endingPoint + chunkOffset, endingPoint + chunkOffset + endingChunk.length, endingChunk.replace(node.name, newName)) + if (endingChunk && endingPoint !== undefined) { + (s ??= new MagicString(code)).overwrite( + endingPoint + chunkOffset, + endingPoint + chunkOffset + endingChunk.length, + endingChunk.replace(node.name, newName) + )- if (s.hasChanged()) { + if (s?.hasChanged()) {Also applies to: 117-118, 121-122, 125-132
103-111: Consider deduping per-file warning spamThe warning for non-lazy components with lazy props could flood logs in large templates. Gate by a Set per file.
// suggestion outside changed hunk const warned = new Set<string>() // ... const key = `${id}:${node.name}` if (!warned.has(key)) { warned.add(key) logger.warn(/* existing message */) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/nuxt/src/components/plugins/lazy-hydration-transform.ts(3 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/components/plugins/lazy-hydration-transform.ts
⏰ 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). (15)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, rspack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-off, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, js, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, js, lts/-1)
- GitHub Check: release-pkg-pr-new
- GitHub Check: test-size
- GitHub Check: typecheck (windows-latest, bundler)
- GitHub Check: test-benchmark
- GitHub Check: typecheck (ubuntu-latest, bundler)
- GitHub Check: code
🔇 Additional comments (1)
packages/nuxt/src/components/plugins/lazy-hydration-transform.ts (1)
63-69: Double-check oxc parser handling for<script setup lang="ts">Passing a
.vuefilename may prevent correct TS/JSX parsing. VerifyparseAndWalkcovers both classic and setup scripts with TS.Would you like a quick repo script to scan for
<script setup lang="ts">usage and run a small fixture through this transform?
| const scopeTracker = new ScopeTracker({ preserveExitedScopes: true }) | ||
| for (const { 0: script } of code.matchAll(SCRIPT_RE)) { | ||
| if (!script) { continue } | ||
| try { | ||
| parseAndWalk(script, id, { scopeTracker }) | ||
| } catch { /* ignore */ } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Scope-collision check likely misses common casings
You compare node.name directly against declarations; Vue templates may use kebab-, Pascal- or camel-case. Normalise before checking to avoid false negatives.
- if (scopeTracker.getDeclaration(node.name)) {
- return
- }
+ const scopeNames = new Set([
+ node.name,
+ pascalCase(node.name.replace(/^(?:Lazy|lazy-)/, '')),
+ camelCase(node.name),
+ camelCase(pascalCase(node.name.replace(/^(?:Lazy|lazy-)/, ''))),
+ ])
+ for (const n of scopeNames) {
+ if (scopeTracker.getDeclaration(n)) { return }
+ }Also applies to: 79-81
🤖 Prompt for AI Agents
In packages/nuxt/src/components/plugins/lazy-hydration-transform.ts around lines
63-69, the scope-collision check compares node.name directly to declarations and
misses matches when casing or kebab/camel/Pascal variants are used; normalize
both sides before comparison (e.g., create a canonical form by lowercasing and
comparing both raw and hyphen-stripped/normalized variants or by converting
kebab-case to camelCase and also lowercasing) and use that canonical form in the
equality checks so kebab-, Pascal- and camel-case variants match; apply the same
normalization fix to the similar check at lines 79-81.
CodSpeed Performance ReportMerging #33213 will not alter performanceComparing Summary
|
🔗 Linked issue
📚 Description
previously we were always scope-tracking every
<script>block. this changes this to do so only if there's a template with lazy hydration attributes present.I also move magic string creation + scope tracking a bit later to save some work if there's an error parsing the
<template>