Skip to content

Commit ae5026a

Browse files
authored
Unused searchParams in "use cache" page should not cause dynamic (#75662)
When `dynamicIO` is not enabled, and only the standalone `useCache` experimental flag is enabled, we can not encode `searchParams` as hanging promises. To still avoid unused search params from making a page dynamic (due to the `searchParams` being encoded into the cache key), we set them to a promise that resolves to an empty object, while also overwriting the to-be-invoked function for generating a cache entry with a function that creates an erroring `searchParams` prop before invoking the original function. This ensures that used `searchParams` inside of cached functions would still yield an error. closes NAR-85
1 parent 1eb708a commit ae5026a

File tree

20 files changed

+376
-116
lines changed

20 files changed

+376
-116
lines changed

Diff for: packages/next/errors.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -631,5 +631,6 @@
631631
"630": "Invariant (SlowModuleDetectionPlugin): Module is recorded after the report is generated. This is a Next.js internal bug.",
632632
"631": "Invariant (SlowModuleDetectionPlugin): Circular dependency detected in module graph. This is a Next.js internal bug.",
633633
"632": "Invariant (SlowModuleDetectionPlugin): Module is missing a required debugId. This is a Next.js internal bug.",
634-
"633": "Dynamic route not found"
634+
"633": "Dynamic route not found",
635+
"634": "Route %s used \"searchParams\" inside \"use cache\". Accessing Dynamic data sources inside a cache scope is not supported. If you need this data inside a cached function use \"searchParams\" outside of the cached function and pass the required dynamic data in as an argument. See more info here: https://nextjs.org/docs/messages/next-request-in-use-cache"
635636
}

Diff for: packages/next/src/build/segment-config/app/app-segments.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ import {
1818
isAppRouteRouteModule,
1919
isAppPageRouteModule,
2020
} from '../../../server/route-modules/checks'
21-
import { isClientReference } from '../../../lib/client-reference'
21+
import { isClientReference } from '../../../lib/client-and-server-references'
2222
import { getSegmentParam } from '../../../server/app-render/get-segment-param'
2323
import {
2424
getLayoutOrPageModule,

Diff for: packages/next/src/build/utils.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ import { Sema } from 'next/dist/compiled/async-sema'
6464
import { denormalizePagePath } from '../shared/lib/page-path/denormalize-page-path'
6565
import { normalizePagePath } from '../shared/lib/page-path/normalize-page-path'
6666
import { getRuntimeContext } from '../server/web/sandbox'
67-
import { isClientReference } from '../lib/client-reference'
67+
import { isClientReference } from '../lib/client-and-server-references'
6868
import { normalizeAppPath } from '../shared/lib/router/utils/app-paths'
6969
import { denormalizeAppPagePath } from '../shared/lib/page-path/denormalize-app-path'
7070
import { RouteKind } from '../server/route-kind'

Diff for: packages/next/src/client/components/router-reducer/reducers/server-action-reducer.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ import { hasBasePath } from '../../../has-base-path'
5555
import {
5656
extractInfoFromServerReferenceId,
5757
omitUnusedArgs,
58-
} from './server-reference-info'
58+
} from '../../../../shared/lib/server-reference-info'
5959
import { revalidateEntireCache } from '../../segment-cache/cache'
6060

6161
type FetchServerActionResult = {
+33
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
import { extractInfoFromServerReferenceId } from '../shared/lib/server-reference-info'
2+
3+
// Only contains the properties we're interested in.
4+
export interface ServerReference {
5+
$$typeof: Symbol
6+
$$id: string
7+
}
8+
9+
export type ServerFunction = ServerReference &
10+
((...args: unknown[]) => Promise<unknown>)
11+
12+
export function isServerReference<T>(
13+
value: T & Partial<ServerReference>
14+
): value is T & ServerFunction {
15+
return value.$$typeof === Symbol.for('react.server.reference')
16+
}
17+
18+
export function isUseCacheFunction<T>(
19+
value: T & Partial<ServerReference>
20+
): value is T & ServerFunction {
21+
if (!isServerReference(value)) {
22+
return false
23+
}
24+
25+
const { type } = extractInfoFromServerReferenceId(value.$$id)
26+
27+
return type === 'use-cache'
28+
}
29+
30+
export function isClientReference(mod: any): boolean {
31+
const defaultExport = mod?.default || mod
32+
return defaultExport?.$$typeof === Symbol.for('react.client.reference')
33+
}

Diff for: packages/next/src/lib/client-reference.ts

-4
This file was deleted.

Diff for: packages/next/src/server/app-render/create-component-tree.tsx

+39-10
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import type { CacheNodeSeedData, PreloadCallbacks } from './types'
22
import React from 'react'
3-
import { isClientReference } from '../../lib/client-reference'
3+
import {
4+
isClientReference,
5+
isUseCacheFunction,
6+
} from '../../lib/client-and-server-references'
47
import { getLayoutOrPageModule } from '../lib/app-dir-module'
58
import type { LoaderTree } from '../lib/app-dir-module'
69
import { interopDefault } from './interop-default'
@@ -19,6 +22,7 @@ import type { Params } from '../request/params'
1922
import { workUnitAsyncStorage } from './work-unit-async-storage.external'
2023
import { OUTLET_BOUNDARY_NAME } from '../../lib/metadata/metadata-constants'
2124
import { DEFAULT_SEGMENT_KEY } from '../../shared/lib/segment'
25+
import type { UseCachePageComponentProps } from '../use-cache/use-cache-wrapper'
2226

2327
/**
2428
* Use the provided loader tree to create the React Component tree.
@@ -652,19 +656,44 @@ async function createComponentTreeInternal({
652656
)
653657
}
654658
} else {
655-
// If we are passing searchParams to a server component Page we need to track their usage in case
656-
// the current render mode tracks dynamic API usage.
659+
// If we are passing params to a server component Page we need to track
660+
// their usage in case the current render mode tracks dynamic API usage.
657661
const params = createServerParamsForServerSegment(
658662
currentParams,
659663
workStore
660664
)
661-
const searchParams = createServerSearchParamsForServerPage(
662-
query,
663-
workStore
664-
)
665-
pageElement = (
666-
<PageComponent params={params} searchParams={searchParams} />
667-
)
665+
666+
// TODO(useCache): Should we use this trick also if dynamicIO is enabled,
667+
// instead of relying on the searchParams being a hanging promise?
668+
if (!experimental.dynamicIO && isUseCacheFunction(PageComponent)) {
669+
const UseCachePageComponent: React.ComponentType<UseCachePageComponentProps> =
670+
PageComponent
671+
672+
// The "use cache" wrapper takes care of converting this into an
673+
// erroring search params promise when passing it to the original
674+
// function.
675+
const searchParams = Promise.resolve({})
676+
677+
pageElement = (
678+
<UseCachePageComponent
679+
params={params}
680+
searchParams={searchParams}
681+
$$isPageComponent
682+
/>
683+
)
684+
} else {
685+
// If we are passing searchParams to a server component Page we need to
686+
// track their usage in case the current render mode tracks dynamic API
687+
// usage.
688+
const searchParams = createServerSearchParamsForServerPage(
689+
query,
690+
workStore
691+
)
692+
693+
pageElement = (
694+
<PageComponent params={params} searchParams={searchParams} />
695+
)
696+
}
668697
}
669698
return [
670699
actualSegment,

Diff for: packages/next/src/server/app-render/work-async-storage.external.ts

+1
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export interface WorkStore {
7272
readonly assetPrefix?: string
7373

7474
rootParams: Params
75+
dynamicIOEnabled: boolean
7576
}
7677

7778
export type WorkAsyncStorage = AsyncLocalStorage<WorkStore>

Diff for: packages/next/src/server/async-storage/work-store.ts

+1
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ export function createWorkStore({
126126
assetPrefix: renderOpts?.assetPrefix || '',
127127

128128
afterContext: createAfterContext(renderOpts),
129+
dynamicIOEnabled: renderOpts.experimental.dynamicIO,
129130
}
130131

131132
// TODO: remove this when we resolve accessing the store outside the execution context

Diff for: packages/next/src/server/request/search-params.ts

+82-92
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
describeHasCheckingStringProperty,
2626
throwWithStaticGenerationBailoutErrorWithDynamicError,
2727
wellKnownProperties,
28+
throwForSearchParamsAccessInUseCache,
2829
} from './utils'
2930
import { scheduleImmediate } from '../../lib/scheduler'
3031

@@ -167,6 +168,11 @@ function createRenderSearchParams(
167168
interface CacheLifetime {}
168169
const CachedSearchParams = new WeakMap<CacheLifetime, Promise<SearchParams>>()
169170

171+
const CachedSearchParamsForUseCache = new WeakMap<
172+
CacheLifetime,
173+
Promise<SearchParams>
174+
>()
175+
170176
function makeAbortingExoticSearchParams(
171177
route: string,
172178
prerenderStore: PrerenderStoreModern
@@ -203,31 +209,9 @@ function makeAbortingExoticSearchParams(
203209
annotateDynamicAccess(expression, prerenderStore)
204210
return ReflectAdapter.get(target, prop, receiver)
205211
}
206-
// Object prototype
207-
case 'hasOwnProperty':
208-
case 'isPrototypeOf':
209-
case 'propertyIsEnumerable':
210-
case 'toString':
211-
case 'valueOf':
212-
case 'toLocaleString':
213-
214-
// Promise prototype
215-
// fallthrough
216-
case 'catch':
217-
case 'finally':
218-
219-
// Common tested properties
220-
// fallthrough
221-
case 'toJSON':
222-
case '$$typeof':
223-
case '__esModule': {
224-
// These properties cannot be shadowed because they need to be the
225-
// true underlying value for Promises to work correctly at runtime
226-
return ReflectAdapter.get(target, prop, receiver)
227-
}
228212

229213
default: {
230-
if (typeof prop === 'string') {
214+
if (typeof prop === 'string' && !wellKnownProperties.has(prop)) {
231215
const expression = describeStringPropertyAccess(
232216
'searchParams',
233217
prop
@@ -306,28 +290,6 @@ function makeErroringExoticSearchParams(
306290
}
307291

308292
switch (prop) {
309-
// Object prototype
310-
case 'hasOwnProperty':
311-
case 'isPrototypeOf':
312-
case 'propertyIsEnumerable':
313-
case 'toString':
314-
case 'valueOf':
315-
case 'toLocaleString':
316-
317-
// Promise prototype
318-
// fallthrough
319-
case 'catch':
320-
case 'finally':
321-
322-
// Common tested properties
323-
// fallthrough
324-
case 'toJSON':
325-
case '$$typeof':
326-
case '__esModule': {
327-
// These properties cannot be shadowed because they need to be the
328-
// true underlying value for Promises to work correctly at runtime
329-
return ReflectAdapter.get(target, prop, receiver)
330-
}
331293
case 'then': {
332294
const expression =
333295
'`await searchParams`, `searchParams.then`, or similar'
@@ -379,7 +341,7 @@ function makeErroringExoticSearchParams(
379341
return
380342
}
381343
default: {
382-
if (typeof prop === 'string') {
344+
if (typeof prop === 'string' && !wellKnownProperties.has(prop)) {
383345
const expression = describeStringPropertyAccess(
384346
'searchParams',
385347
prop
@@ -469,6 +431,63 @@ function makeErroringExoticSearchParams(
469431
return proxiedPromise
470432
}
471433

434+
/**
435+
* This is a variation of `makeErroringExoticSearchParams` that always throws an
436+
* error on access, because accessing searchParams inside of `"use cache"` is
437+
* not allowed.
438+
*/
439+
export function makeErroringExoticSearchParamsForUseCache(
440+
workStore: WorkStore
441+
): Promise<SearchParams> {
442+
const cachedSearchParams = CachedSearchParamsForUseCache.get(workStore)
443+
if (cachedSearchParams) {
444+
return cachedSearchParams
445+
}
446+
447+
const promise = Promise.resolve({})
448+
449+
const proxiedPromise = new Proxy(promise, {
450+
get(target, prop, receiver) {
451+
if (Object.hasOwn(promise, prop)) {
452+
// The promise has this property directly. we must return it. We know it
453+
// isn't a dynamic access because it can only be something that was
454+
// previously written to the promise and thus not an underlying
455+
// searchParam value
456+
return ReflectAdapter.get(target, prop, receiver)
457+
}
458+
459+
if (
460+
typeof prop === 'string' &&
461+
(prop === 'then' || !wellKnownProperties.has(prop))
462+
) {
463+
throwForSearchParamsAccessInUseCache(workStore.route)
464+
}
465+
466+
return ReflectAdapter.get(target, prop, receiver)
467+
},
468+
has(target, prop) {
469+
// We don't expect key checking to be used except for testing the existence of
470+
// searchParams so we make all has tests throw an error. this means that `promise.then`
471+
// can resolve to the then function on the Promise prototype but 'then' in promise will assume
472+
// you are testing whether the searchParams has a 'then' property.
473+
if (
474+
typeof prop === 'string' &&
475+
(prop === 'then' || !wellKnownProperties.has(prop))
476+
) {
477+
throwForSearchParamsAccessInUseCache(workStore.route)
478+
}
479+
480+
return ReflectAdapter.has(target, prop)
481+
},
482+
ownKeys() {
483+
throwForSearchParamsAccessInUseCache(workStore.route)
484+
},
485+
})
486+
487+
CachedSearchParamsForUseCache.set(workStore, proxiedPromise)
488+
return proxiedPromise
489+
}
490+
472491
function makeUntrackedExoticSearchParams(
473492
underlyingSearchParams: SearchParams,
474493
store: WorkStore
@@ -485,52 +504,23 @@ function makeUntrackedExoticSearchParams(
485504
CachedSearchParams.set(underlyingSearchParams, promise)
486505

487506
Object.keys(underlyingSearchParams).forEach((prop) => {
488-
switch (prop) {
489-
// Object prototype
490-
case 'hasOwnProperty':
491-
case 'isPrototypeOf':
492-
case 'propertyIsEnumerable':
493-
case 'toString':
494-
case 'valueOf':
495-
case 'toLocaleString':
496-
497-
// Promise prototype
498-
// fallthrough
499-
case 'then':
500-
case 'catch':
501-
case 'finally':
502-
503-
// React Promise extension
504-
// fallthrough
505-
case 'status':
506-
507-
// Common tested properties
508-
// fallthrough
509-
case 'toJSON':
510-
case '$$typeof':
511-
case '__esModule': {
512-
// These properties cannot be shadowed because they need to be the
513-
// true underlying value for Promises to work correctly at runtime
514-
break
515-
}
516-
default: {
517-
Object.defineProperty(promise, prop, {
518-
get() {
519-
const workUnitStore = workUnitAsyncStorage.getStore()
520-
trackDynamicDataInDynamicRender(store, workUnitStore)
521-
return underlyingSearchParams[prop]
522-
},
523-
set(value) {
524-
Object.defineProperty(promise, prop, {
525-
value,
526-
writable: true,
527-
enumerable: true,
528-
})
529-
},
530-
enumerable: true,
531-
configurable: true,
532-
})
533-
}
507+
if (!wellKnownProperties.has(prop)) {
508+
Object.defineProperty(promise, prop, {
509+
get() {
510+
const workUnitStore = workUnitAsyncStorage.getStore()
511+
trackDynamicDataInDynamicRender(store, workUnitStore)
512+
return underlyingSearchParams[prop]
513+
},
514+
set(value) {
515+
Object.defineProperty(promise, prop, {
516+
value,
517+
writable: true,
518+
enumerable: true,
519+
})
520+
},
521+
enumerable: true,
522+
configurable: true,
523+
})
534524
}
535525
})
536526

0 commit comments

Comments
 (0)