fix(app-router): scope layout params and layout error boundaries#938
Conversation
Layouts received full route params, layout generateMetadata failures were swallowed, and layout-thrown forbidden()/unauthorized() fell back through the not-found boundary path. Those diverge from Next.js when nested layouts depend on segment params, when head generation fails before page render, or when HTTP access APIs are thrown from a layout. The generated RSC entry now delegates param slicing, head resolution, and parent access-boundary selection to typed runtime helpers. The helpers keep route-specific imports in the entry while moving the behavioral core into unit-tested modules. Tests port Next.js layout params, global-error, forbidden, and unauthorized coverage into focused helper and integration regressions.
commit: |
There was a problem hiding this comment.
Pull request overview
This PR improves App Router Next.js compatibility by scoping layout params to the segment depth (instead of passing full route params everywhere) and by routing layout metadata/access-fallback failures through the correct boundary paths. It also keeps the generated RSC entry slimmer by delegating param slicing, head resolution, and parent access-boundary selection to server helpers.
Changes:
- Add server helpers for segment-scoped params, search param collection, and unified head (metadata/viewport) resolution.
- Route layout
generateMetadata()errors through normal error boundaries; route layoutforbidden()/unauthorized()throws to the correct parent HTTP access boundary. - Update router scanning/entry generation to carry per-layout forbidden/unauthorized boundary arrays and to use the new helpers; add/extend tests and fixtures for Next.js parity.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
packages/vinext/src/server/app-page-params.ts |
New helper to slice matched route params by segment depth for layouts. |
packages/vinext/src/server/app-page-head.ts |
New helper to collect search params and resolve layout+page metadata/viewport with segment-scoped layout params. |
packages/vinext/src/server/app-page-route-wiring.tsx |
Pass segment-scoped params to each layout during element wiring. |
packages/vinext/src/server/app-page-boundary.ts |
Scope params during boundary/layout wrapping; add parent HTTP access boundary selection helper. |
packages/vinext/src/server/app-page-boundary-render.ts |
Use shared head resolution helper for fallback pages; remove duplicated head-resolution logic. |
packages/vinext/src/routing/app-router.ts |
Track forbidden/unauthorized boundary paths per layout level (aligned with layouts). |
packages/vinext/src/entries/app-rsc-entry.ts |
Keep generated entry thin by importing helpers for head resolution, param slicing, and parent access-boundary selection; serialize per-layout boundary arrays. |
packages/vinext/src/shims/metadata.tsx |
Broaden searchParams typing to support repeated query keys (`string |
tests/app-page-params.test.ts |
New unit tests for segment param scoping (including catch-all/optional catch-all behavior). |
tests/app-page-head.test.ts |
New unit tests for search param collection and scoped layout metadata vs full page metadata inputs. |
tests/app-page-route-wiring.test.ts |
Assert layout probing/wiring uses only segment-applicable params. |
tests/app-page-boundary.test.ts |
Update expectations to reflect new segment/param scoping behavior. |
tests/app-router.test.ts |
New integration assertions for layout-thrown forbidden/unauthorized selecting correct boundaries. |
tests/nextjs-compat/global-error.test.ts |
Add coverage for layout generateMetadata() errors flowing to local/global error boundaries. |
tests/fixtures/app-basic/app/nextjs-compat/* |
New fixtures for layout metadata error + layout access fallback scenarios. |
tests/__snapshots__/entry-templates.test.ts.snap |
Snapshot updates reflecting new generated entry imports/behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Layout generateViewport() failures now intentionally share the same boundary path as layout generateMetadata() failures. Copilot flagged that this behavior change needed direct coverage. Add dev and production-preview compat assertions for co-located error.tsx handling and global-error escalation, with fixtures that throw from layout generateViewport().
|
/bigbonk review |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
There was a problem hiding this comment.
Solid PR. The refactoring is well-structured: extracted helpers are small, single-purpose, and properly tested. The entry stays thin (matching the repo's architecture guidance), and the boundary arrays for forbidden/unauthorized are properly aligned with the existing not-found pattern.
The behavior changes are well-justified by the Next.js references, and the test coverage is thorough — unit tests for the new helpers, integration tests for both dev and production preview, and the fixtures are minimal and focused.
A few minor observations:
1. Metadata cascade on layout rejection (app-page-head.ts:97-103)
When metadataPromise rejects, the .then() chain on line 97 also rejects. The void accumulatedMetadata.catch(() => null) on line 103 prevents unhandled rejections. This means if layout N throws, layouts N+1..M will also have their parentForLayout reject and their own promises will be rejected too. The Promise.all on line 109 surfaces the first rejection, which is the correct one. The logic is sound.
2. Naming consistency (app-rsc-entry.ts)
forbiddens and unauthorizeds as property names read awkwardly, but consistency with the existing notFounds pattern is more important. If there's ever a rename pass, forbiddenModules/unauthorizedModules would be clearer.
3. Dot exclusion in param parsing (app-page-params.ts:15)
The !segment.includes(".") check correctly prevents matching segments with dots (e.g. route groups like (...)) after catch-all patterns are handled. This matches Next.js behavior where dynamic segments can't contain dots.
4. searchParams type widening (metadata.tsx:357)
The searchParams type change from Record<string, string> to Record<string, string | string[]> is correct — repeated query keys produce arrays in collectAppPageSearchParams, and this was a latent type mismatch.
|
The review has been submitted. Here's a summary of my findings: Approved — this is a well-structured PR that improves Next.js parity in two areas:
The implementation follows the repo's architecture guidelines well — the generated RSC entry stays thin, with real behavior extracted into typed helpers under |
| params: options.makeThenableParams( | ||
| resolveAppPageSegmentParams( | ||
| options.route.routeSegments, | ||
| layoutEntry.treePosition, | ||
| options.matchedParams, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
i think we may also need to align the params passed to the page as well, like we've done for the layout here.
There was a problem hiding this comment.
There was a problem hiding this comment.
There was a problem hiding this comment.
I checked this against Next.js and I don't think the page params should be aligned the same way as ancestor layouts.
In Next.js create-component-tree.tsx, each segment builds currentParams from parentParams plus the current segment param, then passes that accumulated object to descendants. By the time rendering reaches the page, that object is the full leaf params object.
The Next.js app-dir e2e asserts the distinction directly in test/e2e/app-dir/app/index.test.ts: for /dynamic/books/hello-world, ancestor layouts receive {}, then { category }, then { category, id }, and the page also receives { category, id }.
So the generated entry passing params to page render/probing is intentional here: params is the matched leaf route params. The layout fix is different because each ancestor layout is rendered at a shallower tree position. If we want to address optional-catchall page param normalization separately, that should be a separate parity fix with direct coverage rather than changing page params as part of this layout-scoping PR.
There was a problem hiding this comment.
I'll open an issue if you accept this
There was a problem hiding this comment.
Understood, thanks for the references
|
/bigbonk review |
There was a problem hiding this comment.
Clean PR. The extraction of head resolution, param slicing, and parent boundary selection into typed helpers under server/ follows the repo's architecture guidelines well — the generated entry stays thin and the runtime behavior is directly unit-testable.
The behavior changes (scoped layout params, metadata/viewport error propagation, status-aware boundary selection for forbidden/unauthorized) are well-justified by the Next.js references and tested at both the unit and integration level.
Two minor observations and one naming nit below. None are blocking.
| const layoutModule = options.layoutModules[index]; | ||
| if (!layoutModule) { | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Nit: when a layout module is null, this continue skips pushing to layoutMetadataPromises but the outer Promise.all at line 109 still maps over options.layoutModules for viewport resolution (line 111). That means layoutMetadataResults and layoutViewportResults have different lengths — metadata results omit null layouts, viewport results include null entries for them.
This works correctly because both arrays are filtered through isMetadata/isViewport before merging (lines 139-144), so the length mismatch doesn't cause a bug. But it's a subtle invariant that could trip someone up during future maintenance. Worth a brief comment noting the asymmetry is intentional, or alternatively, skipping null modules in the viewport .map() the same way the metadata loop does.
There was a problem hiding this comment.
Fixed in bc6ae85. resolveAppPageHead() now builds layout metadata and viewport promises from the same non-null layout pass, so the result arrays have the same shape and reuse the same segment-scoped layoutParams.
| const _asyncSearchParams = makeThenableParams( | ||
| __collectAppPageSearchParams(url.searchParams).searchParamsObject, | ||
| ); | ||
| return PageComponent({ params: _asyncLayoutParams, searchParams: _asyncSearchParams }); |
There was a problem hiding this comment.
This is the point james-elicx flagged — probePage() still passes _asyncLayoutParams (the full matched params) to the page. The author's response (citing Next.js create-component-tree.tsx where the accumulated params at the leaf are the full params) is correct. The page should receive all route params.
However, the variable name _asyncLayoutParams is now misleading since layouts get scoped params but this variable still holds full params and is used for the page. Consider renaming to something like _asyncRouteParams or _asyncPageParams to reduce confusion in a follow-up.
There was a problem hiding this comment.
Fixed in bc6ae85. Renamed _asyncLayoutParams to _asyncRouteParams so the name reflects that this is the full matched route params object used for page probing, while layout probes continue to receive scoped params.
| } | ||
|
|
||
| if (routeModules) { | ||
| for (let index = options.layoutIndex - 1; index >= 0; index--) { |
There was a problem hiding this comment.
Minor: this walks index = layoutIndex - 1 down to 0, which is correct for "find the nearest parent boundary above the throwing layout." The semantics match the old notFounds loop that was inlined in the entry. Good.
|
Review submitted. Here's a summary: Approved. This is a well-structured PR that improves Next.js parity in two areas:
The implementation follows the repo's architecture well: the generated RSC entry stays thin, with behavior extracted into small, typed helpers ( I left three non-blocking comments:
|
What this changes
App Router layouts now receive only the params that apply at their segment depth during both page tree wiring and layout probing. Layout metadata and viewport resolution use the same segment-scoped params, while page metadata still receives full route params and searchParams.
Layout
generateMetadata()andgenerateViewport()failures now propagate to the normal App Router error boundary path instead of being swallowed. Layout-thrownforbidden()andunauthorized()now select the matching parent HTTP access boundary instead of falling through the not-found-only path.The generated RSC entry stays thin: it serializes route imports and delegates param slicing, head resolution, search param collection, and parent access-boundary selection to typed server helpers.
Why
Next.js scopes layout params by walking accumulated parent params per segment, not by passing the full route params object to every layout. Relevant references:
Next.js also routes metadata and HTTP access fallback failures through the relevant error/access boundary paths:
Approach
Add small functional helpers under
server/for segment param slicing, search param collection, head resolution, and parent access-boundary selection. The RSC entry now imports those helpers and keeps the route-specific imperative work in place.The scanner now keeps forbidden and unauthorized boundary arrays aligned with layout levels, matching the existing not-found array shape.
Validation
vp test run tests/app-page-params.test.ts tests/app-page-head.test.ts tests/app-page-boundary.test.ts tests/app-page-route-wiring.test.ts tests/entry-templates.test.ts tests/routing.test.tsvp test run tests/nextjs-compat/global-error.test.tsvp test run tests/app-router.test.ts -t "thrown from a layout uses|layout generateMetadata\(\) does not receive searchParams"vp check tests/app-page-params.test.ts tests/app-page-head.test.ts tests/app-page-boundary.test.ts tests/app-page-route-wiring.test.ts tests/entry-templates.test.ts tests/routing.test.ts tests/nextjs-compat/global-error.test.ts tests/app-router.test.tsvp check tests/nextjs-compat/global-error.test.tsgit diff --checkRisks / follow-ups
Checked open PRs before publishing. Closest adjacent work is #891, #735, and #822; this PR should not overlap their behavior directly.
This PR intentionally does not address remaining App Router opportunities like unknown Server Action IDs, non-action method handling, route-handler
NextResponse.next()validation, or route-handler cookie precedence.