-
Notifications
You must be signed in to change notification settings - Fork 337
fix(pages): preserve app props for GSSP requests #1996
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
Changes from 11 commits
5382aff
2e9304f
17336f7
f238575
58ed1ea
fb8d47f
deb0773
71fe549
4a650fe
06d260f
8368adc
197817c
40b66d0
8ef56bb
c1107b0
cbdcdf6
4b62a76
3720ad5
25556de
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 |
|---|---|---|
|
|
@@ -33,6 +33,7 @@ import { pickRootParams, setRootParams, type RootParams } from "vinext/shims/roo | |
| import { createRequestContext, runWithRequestContext } from "vinext/shims/unified-request-context"; | ||
| import { flattenErrorCauses } from "../utils/error-cause.js"; | ||
| import { hasBasePath } from "../utils/base-path.js"; | ||
| import { mergeRewriteQuery } from "../utils/query.js"; | ||
| import { applyAppMiddleware, type AppMiddlewareContext } from "./app-middleware.js"; | ||
| import { mergeMiddlewareResponseHeaders } from "./app-page-response.js"; | ||
| import { handleAppPrerenderEndpoint } from "./app-prerender-endpoints.js"; | ||
|
|
@@ -353,6 +354,20 @@ async function applyRewrite( | |
| return rewritten; | ||
| } | ||
|
|
||
| function applyInternalRewriteDestination(rewritten: string, url: URL): string { | ||
| const merged = mergeRewriteQuery(`${url.pathname}${url.search}`, rewritten); | ||
| const hashIndex = merged.indexOf("#"); | ||
| const beforeHash = hashIndex === -1 ? merged : merged.slice(0, hashIndex); | ||
| const queryIndex = beforeHash.indexOf("?"); | ||
| if (queryIndex === -1) { | ||
| url.search = ""; | ||
| return beforeHash; | ||
| } | ||
|
|
||
| url.search = beforeHash.slice(queryIndex); | ||
| return beforeHash.slice(0, queryIndex); | ||
| } | ||
|
|
||
| function applyConfigHeadersToMiddlewareRedirect( | ||
| response: Response, | ||
| options: { | ||
|
|
@@ -580,7 +595,7 @@ async function handleAppRscRequest<TRoute extends AppRscHandlerRoute>( | |
| matchPathname(cleanPathname), | ||
| ); | ||
| if (beforeFilesRewrite instanceof Response) return beforeFilesRewrite; | ||
| if (beforeFilesRewrite) cleanPathname = beforeFilesRewrite; | ||
| if (beforeFilesRewrite) cleanPathname = applyInternalRewriteDestination(beforeFilesRewrite, url); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New App Router behavior with no App Router test: routing |
||
|
|
||
| if (isImageOptimizationPath(cleanPathname)) { | ||
| const imageRedirect = resolveDevImageRedirect( | ||
|
|
@@ -701,7 +716,7 @@ async function handleAppRscRequest<TRoute extends AppRscHandlerRoute>( | |
| ); | ||
| if (afterFilesRewrite instanceof Response) return afterFilesRewrite; | ||
| if (afterFilesRewrite) { | ||
| cleanPathname = afterFilesRewrite; | ||
| cleanPathname = applyInternalRewriteDestination(afterFilesRewrite, url); | ||
| match = options.matchRoute(cleanPathname); | ||
| } | ||
| } | ||
|
|
@@ -720,7 +735,7 @@ async function handleAppRscRequest<TRoute extends AppRscHandlerRoute>( | |
| ); | ||
| if (fallbackRewrite instanceof Response) return fallbackRewrite; | ||
| if (fallbackRewrite) { | ||
| cleanPathname = fallbackRewrite; | ||
| cleanPathname = applyInternalRewriteDestination(fallbackRewrite, url); | ||
| match = options.matchRoute(cleanPathname); | ||
| } | ||
| } | ||
|
|
||
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.
New App Router behavior with no App Router test. This PR routes the App Router's
beforeFiles/afterFiles/fallbackrewrites through the newapplyInternalRewriteDestination, which (viaconfig-matchers.ts) appends unused source params to the destination query and merges them intourl.search. That's a real cross-cutting change to App Router request handling — e.g. a config rewrite{ source: "/blog-:param", destination: "/blog/post-3" }now exposes?param=...to the App Router page'ssearchParams, andurl.searchmutation feeds downstream searchParams.The new tests cover this for the Pages Router (
pages-request-pipeline.test.ts,pages-router.test.ts) and the matcher itself (shims.test.ts), but there is no App Router test asserting that a rewrite's unused source params reachsearchParams(or that the original query/_rscsurvives).app-router-next-config-dev.test.tsalready has a rewrite fixture harness — please add a case there so this App Router code path is covered and protected against regression.