fix(nuxt): improve page meta extraction + performance - #35869
Merged
Conversation
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/nuxt/test/page-metadata.test.ts (1)
1242-1322: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider covering the
dynamicclassification in the agreement suite.The suite covers
extract,reshape,runtimeand the non-object-literal case, but notdynamic— an extraction key whose value is not statically serialisable (e.g.definePageMeta({ name: someVar })). That is the case where the transform must keep the property whilegetRouteMetamarksnamedynamic, so the two sides drifting there would be silent.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/nuxt/test/page-metadata.test.ts` around lines 1242 - 1322, Extend the “agreement with extracted route meta” suite with a test for a statically non-serialisable extracted value, such as definePageMeta({ name: someVar }). Assert macroModule keeps the property and getRouteMeta marks name as dynamic, covering the dynamic classification alongside the existing extract, reshape, runtime, and non-object-literal cases.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/nuxt/src/pages/plugins/page-meta.ts`:
- Around line 262-282: The reshape handling around
classification.node.properties currently omits the entire layout property after
mapping only name and props, silently dropping unknown sub-properties. Before
calling omitProp, detect any unrecognised or unmappable layout sub-property and
bail out of the reshape handling so the original layout value is preserved; keep
the existing mappings for name and props unchanged.
In `@packages/nuxt/src/pages/utils.ts`:
- Around line 385-398: Update the duplicate-key handling in the classification
loop around classifyPageMetaProperty so classified uses last-wins semantics:
assign or replace the existing entry for each extract or dynamic key instead of
retaining the first occurrence. Keep the existing runtime-property handling
unchanged and align the result with JavaScript object evaluation and
isSerializable.
---
Nitpick comments:
In `@packages/nuxt/test/page-metadata.test.ts`:
- Around line 1242-1322: Extend the “agreement with extracted route meta” suite
with a test for a statically non-serialisable extracted value, such as
definePageMeta({ name: someVar }). Assert macroModule keeps the property and
getRouteMeta marks name as dynamic, covering the dynamic classification
alongside the existing extract, reshape, runtime, and non-object-literal cases.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 166fa4f1-7541-4f8e-b2db-a93923b7bf2b
📒 Files selected for processing (6)
packages/kit/src/diagnostics/pages.tspackages/nuxt/src/pages/module.tspackages/nuxt/src/pages/plugins/page-meta.tspackages/nuxt/src/pages/utils.tspackages/nuxt/test/is-serializable.test.tspackages/nuxt/test/page-metadata.test.ts
Comment on lines
+262
to
+282
| if (classification.kind !== 'reshape') { | ||
| continue | ||
| } | ||
|
|
||
| for (const layoutProp of classification.node.properties) { | ||
| if (layoutProp.type !== 'Property' || layoutProp.key.type !== 'Identifier') { | ||
| continue | ||
| } | ||
| } else if (prop.key.name === 'layout' && prop.value.type === 'ObjectExpression') { | ||
| for (const layoutProp of prop.value.properties) { | ||
| if (layoutProp.type !== 'Property' || layoutProp.key.type !== 'Identifier') { | ||
| continue | ||
| } | ||
| if (layoutProp.key.name === 'name') { | ||
| m.appendLeft( | ||
| prop.start - meta.start, | ||
| `layout: ${code.slice(layoutProp.value.start, layoutProp.value.end)},\n`, | ||
| ) | ||
| } else if (layoutProp.key.name === 'props') { | ||
| m.appendLeft( | ||
| prop.start - meta.start, | ||
| `layoutProps: ${code.slice(layoutProp.value.start, layoutProp.value.end)},\n`, | ||
| ) | ||
| } | ||
| if (layoutProp.key.name === 'name') { | ||
| m.appendLeft( | ||
| prop.start - meta.start, | ||
| `layout: ${code.slice(layoutProp.value.start, layoutProp.value.end)},\n`, | ||
| ) | ||
| } else if (layoutProp.key.name === 'props') { | ||
| m.appendLeft( | ||
| prop.start - meta.start, | ||
| `layoutProps: ${code.slice(layoutProp.value.start, layoutProp.value.end)},\n`, | ||
| ) | ||
| } | ||
| omitProp(prop, i) | ||
| } | ||
| omitProp(prop, i) |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Look for existing coverage/handling of non name/props keys in an object `layout`
rg -nP -C4 'layoutProps' --type=tsRepository: nuxt/nuxt
Length of output: 147
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="packages/nuxt/src/pages/plugins/page-meta.ts"
if [ -f "$file" ]; then
echo "=== file exists and line count ==="
wc -l "$file"
echo "=== outline ==="
ast-grep outline "$file" --match pageMeta | sed -n '1,120p' || true
echo "=== relevant lines 220-310 ==="
sed -n '220,310p' "$file" | nl -ba -v220
echo "=== omitProp definitions/usages ==="
rg -n "function omitProp|const omitProp|omitProp\\(" "$file" -C 5
else
echo "missing $file"
fi
echo "=== git diff stat/name-only ==="
git diff --stat -- packages/nuxt/src/pages/plugins/page-meta.ts || true
git diff -- packages/nuxt/src/pages/plugins/page-meta.ts | sed -n '1,220p' || trueRepository: nuxt/nuxt
Length of output: 378
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="packages/nuxt/src/pages/plugins/page-meta.ts"
echo "=== relevant lines 220-310 ==="
sed -n '220,310p' "$file"
echo "=== omitProp definitions/usages ==="
grep -n "omitProp\|function omitProp\|const omitProp" "$file" -C 5 || true
echo "=== layout reshape handling around properties ==="
grep -n "layoutProp\|reshape\|classification\\.node\\.properties\|layoutProps" "$file" -C 3 || true
echo "=== git diff stat/name-only ==="
git diff --stat -- "$file" || true
git diff -- "$file" | sed -n '1,260p' || trueRepository: nuxt/nuxt
Length of output: 6661
🏁 Script executed:
#!/bin/bash
set -euo pipefail
file="packages/nuxt/src/pages/plugins/page-meta.ts"
echo "=== classifyPageMetaProperty definition/usages ==="
grep -n "classifyPageMetaProperty\|reshape" "$file" -C 8 || true
sed -n '1,120p' "$file"
echo "=== repository references for definePageMeta layout object keys / layoutProps ==="
rg -n "definePageMeta|definePageMeta\\(|layoutProps|layoutProps:" . --glob '!node_modules/**' --glob '!dist/**' --glob '!build/**' | head -200 || trueRepository: nuxt/nuxt
Length of output: 31147
Preserve unknown layout sub-properties instead of dropping them.
The loop only maps identifier-keyed name and props; any other property in layout: { ... } is ignored before omitProp(prop, i) removes the whole property. Return from the reshape handler for unrecognised properties, or at least bail out when any sub-property cannot be mapped so the value is not silently dropped.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/nuxt/src/pages/plugins/page-meta.ts` around lines 262 - 282, The
reshape handling around classification.node.properties currently omits the
entire layout property after mapping only name and props, silently dropping
unknown sub-properties. Before calling omitProp, detect any unrecognised or
unmappable layout sub-property and bail out of the reshape handling so the
original layout value is preserved; keep the existing mappings for name and
props unchanged.
Merging this PR will not alter performance
Comparing 
Footnotes
|
danielroe
added a commit
that referenced
this pull request
Jul 30, 2026
Merged
danielroe
added a commit
that referenced
this pull request
Jul 30, 2026
danielroe
added a commit
that referenced
this pull request
Jul 30, 2026
This was referenced Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Linked issue
📚 Description
I started work on this originally as a performance improvement - to prevent having to pull in every single page (via macro transform) to render any page (the imports were always being rendered in
#build/routes.mjs)along the way, I discovered some correctness issues (for example, divergence between our static analysis and plugin transform), as well as some opportunities to give users better errors (e.g. detecting a
definePageMetamacro inside a conditional, which can only indicate they have misunderstood the concept