Skip to content

Commit 29ff999

Browse files
committed
wip: correctly label IO promises in devtools
1 parent 3dd1939 commit 29ff999

File tree

10 files changed

+261
-61
lines changed

10 files changed

+261
-61
lines changed

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

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2757,6 +2757,12 @@ async function renderWithRestartOnCacheMissInDev(
27572757
stagedRendering: initialStageController,
27582758
prerenderResumeDataCache,
27592759
cacheSignal,
2760+
asyncApiPromises: createAsyncApiPromisesInDev(
2761+
initialStageController,
2762+
requestStore.cookies,
2763+
requestStore.mutableCookies,
2764+
requestStore.headers
2765+
),
27602766
dynamicTracking: createDynamicTrackingState(
27612767
false // isDebugDynamicAccesses
27622768
),
@@ -2870,6 +2876,12 @@ async function renderWithRestartOnCacheMissInDev(
28702876
stagedRendering: finalStageController,
28712877
prerenderResumeDataCache: null,
28722878
cacheSignal: null,
2879+
asyncApiPromises: createAsyncApiPromisesInDev(
2880+
initialStageController,
2881+
requestStore.cookies,
2882+
requestStore.mutableCookies,
2883+
requestStore.headers
2884+
),
28732885
dynamicTracking: createDynamicTrackingState(
28742886
false // isDebugDynamicAccesses
28752887
),
@@ -2920,6 +2932,48 @@ async function renderWithRestartOnCacheMissInDev(
29202932
}
29212933
}
29222934

2935+
function createAsyncApiPromisesInDev(
2936+
stagedRendering: StagedRenderingController,
2937+
cookies: RequestStore['cookies'],
2938+
mutableCookies: RequestStore['mutableCookies'],
2939+
headers: RequestStore['headers']
2940+
): DevRequestStoreModern['asyncApiPromises'] {
2941+
return {
2942+
// Runtime APIs
2943+
cookies: stagedRendering.delayUntilStage(
2944+
RenderStage.Runtime,
2945+
'cookies',
2946+
cookies
2947+
),
2948+
mutableCookies: stagedRendering.delayUntilStage(
2949+
RenderStage.Runtime,
2950+
'cookies',
2951+
mutableCookies as RequestStore['cookies']
2952+
),
2953+
headers: stagedRendering.delayUntilStage(
2954+
RenderStage.Runtime,
2955+
'headers',
2956+
headers
2957+
),
2958+
// These are not used directly, but we chain other `params`/`searchParams` promises off of them.
2959+
sharedParamsParent: stagedRendering.delayUntilStage(
2960+
RenderStage.Runtime,
2961+
'params',
2962+
'<internal params>'
2963+
),
2964+
sharedSearchParamsParent: stagedRendering.delayUntilStage(
2965+
RenderStage.Runtime,
2966+
'searchParams',
2967+
'<internal searchParams>'
2968+
),
2969+
connection: stagedRendering.delayUntilStage(
2970+
RenderStage.Dynamic,
2971+
'connection',
2972+
undefined
2973+
),
2974+
}
2975+
}
2976+
29232977
type DebugChannelPair = {
29242978
serverSide: DebugChannelServer
29252979
clientSide: DebugChannelClient

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

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

55-
delayUntilStage<T>(stage: NonStaticRenderStage, resolvedValue: T) {
55+
delayUntilStage<T>(
56+
stage: NonStaticRenderStage,
57+
displayName: string | undefined,
58+
resolvedValue: T
59+
) {
5660
let stagePromise: Promise<void>
5761
switch (stage) {
5862
case RenderStage.Runtime: {
@@ -69,13 +73,11 @@ export class StagedRenderingController {
6973
}
7074
}
7175

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-
})
76+
const promise = createDevtoolsIOPromise(
77+
stagePromise,
78+
displayName,
79+
resolvedValue
80+
)
7981

8082
// Analogously to `makeHangingPromise`, we might reject this promise if the signal is invoked.
8183
// (e.g. in the case where we don't want want the render to proceed to the dynamic stage and abort it).
@@ -87,4 +89,24 @@ export class StagedRenderingController {
8789
}
8890
}
8991

92+
function createDevtoolsIOPromise<T>(
93+
trigger: Promise<any>,
94+
displayName: string | undefined,
95+
resolvedValue: T
96+
): Promise<T> {
97+
// If we create a `new Promise` and give it a displayName
98+
// (with no userspace code above us in the stack)
99+
// ReactDevtools will use it as the IO cause when determining "suspended by".
100+
// In particular, it should shadow any inner IO that resolved/rejected the promise
101+
// (which in this case will be the `setTimeout` that triggers the relevant stage)
102+
const promise = new Promise<T>((resolve, reject) => {
103+
trigger.then(resolve.bind(null, resolvedValue), reject)
104+
})
105+
if (displayName !== undefined) {
106+
// @ts-expect-error
107+
promise.displayName = displayName
108+
}
109+
return promise
110+
}
111+
90112
function ignoreReject() {}

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
@@ -85,6 +85,7 @@ export type CommonDevStoreModern = {
8585
readonly stagedRendering: StagedRenderingController
8686
readonly captureOwnerStack: () => string | null
8787
readonly dynamicTracking: DynamicTrackingState
88+
readonly asyncApiPromises: DevAsyncApiPromises
8889
} & (
8990
| {
9091
// In the initial render, we track and fill caches
@@ -102,6 +103,17 @@ type NoneOf<TObj extends Record<string, any>> = {
102103
[key in keyof TObj]?: undefined
103104
}
104105

106+
type DevAsyncApiPromises = {
107+
cookies: Promise<ReadonlyRequestCookies>
108+
mutableCookies: Promise<ReadonlyRequestCookies>
109+
headers: Promise<ReadonlyHeaders>
110+
111+
sharedParamsParent: Promise<string>
112+
sharedSearchParamsParent: Promise<string>
113+
114+
connection: Promise<undefined>
115+
}
116+
105117
/**
106118
* The Prerender store is for tracking information related to prerenders.
107119
*

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/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

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

Lines changed: 28 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -455,6 +455,19 @@ function makeDynamicallyTrackedParamsWithDevWarnings(
455455
workStore: WorkStore,
456456
requestStore: RequestStore
457457
): Promise<Params> {
458+
if (requestStore.asyncApiPromises && hasFallbackParams) {
459+
const promise = requestStore.asyncApiPromises.sharedParamsParent.then(
460+
() => underlyingParams
461+
)
462+
// TODO(restart-on-cache-miss): Instrument with warnings
463+
// return instrumentParamsPromiseWithDevWarnings(
464+
// underlyingParams,
465+
// promise,
466+
// workStore
467+
// )
468+
return promise
469+
}
470+
458471
const cachedParams = CachedParams.get(underlyingParams)
459472
if (cachedParams) {
460473
return cachedParams
@@ -472,6 +485,20 @@ function makeDynamicallyTrackedParamsWithDevWarnings(
472485
: // We don't want to force an environment transition when this params is not part of the fallback params set
473486
Promise.resolve(underlyingParams)
474487

488+
const proxiedPromise = instrumentParamsPromiseWithDevWarnings(
489+
underlyingParams,
490+
promise,
491+
workStore
492+
)
493+
CachedParams.set(underlyingParams, proxiedPromise)
494+
return proxiedPromise
495+
}
496+
497+
function instrumentParamsPromiseWithDevWarnings(
498+
underlyingParams: Params,
499+
promise: Promise<Params>,
500+
workStore: WorkStore
501+
): Promise<Params> {
475502
// Track which properties we should warn for.
476503
const proxiedProperties = new Set<string>()
477504

@@ -484,7 +511,7 @@ function makeDynamicallyTrackedParamsWithDevWarnings(
484511
}
485512
})
486513

487-
const proxiedPromise = new Proxy(promise, {
514+
return new Proxy(promise, {
488515
get(target, prop, receiver) {
489516
if (typeof prop === 'string') {
490517
if (
@@ -509,9 +536,6 @@ function makeDynamicallyTrackedParamsWithDevWarnings(
509536
return Reflect.ownKeys(target)
510537
},
511538
})
512-
513-
CachedParams.set(underlyingParams, proxiedPromise)
514-
return proxiedPromise
515539
}
516540

517541
const warnForSyncAccess = createDedupedByCallsiteServerErrorLoggerDev(

0 commit comments

Comments
 (0)