Skip to content

Commit f08c335

Browse files
lubieowocegnoff
authored andcommitted
wip: correctly label IO promises in devtools
1 parent 1b949df commit f08c335

File tree

12 files changed

+294
-67
lines changed

12 files changed

+294
-67
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 a 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: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,7 @@ export function createPatchedFetcher(
564564
}
565565
await workUnitStore.stagedRendering.delayUntilStage(
566566
RenderStage.Dynamic,
567+
undefined,
567568
undefined
568569
)
569570
}
@@ -691,6 +692,7 @@ export function createPatchedFetcher(
691692
}
692693
await workUnitStore.stagedRendering.delayUntilStage(
693694
RenderStage.Dynamic,
695+
undefined,
694696
undefined
695697
)
696698
}
@@ -964,6 +966,7 @@ export function createPatchedFetcher(
964966
) {
965967
await workUnitStore.stagedRendering.delayUntilStage(
966968
RenderStage.Dynamic,
969+
undefined,
967970
undefined
968971
)
969972
}
@@ -1093,6 +1096,7 @@ export function createPatchedFetcher(
10931096
}
10941097
await workUnitStore.stagedRendering.delayUntilStage(
10951098
RenderStage.Dynamic,
1099+
undefined,
10961100
undefined
10971101
)
10981102
}
@@ -1140,6 +1144,7 @@ export function createPatchedFetcher(
11401144
) {
11411145
await workUnitStore.stagedRendering.delayUntilStage(
11421146
RenderStage.Dynamic,
1147+
undefined,
11431148
undefined
11441149
)
11451150
}

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,

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

Lines changed: 35 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,22 @@ function makeUntrackedCookiesWithDevWarnings(
190190
underlyingCookies: ReadonlyRequestCookies,
191191
route?: string
192192
): Promise<ReadonlyRequestCookies> {
193+
if (requestStore.asyncApiPromises) {
194+
let promise: Promise<ReadonlyRequestCookies>
195+
if (underlyingCookies === requestStore.mutableCookies) {
196+
promise = requestStore.asyncApiPromises.mutableCookies
197+
} else if (underlyingCookies === requestStore.cookies) {
198+
promise = requestStore.asyncApiPromises.cookies
199+
} else {
200+
throw new InvariantError(
201+
'Received a underlying cookies object that does not match either `cookies` or `mutableCookies`'
202+
)
203+
}
204+
// TODO(restart-on-cache-miss): Instrument with warnings
205+
// return instrumentCookiesPromiseWithDevWarnings(promise, route)
206+
return promise
207+
}
208+
193209
const cachedCookies = CachedCookies.get(underlyingCookies)
194210
if (cachedCookies) {
195211
return cachedCookies
@@ -201,7 +217,22 @@ function makeUntrackedCookiesWithDevWarnings(
201217
RenderStage.Runtime
202218
)
203219

204-
const proxiedPromise = new Proxy(promise, {
220+
const proxiedPromise = instrumentCookiesPromiseWithDevWarnings(promise, route)
221+
222+
CachedCookies.set(underlyingCookies, proxiedPromise)
223+
224+
return proxiedPromise
225+
}
226+
227+
const warnForSyncAccess = createDedupedByCallsiteServerErrorLoggerDev(
228+
createCookiesAccessError
229+
)
230+
231+
function instrumentCookiesPromiseWithDevWarnings(
232+
promise: Promise<ReadonlyRequestCookies>,
233+
route: string | undefined
234+
) {
235+
return new Proxy(promise, {
205236
get(target, prop, receiver) {
206237
switch (prop) {
207238
case Symbol.iterator: {
@@ -226,17 +257,12 @@ function makeUntrackedCookiesWithDevWarnings(
226257

227258
return ReflectAdapter.get(target, prop, receiver)
228259
},
260+
set(target, prop, newValue, receiver) {
261+
return ReflectAdapter.set(target, prop, newValue, receiver)
262+
},
229263
})
230-
231-
CachedCookies.set(underlyingCookies, proxiedPromise)
232-
233-
return proxiedPromise
234264
}
235265

236-
const warnForSyncAccess = createDedupedByCallsiteServerErrorLoggerDev(
237-
createCookiesAccessError
238-
)
239-
240266
function createCookiesAccessError(
241267
route: string | undefined,
242268
expression: string

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

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,13 @@ function makeUntrackedHeadersWithDevWarnings(
199199
route: string | undefined,
200200
requestStore: RequestStore
201201
): Promise<ReadonlyHeaders> {
202+
if (requestStore.asyncApiPromises) {
203+
const promise = requestStore.asyncApiPromises.headers
204+
// TODO(restart-on-cache-miss): Instrument with warnings
205+
// return instrumentHeadersPromiseWithDevWarnings(promise, route)
206+
return promise
207+
}
208+
202209
const cachedHeaders = CachedHeaders.get(underlyingHeaders)
203210
if (cachedHeaders) {
204211
return cachedHeaders
@@ -210,7 +217,22 @@ function makeUntrackedHeadersWithDevWarnings(
210217
RenderStage.Runtime
211218
)
212219

213-
const proxiedPromise = new Proxy(promise, {
220+
const proxiedPromise = instrumentHeadersPromiseWithDevWarnings(promise, route)
221+
222+
CachedHeaders.set(underlyingHeaders, proxiedPromise)
223+
224+
return proxiedPromise
225+
}
226+
227+
const warnForSyncAccess = createDedupedByCallsiteServerErrorLoggerDev(
228+
createHeadersAccessError
229+
)
230+
231+
function instrumentHeadersPromiseWithDevWarnings(
232+
promise: Promise<ReadonlyHeaders>,
233+
route: string | undefined
234+
) {
235+
return new Proxy(promise, {
214236
get(target, prop, receiver) {
215237
switch (prop) {
216238
case Symbol.iterator: {
@@ -237,17 +259,12 @@ function makeUntrackedHeadersWithDevWarnings(
237259

238260
return ReflectAdapter.get(target, prop, receiver)
239261
},
262+
set(target, prop, newValue, receiver) {
263+
return ReflectAdapter.set(target, prop, newValue, receiver)
264+
},
240265
})
241-
242-
CachedHeaders.set(underlyingHeaders, proxiedPromise)
243-
244-
return proxiedPromise
245266
}
246267

247-
const warnForSyncAccess = createDedupedByCallsiteServerErrorLoggerDev(
248-
createHeadersAccessError
249-
)
250-
251268
function createHeadersAccessError(
252269
route: string | undefined,
253270
expression: string

0 commit comments

Comments
 (0)