Skip to content

Commit de37cd7

Browse files
authored
Track searchParams access through a Proxy to catch missing-key reads (vercel#93549)
Under `experimental.varyParams`, when a page reads `searchParams` from a URL without a query string, `createVaryingSearchParams` previously returned a plain object whose only own properties were getters for keys that already existed in the URL. With no keys present, that's an empty `{}` — so `Object.entries`, spread, `in` checks, and missing-key reads (e.g. `searchParams.foo` returning `undefined`) all silently no-opped without registering an access. The segment ended up keyed at the `Fallback` search slot in the segment cache, where it shadowed subsequent prefetches with non-empty search params via `Fallback` resolution, silently serving the stale empty-query cache entry on navigation. This switches the implementation to a `Proxy` with `get`, `has`, and `ownKeys` traps that record the `'?'` sentinel on every string access. Search params have no fixed schema, so any access — including missing-key reads, `in` checks, and enumeration — must register as varying. The `get` trap has one carve-out: `.then` reads are skipped when no `then` key exists on the target, since `Promise.resolve` and React Flight probe `.then` to test thenability and those probes shouldn't count as user dependencies. As a trade-off, a page that reads `searchParams.then` to compute its output and is prefetched without a `then` key won't register the access — accepted because `then` is reserved by the Promise protocol and essentially never used as a query parameter name. The added regression test prefetches the no-query URL first and asserts that a subsequent `?foo=1` prefetch still triggers a fresh request. Without the fix the lookup resolves to the empty-query cache entry through `Fallback`, no request is initiated, and the test times out waiting for one.
1 parent c06d94b commit de37cd7

3 files changed

Lines changed: 87 additions & 14 deletions

File tree

packages/next/src/server/app-render/vary-params.ts

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -301,20 +301,30 @@ export function createVaryingSearchParams(
301301
accumulator: VaryParamsAccumulator,
302302
originalSearchParamsObject: SearchParams
303303
): SearchParams {
304-
const underlyingSearchParamsWithVarying: SearchParams = {}
305-
for (const searchParamName in originalSearchParamsObject) {
306-
Object.defineProperty(underlyingSearchParamsWithVarying, searchParamName, {
307-
get() {
308-
// TODO: Unlike path params, we don't vary track each search param
309-
// individually. The entire search string is treated as a single param.
310-
// This may change in the future.
304+
// Search params have no fixed schema, so any access — missing-key reads, `in`
305+
// checks, or enumeration — must register as varying. A Proxy is required
306+
// (rather than per-property getters) so that enumeration of an empty
307+
// searchParams object still triggers a vary. All accesses bucket into the
308+
// single sentinel '?'; the segment is keyed by the whole query string.
309+
// TODO: Split into per-param tracking if the cache key evolves.
310+
return new Proxy(originalSearchParamsObject, {
311+
get(target, prop, receiver) {
312+
if (typeof prop === 'string') {
311313
accumulateVaryParam(accumulator, '?')
312-
return originalSearchParamsObject[searchParamName]
313-
},
314-
enumerable: true,
315-
})
316-
}
317-
return underlyingSearchParamsWithVarying
314+
}
315+
return Reflect.get(target, prop, receiver)
316+
},
317+
has(target, prop) {
318+
if (typeof prop === 'string') {
319+
accumulateVaryParam(accumulator, '?')
320+
}
321+
return Reflect.has(target, prop)
322+
},
323+
ownKeys(target) {
324+
accumulateVaryParam(accumulator, '?')
325+
return Reflect.ownKeys(target)
326+
},
327+
})
318328
}
319329

320330
/**

test/e2e/app-dir/segment-cache/vary-params/app/(main)/search-params/page.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,11 @@ export default function SearchParamsIndexPage() {
2727

2828
<h2>Target (accesses searchParams)</h2>
2929
<ul>
30+
<li>
31+
<LinkAccordion href="/search-params/target-page">
32+
Target with no search params
33+
</LinkAccordion>
34+
</li>
3035
<li>
3136
<LinkAccordion href="/search-params/target-page?foo=1">
3237
Target with foo=1

test/e2e/app-dir/segment-cache/vary-params/vary-params.test.ts

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,56 @@ describe('segment cache - vary params', () => {
250250
)
251251
})
252252

253+
it('does not reuse prefetched empty-query segment for prefetches with searchParams', async () => {
254+
// When a page reads searchParams that don't exist on the request URL (e.g.
255+
// destructuring `foo` from `/search-params/target-page` with no query),
256+
// that's still an access that affects the response and must register the
257+
// segment as varying by '?'. Otherwise the empty-query prefetch ends up
258+
// keyed at the Fallback search-slot, which shadows subsequent ?foo=N
259+
// prefetches via Fallback resolution and causes them to silently serve the
260+
// wrong (empty-query) response.
261+
let act: ReturnType<typeof createRouterAct>
262+
const browser = await next.browser('/search-params', {
263+
beforePageLoad(p: Playwright.Page) {
264+
act = createRouterAct(p)
265+
},
266+
})
267+
268+
// Prefetch the no-query URL first. The page reads `foo` (a missing key),
269+
// which must register as a vary access.
270+
await act(
271+
async () => {
272+
const toggle = await browser.elementByCss(
273+
'input[data-link-accordion="/search-params/target-page"]'
274+
)
275+
await toggle.click()
276+
},
277+
{ includes: 'Search params target - foo: undefined' }
278+
)
279+
280+
// Prefetching with a search param value must still trigger a new request,
281+
// not silently reuse the empty-query entry through Fallback resolution.
282+
await act(
283+
async () => {
284+
const toggle = await browser.elementByCss(
285+
'input[data-link-accordion="/search-params/target-page?foo=1"]'
286+
)
287+
await toggle.click()
288+
},
289+
{ includes: 'Search params target - foo: 1' }
290+
)
291+
292+
// Navigate and verify the correct content renders for ?foo=1.
293+
const link = await browser.elementByCss(
294+
'a[href="/search-params/target-page?foo=1"]'
295+
)
296+
await link.click()
297+
const content = await browser.elementByCss(
298+
'[data-search-params-content="true"]'
299+
)
300+
expect(await content.text()).toContain('Search params target - foo: 1')
301+
})
302+
253303
it('reuses prefetched segment when page does not access searchParams', async () => {
254304
// When a page does NOT await searchParams, the cache key does NOT include
255305
// search params, so different values share cached prefetch data.
@@ -600,7 +650,15 @@ describe('segment cache - vary params', () => {
600650
)
601651
})
602652

603-
it('shares cached segment across search params when not accessed (runtime prefetch)', async () => {
653+
// TODO: When a Promise resolves with the searchParams Proxy as its value, the
654+
// Promise spec's `[[Resolve]]` algorithm reads `.then` on the Proxy to check
655+
// for thenable assimilation. The Proxy can't distinguish that probe from a
656+
// real `searchParams.then` access, so any runtime-prefetched page that
657+
// doesn't read `searchParams` ends up varying on the entire query string and
658+
// can't share a cached segment. Re-enable once vary-param tracking moves to
659+
// per-param keys. The spec-driven `.then` probe will then resolve to the same
660+
// (undefined) value across these URLs and the cache entry will be reused.
661+
it.skip('shares cached segment across search params when not accessed (runtime prefetch)', async () => {
604662
// Runtime prefetch page that does NOT access searchParams. Since '?'
605663
// is not in varyParams, different search param values share the cache.
606664
let act: ReturnType<typeof createRouterAct>

0 commit comments

Comments
 (0)