Skip to content

Commit 29fc704

Browse files
committed
wip: correctly label IO promises in devtools
1 parent 1f62ec4 commit 29fc704

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
@@ -2797,6 +2797,12 @@ async function renderWithRestartOnCacheMissInDev(
27972797
stagedRendering: initialStageController,
27982798
prerenderResumeDataCache,
27992799
cacheSignal,
2800+
asyncApiPromises: createAsyncApiPromisesInDev(
2801+
initialStageController,
2802+
requestStore.cookies,
2803+
requestStore.mutableCookies,
2804+
requestStore.headers
2805+
),
28002806
dynamicTracking: createDynamicTrackingState(
28012807
false // isDebugDynamicAccesses
28022808
),
@@ -2910,6 +2916,12 @@ async function renderWithRestartOnCacheMissInDev(
29102916
stagedRendering: finalStageController,
29112917
prerenderResumeDataCache: null,
29122918
cacheSignal: null,
2919+
asyncApiPromises: createAsyncApiPromisesInDev(
2920+
initialStageController,
2921+
requestStore.cookies,
2922+
requestStore.mutableCookies,
2923+
requestStore.headers
2924+
),
29132925
dynamicTracking: createDynamicTrackingState(
29142926
false // isDebugDynamicAccesses
29152927
),
@@ -2960,6 +2972,48 @@ async function renderWithRestartOnCacheMissInDev(
29602972
}
29612973
}
29622974

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