-
Notifications
You must be signed in to change notification settings - Fork 296
feat: flat keyed payload for App Router layout persistence #750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 27 commits
7f4f8eb
5d8525b
be33773
ca40d05
bddda39
38d33ca
8c22db3
d488978
ec008fa
5395efc
955f577
ce76239
c7a03d5
7fead69
311b10a
2b5f68c
7554b20
1014aed
5e516bd
ee2fbdd
f8e2276
c346485
d2a4d13
9b9a6d5
f4949b4
c606e62
a220f3d
9959097
7c62628
29ca8ea
a345706
b529eb9
e77d9a1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -149,9 +149,7 @@ export function generateRscEntry( | |||||||||||||||||||||||||||||||||||||||||||||
| if (route.pagePath) getImportVar(route.pagePath); | ||||||||||||||||||||||||||||||||||||||||||||||
| if (route.routePath) getImportVar(route.routePath); | ||||||||||||||||||||||||||||||||||||||||||||||
| for (const layout of route.layouts) getImportVar(layout); | ||||||||||||||||||||||||||||||||||||||||||||||
| for (const tmpl of route.templates) { | ||||||||||||||||||||||||||||||||||||||||||||||
| if (tmpl) getImportVar(tmpl); | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| for (const tmpl of route.templates) getImportVar(tmpl); | ||||||||||||||||||||||||||||||||||||||||||||||
| if (route.loadingPath) getImportVar(route.loadingPath); | ||||||||||||||||||||||||||||||||||||||||||||||
| if (route.errorPath) getImportVar(route.errorPath); | ||||||||||||||||||||||||||||||||||||||||||||||
| if (route.layoutErrorPaths) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -181,7 +179,7 @@ export function generateRscEntry( | |||||||||||||||||||||||||||||||||||||||||||||
| // Build route table as serialized JS | ||||||||||||||||||||||||||||||||||||||||||||||
| const routeEntries = routes.map((route) => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const layoutVars = route.layouts.map((l) => getImportVar(l)); | ||||||||||||||||||||||||||||||||||||||||||||||
| const templateVars = route.templates.map((t) => (t ? getImportVar(t) : "null")); | ||||||||||||||||||||||||||||||||||||||||||||||
| const templateVars = route.templates.map((t) => getImportVar(t)); | ||||||||||||||||||||||||||||||||||||||||||||||
| const notFoundVars = (route.notFoundPaths || []).map((nf) => (nf ? getImportVar(nf) : "null")); | ||||||||||||||||||||||||||||||||||||||||||||||
| const slotEntries = route.parallelSlots.map((slot) => { | ||||||||||||||||||||||||||||||||||||||||||||||
| const interceptEntries = slot.interceptingRoutes.map( | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -217,6 +215,7 @@ ${interceptEntries.join(",\n")} | |||||||||||||||||||||||||||||||||||||||||||||
| routeHandler: ${route.routePath ? getImportVar(route.routePath) : "null"}, | ||||||||||||||||||||||||||||||||||||||||||||||
| layouts: [${layoutVars.join(", ")}], | ||||||||||||||||||||||||||||||||||||||||||||||
| routeSegments: ${JSON.stringify(route.routeSegments)}, | ||||||||||||||||||||||||||||||||||||||||||||||
| templateTreePositions: ${JSON.stringify(route.templateTreePositions)}, | ||||||||||||||||||||||||||||||||||||||||||||||
| layoutTreePositions: ${JSON.stringify(route.layoutTreePositions)}, | ||||||||||||||||||||||||||||||||||||||||||||||
| templates: [${templateVars.join(", ")}], | ||||||||||||||||||||||||||||||||||||||||||||||
| errors: [${layoutErrorVars.join(", ")}], | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -385,7 +384,7 @@ import { | |||||||||||||||||||||||||||||||||||||||||||||
| renderAppPageHttpAccessFallback as __renderAppPageHttpAccessFallback, | ||||||||||||||||||||||||||||||||||||||||||||||
| } from ${JSON.stringify(appPageBoundaryRenderPath)}; | ||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||
| buildAppPageRouteElement as __buildAppPageRouteElement, | ||||||||||||||||||||||||||||||||||||||||||||||
| buildAppPageElements as __buildAppPageElements, | ||||||||||||||||||||||||||||||||||||||||||||||
| resolveAppPageChildSegments as __resolveAppPageChildSegments, | ||||||||||||||||||||||||||||||||||||||||||||||
| } from ${JSON.stringify(appPageRouteWiringPath)}; | ||||||||||||||||||||||||||||||||||||||||||||||
| import { | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -906,10 +905,21 @@ function findIntercept(pathname) { | |||||||||||||||||||||||||||||||||||||||||||||
| return null; | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| async function buildPageElement(route, params, opts, searchParams) { | ||||||||||||||||||||||||||||||||||||||||||||||
| async function buildPageElements(route, params, routePath, opts, searchParams) { | ||||||||||||||||||||||||||||||||||||||||||||||
| const PageComponent = route.page?.default; | ||||||||||||||||||||||||||||||||||||||||||||||
| if (!PageComponent) { | ||||||||||||||||||||||||||||||||||||||||||||||
| return createElement("div", null, "Page has no default export"); | ||||||||||||||||||||||||||||||||||||||||||||||
| const _noExportRouteId = "route:" + routePath; | ||||||||||||||||||||||||||||||||||||||||||||||
| let _noExportRootLayout = null; | ||||||||||||||||||||||||||||||||||||||||||||||
| if (route.layouts?.length > 0) { | ||||||||||||||||||||||||||||||||||||||||||||||
| const _tp = route.layoutTreePositions?.[0] ?? 0; | ||||||||||||||||||||||||||||||||||||||||||||||
| const _segs = route.routeSegments?.slice(0, _tp) ?? []; | ||||||||||||||||||||||||||||||||||||||||||||||
| _noExportRootLayout = _segs.length === 0 ? "/" : "/" + _segs.join("/"); | ||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
| _noExportRootLayout = _segs.length === 0 ? "/" : "/" + _segs.join("/"); | |
| // Compute the root layout tree path for this error payload using the | |
| // canonical helper to avoid drift with buildAppPageElements(). | |
| const _tp = route.layoutTreePositions?.[0] ?? 0; | |
| _noExportRootLayout = __createAppPageTreePath(route.routeSegments, _tp); |
This would require adding createAppPageTreePath to the import from app-page-route-wiring.tsx at the top of the generated entry. If there's a reason this needs to stay inline (e.g., avoiding import weight for this error path), the current approach is fine — just flagging the risk.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regression: middleware can clobber framework redirect headers.
The old code intentionally set x-action-redirect* headers after __mergeMiddlewareResponseHeaders with an explicit comment:
"Merge middleware headers first so the framework's own redirect control headers below are always authoritative and cannot be clobbered by middleware that happens to set x-action-redirect* keys."
The new code sets these headers in the Headers constructor before the merge call. Since mergeMiddlewareResponseHeaders uses target.set(key, value) for non-cookie/non-vary headers (app-page-response.ts:183), if middleware sets x-action-redirect, x-action-redirect-type, or x-action-redirect-status, those values will overwrite the framework's authoritative values.
This is a security-relevant regression — middleware could redirect server action responses to an unintended URL.
| const redirectHeaders = new Headers({ | |
| "Content-Type": "text/x-component; charset=utf-8", | |
| "Vary": "RSC, Accept", | |
| "x-action-redirect": actionRedirect.url, | |
| "x-action-redirect-type": actionRedirect.type, | |
| "x-action-redirect-status": String(actionRedirect.status), | |
| }); | |
| // Merge middleware headers first so the framework's own redirect control | |
| // headers below are always authoritative and cannot be clobbered by | |
| // middleware that happens to set x-action-redirect* keys. | |
| __mergeMiddlewareResponseHeaders(redirectHeaders, _mwCtx.headers); | |
| const redirectHeaders = new Headers({ | |
| "Content-Type": "text/x-component; charset=utf-8", | |
| "Vary": "RSC, Accept", | |
| }); | |
| // Merge middleware headers first so the framework's own redirect control | |
| // headers below are always authoritative and cannot be clobbered by | |
| // middleware that happens to set x-action-redirect* keys. | |
| __mergeMiddlewareResponseHeaders(redirectHeaders, _mwCtx.headers); | |
| redirectHeaders.set("x-action-redirect", actionRedirect.url); | |
| redirectHeaders.set("x-action-redirect-type", actionRedirect.type); | |
| redirectHeaders.set("x-action-redirect-status", String(actionRedirect.status)); |
Note: this pattern appears in the generated code for all route variants (the same regression is repeated in each snapshot branch of the entry template).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good move. _probeSearchObj and _asyncSearchParams were being computed unconditionally at line 2260 (old code) even when they were only used inside probePage(). Moving them inside the closure is both cleaner and avoids unnecessary work when probePage() is never called (e.g., the page is served from ISR cache).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
buildPageElementsno-default-export path (line 906-921) builds a minimal flat payload with__route,__rootLayout, and a single element. The__rootLayoutcomputation replicatescreateAppPageLayoutEntrieslogic inline — computing tree position fromlayoutTreePositions[0]and slicingrouteSegments.This is acceptable for now since it's the error path (no page export), but if the tree path computation logic ever changes in
createAppPageTreePath, this inline version would drift. Consider adding a comment noting the dependency: