Skip to content

Commit 553f2b6

Browse files
lubieowocegnoff
andauthored
[Cache Components] correctly label IO promises in devtools (#84928)
Updates to staged rendering (cacheComponents dev) to support "suspended by" in Suspense Devtools - promises for `cookies()` and other user-callable APIs are now created before we start the render. Each call to `cookies()` will return the same promise (although currently it's wrapped in a fresh proxy). The promise is created via `new Promise`, triggered by a timeout (for the relevant stage) and has `displayName`. This marks it as an IO operation. By re-using the same promise, we make sure that all callsites that await it are considered to be suspended by the same IO operation. - promises for `params` and `searchParams` use a similar trick. All instances of `params` and will be derived from one shared IO promise `sharedParamsParent`. When determining "suspended by", React will walk up the promise chain, find this parent, and use its `displayName`, which we set to `"params"`. `searchParams` work analogously --------- Co-authored-by: Josh Story <story@hey.com>
1 parent 7336e83 commit 553f2b6

File tree

12 files changed

+406
-120
lines changed

12 files changed

+406
-120
lines changed

packages/next/errors.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -887,5 +887,6 @@
887887
"886": "`cacheTag()` is only available with the `cacheComponents` config.",
888888
"887": "`cacheLife()` is only available with the `cacheComponents` config.",
889889
"888": "Unknown \\`cacheLife()\\` profile \"%s\" is not configured in next.config.js\\nmodule.exports = {\n cacheLife: {\n \"%s\": ...\\n }\n}",
890-
"889": "Unknown \\`cacheLife()\\` profile \"%s\" is not configured in next.config.js\\nmodule.exports = {\n cacheLife: {\n \"%s\": ...\\n }\n}"
890+
"889": "Unknown \\`cacheLife()\\` profile \"%s\" is not configured in next.config.js\\nmodule.exports = {\n cacheLife: {\n \"%s\": ...\\n }\n}",
891+
"890": "Received an underlying cookies object that does not match either `cookies` or `mutableCookies`"
891892
}

packages/next/src/server/app-render/app-render.tsx

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2821,6 +2821,12 @@ async function renderWithRestartOnCacheMissInDev(
28212821
// so not having a resume data cache won't break any expectations in case we don't need to restart.
28222822
requestStore.renderResumeDataCache = null
28232823
requestStore.stagedRendering = initialStageController
2824+
requestStore.asyncApiPromises = createAsyncApiPromisesInDev(
2825+
initialStageController,
2826+
requestStore.cookies,
2827+
requestStore.mutableCookies,
2828+
requestStore.headers
2829+
)
28242830
requestStore.cacheSignal = cacheSignal
28252831

28262832
let debugChannel = setReactDebugChannel && createDebugChannel()
@@ -2921,6 +2927,12 @@ async function renderWithRestartOnCacheMissInDev(
29212927
)
29222928
requestStore.stagedRendering = finalStageController
29232929
requestStore.cacheSignal = null
2930+
requestStore.asyncApiPromises = createAsyncApiPromisesInDev(
2931+
finalStageController,
2932+
requestStore.cookies,
2933+
requestStore.mutableCookies,
2934+
requestStore.headers
2935+
)
29242936

29252937
// The initial render already wrote to its debug channel.
29262938
// We're not using it, so we need to create a new one.
@@ -2962,6 +2974,48 @@ async function renderWithRestartOnCacheMissInDev(
29622974
}
29632975
}
29642976

2977+
function createAsyncApiPromisesInDev(
2978+
stagedRendering: StagedRenderingController,
2979+
cookies: RequestStore['cookies'],
2980+
mutableCookies: RequestStore['mutableCookies'],
2981+
headers: RequestStore['headers']
2982+
): NonNullable<RequestStore['asyncApiPromises']> {
2983+
return {
2984+
// Runtime APIs
2985+
cookies: stagedRendering.delayUntilStage(
2986+
RenderStage.Runtime,
2987+
'cookies',
2988+
cookies
2989+
),
2990+
mutableCookies: stagedRendering.delayUntilStage(
2991+
RenderStage.Runtime,
2992+
'cookies',
2993+
mutableCookies as RequestStore['cookies']
2994+
),
2995+
headers: stagedRendering.delayUntilStage(
2996+
RenderStage.Runtime,
2997+
'headers',
2998+
headers
2999+
),
3000+
// These are not used directly, but we chain other `params`/`searchParams` promises off of them.
3001+
sharedParamsParent: stagedRendering.delayUntilStage(
3002+
RenderStage.Runtime,
3003+
'params',
3004+
'<internal params>'
3005+
),
3006+
sharedSearchParamsParent: stagedRendering.delayUntilStage(
3007+
RenderStage.Runtime,
3008+
'searchParams',
3009+
'<internal searchParams>'
3010+
),
3011+
connection: stagedRendering.delayUntilStage(
3012+
RenderStage.Dynamic,
3013+
'connection',
3014+
undefined
3015+
),
3016+
}
3017+
}
3018+
29653019
type DebugChannelPair = {
29663020
serverSide: DebugChannelServer
29673021
clientSide: DebugChannelClient

packages/next/src/server/app-render/staged-rendering.ts

Lines changed: 43 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -52,30 +52,37 @@ export class StagedRenderingController {
5252
}
5353
}
5454

55-
delayUntilStage<T>(stage: NonStaticRenderStage, resolvedValue: T) {
56-
let stagePromise: Promise<void>
55+
private getStagePromise(stage: NonStaticRenderStage): Promise<void> {
5756
switch (stage) {
5857
case RenderStage.Runtime: {
59-
stagePromise = this.runtimeStagePromise.promise
60-
break
58+
return this.runtimeStagePromise.promise
6159
}
6260
case RenderStage.Dynamic: {
63-
stagePromise = this.dynamicStagePromise.promise
64-
break
61+
return this.dynamicStagePromise.promise
6562
}
6663
default: {
6764
stage satisfies never
6865
throw new InvariantError(`Invalid render stage: ${stage}`)
6966
}
7067
}
68+
}
69+
70+
waitForStage(stage: NonStaticRenderStage) {
71+
return this.getStagePromise(stage)
72+
}
73+
74+
delayUntilStage<T>(
75+
stage: NonStaticRenderStage,
76+
displayName: string | undefined,
77+
resolvedValue: T
78+
) {
79+
const ioTriggerPromise = this.getStagePromise(stage)
7180

72-
// FIXME: this seems to be the only form that leads to correct API names
73-
// being displayed in React Devtools (in the "suspended by" section).
74-
// If we use `promise.then(() => resolvedValue)`, the names are lost.
75-
// It's a bit strange that only one of those works right.
76-
const promise = new Promise<T>((resolve, reject) => {
77-
stagePromise.then(resolve.bind(null, resolvedValue), reject)
78-
})
81+
const promise = makeDevtoolsIOPromiseFromIOTrigger(
82+
ioTriggerPromise,
83+
displayName,
84+
resolvedValue
85+
)
7986

8087
// Analogously to `makeHangingPromise`, we might reject this promise if the signal is invoked.
8188
// (e.g. in the case where we don't want want the render to proceed to the dynamic stage and abort it).
@@ -88,3 +95,26 @@ export class StagedRenderingController {
8895
}
8996

9097
function ignoreReject() {}
98+
99+
// TODO(restart-on-cache-miss): the layering of `delayUntilStage`,
100+
// `makeDevtoolsIOPromiseFromIOTrigger` and and `makeDevtoolsIOAwarePromise`
101+
// is confusing, we should clean it up.
102+
function makeDevtoolsIOPromiseFromIOTrigger<T>(
103+
ioTrigger: Promise<any>,
104+
displayName: string | undefined,
105+
resolvedValue: T
106+
): Promise<T> {
107+
// If we create a `new Promise` and give it a displayName
108+
// (with no userspace code above us in the stack)
109+
// React Devtools will use it as the IO cause when determining "suspended by".
110+
// In particular, it should shadow any inner IO that resolved/rejected the promise
111+
// (in case of staged rendering, this will be the `setTimeout` that triggers the relevant stage)
112+
const promise = new Promise<T>((resolve, reject) => {
113+
ioTrigger.then(resolve.bind(null, resolvedValue), reject)
114+
})
115+
if (displayName !== undefined) {
116+
// @ts-expect-error
117+
promise.displayName = displayName
118+
}
119+
return promise
120+
}

packages/next/src/server/app-render/work-unit-async-storage.external.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,22 @@ export interface RequestStore extends CommonWorkUnitStore {
7070
usedDynamic?: boolean
7171
devFallbackParams?: OpaqueFallbackRouteParams | null
7272
stagedRendering?: StagedRenderingController | null
73+
asyncApiPromises?: DevAsyncApiPromises
7374
cacheSignal?: CacheSignal | null
7475
prerenderResumeDataCache?: PrerenderResumeDataCache | null
7576
}
7677

78+
type DevAsyncApiPromises = {
79+
cookies: Promise<ReadonlyRequestCookies>
80+
mutableCookies: Promise<ReadonlyRequestCookies>
81+
headers: Promise<ReadonlyHeaders>
82+
83+
sharedParamsParent: Promise<string>
84+
sharedSearchParamsParent: Promise<string>
85+
86+
connection: Promise<undefined>
87+
}
88+
7789
/**
7890
* The Prerender store is for tracking information related to prerenders.
7991
*

packages/next/src/server/dynamic-rendering-utils.ts

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,11 @@ export function makeDevtoolsIOAwarePromise<T>(
8383
): Promise<T> {
8484
if (requestStore.stagedRendering) {
8585
// We resolve each stage in a timeout, so React DevTools will pick this up as IO.
86-
return requestStore.stagedRendering.delayUntilStage(stage, underlying)
86+
return requestStore.stagedRendering.delayUntilStage(
87+
stage,
88+
undefined,
89+
underlying
90+
)
8791
}
8892
// in React DevTools if we resolve in a setTimeout we will observe
8993
// the promise resolution as something that can suspend a boundary or root.

packages/next/src/server/lib/patch-fetch.ts

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -562,9 +562,8 @@ export function createPatchedFetcher(
562562
cacheSignal.endRead()
563563
cacheSignal = null
564564
}
565-
await workUnitStore.stagedRendering.delayUntilStage(
566-
RenderStage.Dynamic,
567-
undefined
565+
await workUnitStore.stagedRendering.waitForStage(
566+
RenderStage.Dynamic
568567
)
569568
}
570569
break
@@ -689,9 +688,8 @@ export function createPatchedFetcher(
689688
cacheSignal.endRead()
690689
cacheSignal = null
691690
}
692-
await workUnitStore.stagedRendering.delayUntilStage(
693-
RenderStage.Dynamic,
694-
undefined
691+
await workUnitStore.stagedRendering.waitForStage(
692+
RenderStage.Dynamic
695693
)
696694
}
697695
break
@@ -962,9 +960,8 @@ export function createPatchedFetcher(
962960
process.env.NODE_ENV === 'development' &&
963961
workUnitStore.stagedRendering
964962
) {
965-
await workUnitStore.stagedRendering.delayUntilStage(
966-
RenderStage.Dynamic,
967-
undefined
963+
await workUnitStore.stagedRendering.waitForStage(
964+
RenderStage.Dynamic
968965
)
969966
}
970967
break
@@ -1091,9 +1088,8 @@ export function createPatchedFetcher(
10911088
cacheSignal.endRead()
10921089
cacheSignal = null
10931090
}
1094-
await workUnitStore.stagedRendering.delayUntilStage(
1095-
RenderStage.Dynamic,
1096-
undefined
1091+
await workUnitStore.stagedRendering.waitForStage(
1092+
RenderStage.Dynamic
10971093
)
10981094
}
10991095
break
@@ -1138,9 +1134,8 @@ export function createPatchedFetcher(
11381134
process.env.NODE_ENV === 'development' &&
11391135
workUnitStore.stagedRendering
11401136
) {
1141-
await workUnitStore.stagedRendering.delayUntilStage(
1142-
RenderStage.Dynamic,
1143-
undefined
1137+
await workUnitStore.stagedRendering.waitForStage(
1138+
RenderStage.Dynamic
11441139
)
11451140
}
11461141
break

packages/next/src/server/request/connection.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -106,6 +106,9 @@ export function connection(): Promise<void> {
106106
// Semantically we only need the dev tracking when running in `next dev`
107107
// but since you would never use next dev with production NODE_ENV we use this
108108
// as a proxy so we can statically exclude this code from production builds.
109+
if (workUnitStore.asyncApiPromises) {
110+
return workUnitStore.asyncApiPromises.connection
111+
}
109112
return makeDevtoolsIOAwarePromise(
110113
undefined,
111114
workUnitStore,

0 commit comments

Comments
 (0)