feat(nuxt): allow updating props with setPageLayout
#33805
Conversation
|
|
@nuxt/kit
@nuxt/nitro-server
nuxt
@nuxt/rspack-builder
@nuxt/schema
@nuxt/vite-builder
@nuxt/webpack-builder
commit: |
CodSpeed Performance ReportMerging #33805 will not alter performanceComparing Summary
|
|
julien, you are the 🐐🔥 |
This reverts commit 714fcd8.
|
@danielroe seems like there's an issue with twoslash. I need to take a look but this is probably a known bug. Can we remove temporarily twoslash from layouts.md ? |
|
I don't think it's a twoslash bug - it's just that twoslash is running in the context of the root normally when something like this happens, I assume it's probably showing us a type error that would occur in an end user's project |
|
Had to update twoslash as dependency |
WalkthroughAdds support for passing serialisable layout props through setPageLayout and propagating them via route.meta.layoutProps and the NuxtLayout provider. Exposes a NuxtLayouts type and MakeSerializableObject utilities in public typings, updates module-generated typings to include per-layout prop interfaces, merges route.meta.layoutProps into layoutProps at render time, and preserves payload hydration of layout props. Tests and fixtures updated to cover layout props. Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 0
🧹 Nitpick comments (1)
docs/2.directory-structure/1.app/1.layouts.md (1)
69-81: Documentation correctly demonstrates type augmentation.The module augmentation examples appropriately show how to declare custom layout types. The pattern using
declare module 'nuxt/app'withinterface NuxtLayoutsis correct and consistent.Optional enhancement: Consider adding a more advanced example that demonstrates augmenting with actual prop types rather than
unknown:declare module 'nuxt/app' { interface NuxtLayouts { 'custom': { foo?: string, bar?: number } } }This would help users understand how to define layout-specific prop types for better type safety when calling
setPageLayout.Also applies to: 124-138
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!pnpm-lock.yaml
📒 Files selected for processing (15)
docs/2.directory-structure/1.app/1.layouts.md(2 hunks)packages/nuxt/src/app/components/nuxt-layout.ts(1 hunks)packages/nuxt/src/app/composables/router.ts(3 hunks)packages/nuxt/src/app/index.ts(1 hunks)packages/nuxt/src/app/plugins/router.ts(1 hunks)packages/nuxt/src/app/types.ts(1 hunks)packages/nuxt/src/pages/module.ts(2 hunks)packages/nuxt/src/pages/runtime/composables.ts(2 hunks)packages/nuxt/src/pages/runtime/index.ts(1 hunks)test/basic.test.ts(1 hunks)test/bundle.test.ts(1 hunks)test/fixtures/basic-types/app/app-types.ts(1 hunks)test/fixtures/basic-types/app/layouts/with-props.vue(1 hunks)test/fixtures/basic/app/layouts/custom.vue(1 hunks)test/fixtures/basic/app/middleware/sets-layout.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx,vue}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Follow standard TypeScript conventions and best practices
Files:
packages/nuxt/src/app/index.tspackages/nuxt/src/app/types.tstest/fixtures/basic/app/layouts/custom.vuetest/fixtures/basic-types/app/layouts/with-props.vuepackages/nuxt/src/app/components/nuxt-layout.tspackages/nuxt/src/app/plugins/router.tspackages/nuxt/src/pages/runtime/index.tstest/bundle.test.tspackages/nuxt/src/pages/runtime/composables.tstest/fixtures/basic/app/middleware/sets-layout.tspackages/nuxt/src/pages/module.tspackages/nuxt/src/app/composables/router.tstest/fixtures/basic-types/app/app-types.tstest/basic.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/src/app/index.tspackages/nuxt/src/app/types.tstest/fixtures/basic/app/layouts/custom.vuetest/fixtures/basic-types/app/layouts/with-props.vuepackages/nuxt/src/app/components/nuxt-layout.tspackages/nuxt/src/app/plugins/router.tspackages/nuxt/src/pages/runtime/index.tstest/bundle.test.tspackages/nuxt/src/pages/runtime/composables.tstest/fixtures/basic/app/middleware/sets-layout.tspackages/nuxt/src/pages/module.tspackages/nuxt/src/app/composables/router.tstest/fixtures/basic-types/app/app-types.tstest/basic.test.ts
**/*.vue
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use
<script setup lang="ts">and the composition API when creating Vue components
Files:
test/fixtures/basic/app/layouts/custom.vuetest/fixtures/basic-types/app/layouts/with-props.vue
**/*.{test,spec}.{ts,tsx,js}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Write unit tests for core functionality using
vitest
Files:
test/bundle.test.tstest/basic.test.ts
🧠 Learnings (5)
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julien
Repo: nuxt/nuxt PR: 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/app/index.tspackages/nuxt/src/app/types.tspackages/nuxt/src/app/components/nuxt-layout.tspackages/nuxt/src/pages/runtime/index.tspackages/nuxt/src/pages/runtime/composables.tspackages/nuxt/src/pages/module.tspackages/nuxt/src/app/composables/router.tsdocs/2.directory-structure/1.app/1.layouts.md
📚 Learning: 2024-11-05T15:22:54.759Z
Learnt from: GalacticHypernova
Repo: nuxt/nuxt PR: 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/app/index.tspackages/nuxt/src/pages/module.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/*.vue : Use `<script setup lang="ts">` and the composition API when creating Vue components
Applied to files:
test/fixtures/basic-types/app/layouts/with-props.vuedocs/2.directory-structure/1.app/1.layouts.md
📚 Learning: 2024-11-11T12:34:22.648Z
Learnt from: Tofandel
Repo: nuxt/nuxt PR: 0
File: :0-0
Timestamp: 2024-11-11T12:34:22.648Z
Learning: Ensure that AI-generated summaries accurately reflect the key changes in the PR, focusing on notable changes such as the removal of unused imports and variables starting with underscores.
Applied to files:
test/bundle.test.tstest/basic.test.ts
📚 Learning: 2025-11-25T11:42:16.132Z
Learnt from: CR
Repo: nuxt/nuxt PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-25T11:42:16.132Z
Learning: Applies to **/e2e/**/*.{ts,tsx,js} : Write end-to-end tests using Playwright and `nuxt/test-utils`
Applied to files:
test/fixtures/basic-types/app/app-types.ts
🪛 Biome (2.1.2)
packages/nuxt/src/pages/runtime/composables.ts
[error] 11-13: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
⏰ 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). (20)
- GitHub Check: test-fixtures (windows-latest, built, vite-env-api, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (windows-latest, dev, 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, built, vite-env-api, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, webpack, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, vite, async, manifest-on, js, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, built, rspack, default, 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, 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-env-api, async, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, default, manifest-on, json, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-on, js, lts/-1)
- GitHub Check: test-fixtures (ubuntu-latest, dev, vite, async, manifest-off, json, lts/-1)
- GitHub Check: release-pkg-pr-new
- GitHub Check: test-benchmark
- GitHub Check: test-size
- GitHub Check: typecheck (windows-latest, bundler)
- GitHub Check: typecheck (ubuntu-latest, bundler)
- GitHub Check: code
🔇 Additional comments (15)
test/bundle.test.ts (1)
98-98: LGTM! Expected bundle size increase.The 1 KB increase in the server bundle size is minimal and expected given the addition of layoutProps functionality.
test/fixtures/basic-types/app/app-types.ts (1)
302-311: LGTM! Comprehensive type-checking coverage.The test cases effectively verify that
setPageLayoutrecognises named layouts and their associated props, including both valid usage patterns and expected type errors for invalid layouts and incorrect prop types.test/basic.test.ts (1)
1408-1408: LGTM! Good integration test coverage.The assertion verifies end-to-end functionality by confirming that middleware-provided layout props render correctly in the DOM, complementing the changes in the middleware fixture.
test/fixtures/basic/app/middleware/sets-layout.ts (1)
3-5: LGTM! Clear demonstration of the new API.The middleware demonstrates the new
setPageLayoutsignature with layout props, which is then verified in the integration tests to ensure the props propagate correctly to the layout component.packages/nuxt/src/pages/runtime/index.ts (1)
2-2: LGTM! Appropriate public API expansion.Adding
NuxtLayoutsto the public type exports enables users to access layout typing, which is necessary for the type-safe layout props feature.packages/nuxt/src/app/plugins/router.ts (1)
241-247: LGTM! Correct hydration flow for layout props.The implementation properly handles layoutProps during hydration by:
- Reading
initialLayoutPropsfrom the server payload (line 241)- Restoring it to
to.meta.layoutPropsduring hydration (line 247)This ensures layout props set on the server are correctly transferred to the client and preserved during the initial navigation.
packages/nuxt/src/app/components/nuxt-layout.ts (1)
99-99: LGTM! Props merging enables route-driven layout props.The
LayoutProvidernow merges three sources of props:
context.attrs(component-level props)route.meta.layoutProps(route/middleware-driven props){ ref: layoutRef }(internal ref)This merge order allows route-driven props from
setPageLayoutto override component-level props, which aligns with the feature's intended behaviour.packages/nuxt/src/app/types.ts (1)
3-3: LGTM! Consistent public API expansion.Adding
NuxtLayoutsto the type exports maintains consistency with the public API surface expansion seen inpackages/nuxt/src/pages/runtime/index.tsand enables users to access layout typing.test/fixtures/basic/app/layouts/custom.vue (1)
15-15: LGTM! Layout props integration looks good.The optional
fooprop is correctly defined and rendered, demonstrating the new layout props feature effectively. The implementation follows Vue composition API best practices.Also applies to: 22-24
packages/nuxt/src/app/index.ts (1)
14-14: LGTM! Public API surface appropriately extended.The export of
NuxtLayoutscorrectly expands the public type surface to support the new layout props feature.packages/nuxt/src/pages/runtime/composables.ts (2)
9-13: LGTM! Empty interface is intentional for module augmentation.The empty
NuxtLayoutsinterface is designed to be augmented at build time with layout-specific typings. This is a standard pattern for runtime-extended types in Nuxt.Note: The Biome static analysis warning can be safely ignored—this is not a code smell but an intentional design pattern for TypeScript module augmentation.
56-62: LGTM! Router meta augmentation correctly preserves inheritance.The
RouteMetaaugmentation properly extendsUnwrapRef<PageMeta>whilst adding the internallayoutPropsfield. The@internalannotation appropriately indicates this is not part of the public API.test/fixtures/basic-types/app/layouts/with-props.vue (1)
1-11: LGTM! Clean test fixture for layout props.The component correctly demonstrates typed layout props with a required
aPropparameter. The implementation follows Vue composition API best practices with<script setup lang="ts">.packages/nuxt/src/app/composables/router.ts (1)
269-299: LGTM! Layout props propagation is well-structured across all contexts.The enhanced
setPageLayoutcorrectly:
- Uses generics to provide type-safe props based on the layout name
- Propagates
layoutPropsthrough server payload (line 281), middleware (line 290), and client route meta (line 297)- Guards the
useRoute()call (line 295) with!inMiddlewareto avoid the warnings mentioned in lines 22-26- Maintains backwards compatibility with the optional
propsparameterThe conditional type signature elegantly handles both typed layouts (when
NuxtLayoutsis augmented) and fallback tostringwhen no layouts are defined.packages/nuxt/src/pages/module.ts (1)
5-5: LGTM! Layout typings generation is well-implemented.The code correctly:
- Imports
genSafeVariableNameto ensure valid variable names for layout components- Generates imports for all layout files with safe identifiers
- Builds the
NuxtLayoutsinterface mapping each layout name toInstanceType<typeof Layout>['$props'], which properly extracts Vue component prop types- Updates
LayoutKeyto derive fromNuxtLayoutswith a string fallback when no layouts existThe generated typings will provide excellent IntelliSense and type safety for
setPageLayoutcalls.Also applies to: 622-643
|
weird - the only thing updated in the lockfile was |
|
hmmm let me try reverting it. Last time i had to run install for twoslash again. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/nuxt/src/app/composables/router.ts (3)
270-273: Expand the JSDoc to document the newpropsparameter.The JSDoc comment doesn't mention the new optional
propsparameter or explain how layout props are propagated. Consider adding:
- A description of the
propsparameter- The serialization constraints on props
- An example showing the new functionality
🔎 Proposed enhancement to JSDoc
/** * Sets the layout for the current page. + * @param layout - The name of the layout to use. + * @param props - Optional props to pass to the layout component. Props must be serializable. * @since 3.0.0 */
274-274: Consider simplifying the conditional type for thepropsparameter.The condition
typeof layout extends Layoutin the props parameter type appears redundant. Sincelayouthas typeLayoutwhen the generic is known,typeof layoutwould always beLayoutat the type level, making this condition always true. Consider simplifying to:props?: unknown extends Layout ? never : MakeSerializableObject<NuxtLayouts[Layout]>This more directly expresses the intent: props are only accepted when Layout is a known key of NuxtLayouts.
281-281: Consider adding a development-mode check to validate layout props are serialisable.The
propsparameter uses theMakeSerializableObjecttype constraint to enforce serializability at compile-time, but this provides no runtime validation. Developers can bypass the type system (e.g., withas any), and non-serialisable values would cause serialization errors during server-side rendering. Adding a console warning in development mode when non-serialisable values are detected would help prevent hydration mismatches, consistent with the existing hydration warnings in this function.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/2.directory-structure/1.app/1.layouts.mdpackages/nuxt/src/app/components/nuxt-layout.tspackages/nuxt/src/app/composables/router.tspackages/nuxt/src/app/plugins/router.tspackages/nuxt/src/pages/module.tspackages/nuxt/src/pages/runtime/composables.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/nuxt/src/pages/module.ts
- packages/nuxt/src/app/components/nuxt-layout.ts
- docs/2.directory-structure/1.app/1.layouts.md
- packages/nuxt/src/app/plugins/router.ts
🧰 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/app/composables/router.tspackages/nuxt/src/pages/runtime/composables.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/app/composables/router.tspackages/nuxt/src/pages/runtime/composables.ts
🧠 Learnings (1)
📚 Learning: 2024-12-12T12:36:34.871Z
Learnt from: huang-julien
Repo: nuxt/nuxt PR: 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/app/composables/router.tspackages/nuxt/src/pages/runtime/composables.ts
🧬 Code graph analysis (1)
packages/nuxt/src/pages/runtime/composables.ts (1)
packages/nuxt/src/pages/runtime/utils.ts (1)
SerializableValue(8-8)
🪛 Biome (2.1.2)
packages/nuxt/src/pages/runtime/composables.ts
[error] 12-14: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
🔇 Additional comments (6)
packages/nuxt/src/pages/runtime/composables.ts (3)
8-9: LGTM! Appropriate import for serializable layout props.The
SerializableValuetype import is necessary for the newlayoutPropsfeature and ensures that layout props can be properly serialised for SSR contexts.
10-14: Valid module augmentation pattern for runtime-generated types.Whilst the static analysis tool flags the empty interface, this is an intentional and valid TypeScript pattern for module augmentation. The comment clearly indicates this interface will be extended at build time with generated layout types. The
eslint-disablecomment confirms the team is aware of this pattern.
57-63: LGTM! Proper module augmentation for layout props.The
RouteMetaaugmentation correctly adds thelayoutPropsfield with an appropriate type for serialisable values. The@internalmarking is correct, as this field should be managed through the framework API (setPageLayout()) rather than manipulated directly.packages/nuxt/src/app/composables/router.ts (3)
288-292: LGTM! Consistent with existing layout handling.The props are correctly propagated via route metadata in the
beforeResolvehook, maintaining consistency with how the layout itself is set. This ensures the props are available during layout rendering.
294-298: LGTM! Props correctly set in non-middleware path.When not in middleware, the layout props are synchronously set on the route metadata alongside the layout itself. This maintains consistency with the middleware path and ensures props are available regardless of when
setPageLayoutis called.
7-7: Import sources are correctly configured and properly exported.The type imports are valid:
NuxtLayoutsis exported frompackages/nuxt/src/pages/runtime/composables.tsandMakeSerializableObjectis exported frompackages/nuxt/src/pages/runtime/utils.ts.
| defineProps<{ | ||
| foo?: string | ||
| }>() |
There was a problem hiding this comment.
Hey @huang-julien, The docs recommend using useAttrs for layout props, I didn't know that we can use defineProps in layouts too. Is this new, or am I missing something?
🔗 Linked issue
close #31204
📚 Description
This PR adds a second parameter to
setPageLayoutto add or update props to the layout