Skip to content

fix: always strip search params from prerenders #81730

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

Open
wants to merge 1 commit into
base: canary
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions packages/next/src/build/templates/app-page.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ export const __next_app__ = {

import * as entryBase from '../../server/app-render/entry-base' with { 'turbopack-transition': 'next-server-utility' }
import { RedirectStatusCode } from '../../client/components/redirect-status-code'
import { stripSearchParamsFromReqUrl } from '../../lib/url'

export * from '../../server/app-render/entry-base' with { 'turbopack-transition': 'next-server-utility' }

Expand Down Expand Up @@ -424,8 +425,19 @@ export async function handler(
*/
fallbackRouteParams: FallbackRouteParams | null
}): Promise<ResponseCacheEntry> => {
const isRevalidate = isSSG && !postponed && !isDynamicRSCRequest
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm not entirely sure about the condition here, someone who knows this area better should double-check that this doesn't include any dynamic requests (where we should definitely NOT strip search params)

let queryForRender = query
if (isRevalidate) {
// search params should never never be included in a prerender.
// (this can happen if we're prerendering a page at runtime because of a request that included search params)
queryForRender = {}
req.url =
req.url !== undefined
? stripSearchParamsFromReqUrl(req.url)
: undefined
}
const context: AppPageRouteHandlerContext = {
query,
query: queryForRender,
params,
page: normalizedSrcPage,
sharedContext: {
Expand Down Expand Up @@ -461,7 +473,7 @@ export async function handler(

dir: routeModule.projectDir,
isDraftMode,
isRevalidate: isSSG && !postponed && !isDynamicRSCRequest,
isRevalidate,
botType,
isOnDemandRevalidate,
isPossibleServerAction,
Expand Down
5 changes: 5 additions & 0 deletions packages/next/src/lib/url.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,8 @@ export function stripNextRscUnionQuery(relativeUrl: string): string {

return urlInstance.pathname + urlInstance.search
}

export function stripSearchParamsFromReqUrl(rawUrl: string): string {
const parsedUrl = parseReqUrl(rawUrl)!
return `${parsedUrl.pathname}${parsedUrl.hash}`
}
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,21 @@ export default function SearchParamsPage() {
searchParam=d_full, prefetch=true
</LinkAccordion>
</li>
<li>
with route param
<ul>
<li>
<LinkAccordion href="/search-params/target-page/one?searchParam=e_PPR">
param=one, searchParam=e_PPR
</LinkAccordion>
</li>
<li>
<LinkAccordion href="/search-params/target-page/one?searchParam=f_PPR">
param=one, searchParam=f_PPR
</LinkAccordion>
</li>
</ul>
</li>
</ul>
</>
)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { Suspense } from 'react'

async function SearchParam({ searchParams }) {
const { searchParam } = await searchParams
return `Search param: ${searchParam}`
}

async function Param({ params }) {
const { param } = await params
return `Param: ${param}`
}

export default async function Target({ params, searchParams }) {
return (
<>
<Suspense fallback="Loading...">
<div id="target-page-with-param">
<Param params={params} />
</div>
</Suspense>
<Suspense fallback="Loading...">
<div id="target-page-with-search-param">
<SearchParam searchParams={searchParams} />
</div>
</Suspense>
</>
)
}
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,68 @@ describe('segment cache (search params)', () => {
expect(await result.innerText()).toBe('Search param: c_PPR')
})

it('when prefetching a lazily generated ISR PPR page, does not include search params in the response', async () => {
let act: ReturnType<typeof createRouterAct>
const browser = await next.browser('/search-params', {
beforePageLoad(page) {
act = createRouterAct(page)
},
})

// Prefetch a page with param 'one' and search param `e_PPR`.
const revealA = await browser.elementByCss(
'input[data-link-accordion="/search-params/target-page/one?searchParam=e_PPR"]'
)
console.log('revealing link (e_PPR)')
await act(
async () => {
await revealA.click()
},
// The response will include a shell of the page, but nothing that is
// based on the search param.
{
includes: 'e_PPR',
block: 'reject',
}
)

console.log('revealing link (f_PPR)')
// Prefetch the same page but with the search param changed to `f_PPR`.
const revealC = await browser.elementByCss(
'input[data-link-accordion="/search-params/target-page/one?searchParam=f_PPR"]'
)
await act(
async () => {
await revealC.click()
},
// This should not issue a new request for the page segment, because
// search params are not included in the the PPR shell. So we can reuse
// the shell we fetched for `searchParam=a`.
{ includes: 'target-page-with-search-param', block: 'reject' }
)

console.log('navigating to link (f_PPR)')
// Navigate to one of the links.
const linkC = await browser.elementByCss(
'a[href="/search-params/target-page/one?searchParam=f_PPR"]'
)
await act(
async () => {
await linkC.click()
},
// The search param streams in on navigation
{
includes: 'Search param: f_PPR',
}
)
expect(await browser.elementById('target-page-with-param').text()).toBe(
'Param: one'
)
expect(
await browser.elementById('target-page-with-search-param').text()
).toBe('Search param: f_PPR')
})

it('when fetching without PPR (e.g. prefetch={true}), includes the search params in the cache key', async () => {
let act: ReturnType<typeof createRouterAct>
const browser = await next.browser('/search-params', {
Expand Down
Loading