From 65a46c7eebb731ba5c1602afef87365491beb75d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 3 Jun 2025 15:04:28 -0400 Subject: [PATCH 1/8] [Flight] Track the function name that was called for I/O entries (#33392) Stacked on #33390. The stack trace doesn't include the thing you called when calling into ignore listed content. We consider the ignore listed content conceptually the abstraction that you called that's interesting. This extracts the name of the first ignore listed function that was called from user space. For example `"fetch"`. So we can know what kind of request this is. This could be enhanced and tweaked with heuristics in the future. For example, when you create a Promise yourself and call I/O inside of it like my `delay` examples, then we use that Promise as the I/O node but its stack doesn't have the actual I/O performed. It might be better to use the inner I/O node in that case. E.g. `setTimeout`. Currently I pick the name from the first party code instead - in my example `delay`. Another case that could be improved is the case where your whole component is third-party. In that case we still log the I/O but it has no context about what kind of I/O since the whole stack is ignored it just gets the component name for example. We could for example look at the first name that is in a different package than the package name of the ignored listed component. So if `node_modules/my-component-library/index.js` calls into `node_modules/mysql/connection.js` then we could use the name from the inner. --- .../react-client/src/ReactFlightClient.js | 2 +- .../react-server/src/ReactFlightServer.js | 147 ++++++++++++------ .../src/ReactFlightStackConfigV8.js | 14 ++ .../ReactFlightAsyncDebugInfo-test.js | 11 +- packages/shared/ReactTypes.js | 1 + 5 files changed, 120 insertions(+), 55 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 2afa52f4c7f64..8d2c215a6b975 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -675,7 +675,7 @@ function nullRefGetter() { } function getIOInfoTaskName(ioInfo: ReactIOInfo): string { - return ''; // TODO + return ioInfo.name || 'unknown'; } function getAsyncInfoTaskName(asyncInfo: ReactAsyncInfo): string { diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 6a27329560882..6bdf2caf597df 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -64,6 +64,7 @@ import type { ReactAsyncInfo, ReactTimeInfo, ReactStackTrace, + ReactCallSite, ReactFunctionLocation, ReactErrorInfo, ReactErrorInfoDev, @@ -164,55 +165,73 @@ function defaultFilterStackFrame( ); } -// DEV-only cache of parsed and filtered stack frames. -const stackTraceCache: WeakMap = __DEV__ - ? new WeakMap() - : (null: any); +function devirtualizeURL(url: string): string { + if (url.startsWith('rsc://React/')) { + // This callsite is a virtual fake callsite that came from another Flight client. + // We need to reverse it back into the original location by stripping its prefix + // and suffix. We don't need the environment name because it's available on the + // parent object that will contain the stack. + const envIdx = url.indexOf('/', 12); + const suffixIdx = url.lastIndexOf('?'); + if (envIdx > -1 && suffixIdx > -1) { + return url.slice(envIdx + 1, suffixIdx); + } + } + return url; +} -function filterStackTrace( +function findCalledFunctionNameFromStackTrace( request: Request, - error: Error, - skipFrames: number, -): ReactStackTrace { - const existing = stackTraceCache.get(error); - if (existing !== undefined) { - // Return a clone because the Flight protocol isn't yet resilient to deduping - // objects in the debug info. TODO: Support deduping stacks. - const clone = existing.slice(0); - for (let i = 0; i < clone.length; i++) { - // $FlowFixMe[invalid-tuple-arity] - clone[i] = clone[i].slice(0); + stack: ReactStackTrace, +): string { + // Gets the name of the first function called from first party code. + let bestMatch = ''; + const filterStackFrame = request.filterStackFrame; + for (let i = 0; i < stack.length; i++) { + const callsite = stack[i]; + const functionName = callsite[0]; + const url = devirtualizeURL(callsite[1]); + if (filterStackFrame(url, functionName)) { + if (bestMatch === '') { + // If we had no good stack frames for internal calls, just use the last + // first party function name. + return functionName; + } + return bestMatch; + } else if (functionName === 'new Promise') { + // Ignore Promise constructors. + } else if (url === 'node:internal/async_hooks') { + // Ignore the stack frames from the async hooks themselves. + } else { + bestMatch = functionName; } - return clone; } + return ''; +} + +function filterStackTrace( + request: Request, + stack: ReactStackTrace, +): ReactStackTrace { // Since stacks can be quite large and we pass a lot of them, we filter them out eagerly // to save bandwidth even in DEV. We'll also replay these stacks on the client so by // stripping them early we avoid that overhead. Otherwise we'd normally just rely on // the DevTools or framework's ignore lists to filter them out. const filterStackFrame = request.filterStackFrame; - const stack = parseStackTrace(error, skipFrames); + const filteredStack: ReactStackTrace = []; for (let i = 0; i < stack.length; i++) { const callsite = stack[i]; const functionName = callsite[0]; - let url = callsite[1]; - if (url.startsWith('rsc://React/')) { - // This callsite is a virtual fake callsite that came from another Flight client. - // We need to reverse it back into the original location by stripping its prefix - // and suffix. We don't need the environment name because it's available on the - // parent object that will contain the stack. - const envIdx = url.indexOf('/', 12); - const suffixIdx = url.lastIndexOf('?'); - if (envIdx > -1 && suffixIdx > -1) { - url = callsite[1] = url.slice(envIdx + 1, suffixIdx); - } - } - if (!filterStackFrame(url, functionName)) { - stack.splice(i, 1); - i--; + const url = devirtualizeURL(callsite[1]); + if (filterStackFrame(url, functionName)) { + // Use a clone because the Flight protocol isn't yet resilient to deduping + // objects in the debug info. TODO: Support deduping stacks. + const clone: ReactCallSite = (callsite.slice(0): any); + clone[1] = url; + filteredStack.push(clone); } } - stackTraceCache.set(error, stack); - return stack; + return filteredStack; } initAsyncDebugInfo(); @@ -240,8 +259,7 @@ function patchConsole(consoleInst: typeof console, methodName: string) { // one stack frame but keeping it simple for now and include all frames. const stack = filterStackTrace( request, - new Error('react-stack-top-frame'), - 1, + parseStackTrace(new Error('react-stack-top-frame'), 1), ); request.pendingChunks++; const owner: null | ReactComponentInfo = resolveOwner(); @@ -1078,7 +1096,7 @@ function callWithDebugContextInDEV( componentDebugInfo.stack = task.debugStack === null ? null - : filterStackTrace(request, task.debugStack, 1); + : filterStackTrace(request, parseStackTrace(task.debugStack, 1)); // $FlowFixMe[cannot-write] componentDebugInfo.debugStack = task.debugStack; // $FlowFixMe[cannot-write] @@ -1279,7 +1297,7 @@ function renderFunctionComponent( componentDebugInfo.stack = task.debugStack === null ? null - : filterStackTrace(request, task.debugStack, 1); + : filterStackTrace(request, parseStackTrace(task.debugStack, 1)); // $FlowFixMe[cannot-write] componentDebugInfo.props = props; // $FlowFixMe[cannot-write] @@ -1615,7 +1633,7 @@ function renderClientElement( task.debugOwner, task.debugStack === null ? null - : filterStackTrace(request, task.debugStack, 1), + : filterStackTrace(request, parseStackTrace(task.debugStack, 1)), validated, ] : [REACT_ELEMENT_TYPE, type, key, props]; @@ -1748,7 +1766,7 @@ function renderElement( stack: task.debugStack === null ? null - : filterStackTrace(request, task.debugStack, 1), + : filterStackTrace(request, parseStackTrace(task.debugStack, 1)), props: props, debugStack: task.debugStack, debugTask: task.debugTask, @@ -1877,7 +1895,10 @@ function visitAsyncNode( // We don't log it yet though. We return it to be logged by the point where it's awaited. // The ioNode might be another PromiseNode in the case where none of the AwaitNode had // unfiltered stacks. - if (filterStackTrace(request, node.stack, 1).length === 0) { + if ( + filterStackTrace(request, parseStackTrace(node.stack, 1)).length === + 0 + ) { // Typically we assume that the outer most Promise that was awaited in user space has the // most actionable stack trace for the start of the operation. However, if this Promise // was created inside only third party code, then try to use the inner node instead. @@ -1898,7 +1919,10 @@ function visitAsyncNode( if (awaited !== null) { const ioNode = visitAsyncNode(request, task, awaited, cutOff, visited); if (ioNode !== null) { - const stack = filterStackTrace(request, node.stack, 1); + const stack = filterStackTrace( + request, + parseStackTrace(node.stack, 1), + ); if (stack.length === 0) { // If this await was fully filtered out, then it was inside third party code // such as in an external library. We return the I/O node and try another await. @@ -3272,7 +3296,7 @@ function emitPostponeChunk( try { // eslint-disable-next-line react-internal/safe-string-coercion reason = String(postponeInstance.message); - stack = filterStackTrace(request, postponeInstance, 0); + stack = filterStackTrace(request, parseStackTrace(postponeInstance, 0)); } catch (x) { stack = []; } @@ -3295,7 +3319,7 @@ function serializeErrorValue(request: Request, error: Error): string { name = error.name; // eslint-disable-next-line react-internal/safe-string-coercion message = String(error.message); - stack = filterStackTrace(request, error, 0); + stack = filterStackTrace(request, parseStackTrace(error, 0)); const errorEnv = (error: any).environmentName; if (typeof errorEnv === 'string') { // This probably came from another FlightClient as a pass through. @@ -3334,7 +3358,7 @@ function emitErrorChunk( name = error.name; // eslint-disable-next-line react-internal/safe-string-coercion message = String(error.message); - stack = filterStackTrace(request, error, 0); + stack = filterStackTrace(request, parseStackTrace(error, 0)); const errorEnv = (error: any).environmentName; if (typeof errorEnv === 'string') { // This probably came from another FlightClient as a pass through. @@ -3496,6 +3520,7 @@ function outlineComponentInfo( function emitIOInfoChunk( request: Request, id: number, + name: string, start: number, end: number, stack: ?ReactStackTrace, @@ -3532,6 +3557,7 @@ function emitIOInfoChunk( const relativeStartTimestamp = start - request.timeOrigin; const relativeEndTimestamp = end - request.timeOrigin; const debugIOInfo: Omit = { + name: name, start: relativeStartTimestamp, end: relativeEndTimestamp, stack: stack, @@ -3551,7 +3577,14 @@ function outlineIOInfo(request: Request, ioInfo: ReactIOInfo): void { // We can't serialize the ConsoleTask/Error objects so we need to omit them before serializing. request.pendingChunks++; const id = request.nextChunkId++; - emitIOInfoChunk(request, id, ioInfo.start, ioInfo.end, ioInfo.stack); + emitIOInfoChunk( + request, + id, + ioInfo.name, + ioInfo.start, + ioInfo.end, + ioInfo.stack, + ); request.writtenObjects.set(ioInfo, serializeByValueID(id)); } @@ -3566,12 +3599,23 @@ function serializeIONode( } let stack = null; + let name = ''; if (ioNode.stack !== null) { - stack = filterStackTrace(request, ioNode.stack, 1); + const fullStack = parseStackTrace(ioNode.stack, 1); + stack = filterStackTrace(request, fullStack); + name = findCalledFunctionNameFromStackTrace(request, fullStack); + // The name can include the object that this was called on but sometimes that's + // just unnecessary context. + if (name.startsWith('Window.')) { + name = name.slice(7); + } else if (name.startsWith('.')) { + name = name.slice(7); + } } + request.pendingChunks++; const id = request.nextChunkId++; - emitIOInfoChunk(request, id, ioNode.start, ioNode.end, stack); + emitIOInfoChunk(request, id, name, ioNode.start, ioNode.end, stack); const ref = serializeByValueID(id); request.writtenObjects.set(ioNode, ref); return ref; @@ -3712,7 +3756,10 @@ function renderConsoleValue( let debugStack: null | ReactStackTrace = null; if (element._debugStack != null) { // Outline the debug stack so that it doesn't get cut off. - debugStack = filterStackTrace(request, element._debugStack, 1); + debugStack = filterStackTrace( + request, + parseStackTrace(element._debugStack, 1), + ); doNotLimit.add(debugStack); for (let i = 0; i < debugStack.length; i++) { doNotLimit.add(debugStack[i]); diff --git a/packages/react-server/src/ReactFlightStackConfigV8.js b/packages/react-server/src/ReactFlightStackConfigV8.js index 25bc2aba9de8b..4905dfd63281e 100644 --- a/packages/react-server/src/ReactFlightStackConfigV8.js +++ b/packages/react-server/src/ReactFlightStackConfigV8.js @@ -126,10 +126,22 @@ function collectStackTrace( const frameRegExp = /^ {3} at (?:(.+) \((?:(.+):(\d+):(\d+)|\)\)|(?:async )?(.+):(\d+):(\d+)|\)$/; +// DEV-only cache of parsed and filtered stack frames. +const stackTraceCache: WeakMap = __DEV__ + ? new WeakMap() + : (null: any); + export function parseStackTrace( error: Error, skipFrames: number, ): ReactStackTrace { + // We can only get structured data out of error objects once. So we cache the information + // so we can get it again each time. It also helps performance when the same error is + // referenced more than once. + const existing = stackTraceCache.get(error); + if (existing !== undefined) { + return existing; + } // We override Error.prepareStackTrace with our own version that collects // the structured data. We need more information than the raw stack gives us // and we need to ensure that we don't get the source mapped version. @@ -148,6 +160,7 @@ export function parseStackTrace( if (collectedStackTrace !== null) { const result = collectedStackTrace; collectedStackTrace = null; + stackTraceCache.set(error, result); return result; } @@ -191,5 +204,6 @@ export function parseStackTrace( const col = +(parsed[4] || parsed[7]); parsedFrames.push([name, filename, line, col, 0, 0]); } + stackTraceCache.set(error, parsedFrames); return parsedFrames; } diff --git a/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js b/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js index 32eafab78626a..7cf6e24fdae07 100644 --- a/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js +++ b/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js @@ -170,6 +170,7 @@ describe('ReactFlightAsyncDebugInfo', () => { { "awaited": { "end": 0, + "name": "delay", "stack": [ [ "delay", @@ -220,6 +221,7 @@ describe('ReactFlightAsyncDebugInfo', () => { { "awaited": { "end": 0, + "name": "delay", "stack": [ [ "delay", @@ -321,9 +323,9 @@ describe('ReactFlightAsyncDebugInfo', () => { [ "Object.", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 291, + 293, 109, - 278, + 280, 67, ], ], @@ -331,13 +333,14 @@ describe('ReactFlightAsyncDebugInfo', () => { { "awaited": { "end": 0, + "name": "setTimeout", "stack": [ [ "Component", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 281, + 283, 7, - 279, + 281, 5, ], ], diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index 793d5dc6e25b0..15863a69cab39 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -231,6 +231,7 @@ export type ReactErrorInfo = ReactErrorInfoProd | ReactErrorInfoDev; // The point where the Async Info started which might not be the same place it was awaited. export type ReactIOInfo = { + +name: string, // the name of the async function being called (e.g. "fetch") +start: number, // the start time +end: number, // the end time (this might be different from the time the await was unblocked) +stack?: null | ReactStackTrace, From 2e9f8cd3e031212bc507e31e2888f8f96b1de138 Mon Sep 17 00:00:00 2001 From: "Sebastian \"Sebbie\" Silbermann" Date: Tue, 3 Jun 2025 21:10:13 +0200 Subject: [PATCH 2/8] Clear bundler cache before bundling fixtures (#33426) --- fixtures/fizz/package.json | 4 ++-- fixtures/flight-esm/package.json | 4 ++-- fixtures/flight/package.json | 4 ++-- fixtures/owner-stacks/package.json | 2 +- fixtures/view-transition/package.json | 6 +++--- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/fixtures/fizz/package.json b/fixtures/fizz/package.json index 096a392f4b524..7156aa9e8fa0c 100644 --- a/fixtures/fizz/package.json +++ b/fixtures/fizz/package.json @@ -28,8 +28,8 @@ "prettier": "1.19.1" }, "scripts": { - "predev": "cp -r ../../build/oss-experimental/* ./node_modules/", - "prestart": "cp -r ../../build/oss-experimental/* ./node_modules/", + "predev": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", + "prestart": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", "dev": "concurrently \"npm run dev:server\" \"npm run dev:bundler\"", "start": "concurrently \"npm run start:server\" \"npm run start:bundler\"", "dev:server": "cross-env NODE_ENV=development nodemon -- --inspect server/server.js", diff --git a/fixtures/flight-esm/package.json b/fixtures/flight-esm/package.json index cb4ca1ea30b82..de711927588ca 100644 --- a/fixtures/flight-esm/package.json +++ b/fixtures/flight-esm/package.json @@ -17,8 +17,8 @@ "webpack-sources": "^3.2.0" }, "scripts": { - "predev": "cp -r ../../build/oss-experimental/* ./node_modules/", - "prestart": "cp -r ../../build/oss-experimental/* ./node_modules/", + "predev": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", + "prestart": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", "dev": "concurrently \"npm run dev:region\" \"npm run dev:global\"", "dev:global": "NODE_ENV=development BUILD_PATH=dist node server/global", "dev:region": "NODE_ENV=development BUILD_PATH=dist nodemon --watch src --watch dist -- --enable-source-maps --experimental-loader ./loader/region.js --conditions=react-server server/region", diff --git a/fixtures/flight/package.json b/fixtures/flight/package.json index c63500727d02a..6d48c311cce3e 100644 --- a/fixtures/flight/package.json +++ b/fixtures/flight/package.json @@ -69,8 +69,8 @@ "@playwright/test": "^1.51.1" }, "scripts": { - "predev": "cp -r ../../build/oss-experimental/* ./node_modules/", - "prebuild": "cp -r ../../build/oss-experimental/* ./node_modules/", + "predev": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", + "prebuild": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", "dev": "concurrently \"npm run dev:region\" \"npm run dev:global\"", "dev:global": "NODE_ENV=development BUILD_PATH=dist node --experimental-loader ./loader/global.js --inspect=127.0.0.1:9230 server/global", "dev:region": "NODE_ENV=development BUILD_PATH=dist nodemon --watch src --watch dist -- --enable-source-maps --experimental-loader ./loader/region.js --conditions=react-server --inspect=127.0.0.1:9229 server/region", diff --git a/fixtures/owner-stacks/package.json b/fixtures/owner-stacks/package.json index cd288c8baeac7..2798aab568ecf 100644 --- a/fixtures/owner-stacks/package.json +++ b/fixtures/owner-stacks/package.json @@ -9,7 +9,7 @@ "web-vitals": "^2.1.0" }, "scripts": { - "prestart": "cp -a ../../build/oss-experimental/. node_modules", + "prestart": "cp -a ../../build/oss-experimental/. node_modules && rm -rf node_modules/.cache;", "start": "react-scripts start", "build": "react-scripts build", "test": "react-scripts test", diff --git a/fixtures/view-transition/package.json b/fixtures/view-transition/package.json index 7b9af04096027..8d222b29d3c07 100644 --- a/fixtures/view-transition/package.json +++ b/fixtures/view-transition/package.json @@ -22,9 +22,9 @@ ] }, "scripts": { - "predev": "cp -r ../../build/oss-experimental/* ./node_modules/", - "prestart": "cp -r ../../build/oss-experimental/* ./node_modules/", - "prebuild": "cp -r ../../build/oss-experimental/* ./node_modules/", + "predev": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", + "prestart": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", + "prebuild": "cp -r ../../build/oss-experimental/* ./node_modules/ && rm -rf node_modules/.cache;", "dev": "concurrently \"npm run dev:server\" \"npm run dev:client\"", "dev:client": "BROWSER=none PORT=3001 react-scripts start", "dev:server": "NODE_ENV=development node server", From d8919a0a6854715a4a77db24ed7a94a124487d86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 3 Jun 2025 15:31:12 -0400 Subject: [PATCH 3/8] [Flight] Log "Server Requests" Track (#33394) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #33392. This adds another track to the Performance Track called `"Server Requests"`. Screenshot 2025-06-01 at 12 02 14 AM This logs the flat list of I/O awaited on by Server Components. There will be other views that are more focused on what data blocks a specific Component or Suspense boundary but this is just the list of all the I/O basically so you can get an overview of those waterfalls without the noise of all the Component trees and rendering. It's similar to what the "Network" track is on the client. I've been going back and forth on what to call this track but I went with `"Server Requests"` for now. The idea is that the name should communicate that this is something that happens on the server and is a pairing with the `"Server Components"` track. Although we don't use that feature, since it's missing granularity, it's also similar to "Server Timings". --- .../react-client/src/ReactFlightClient.js | 4 ++ .../src/ReactFlightPerformanceTrack.js | 66 ++++++++++++++++++- 2 files changed, 67 insertions(+), 3 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 8d2c215a6b975..3385a3d632751 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -78,6 +78,7 @@ import { logComponentRender, logDedupedComponentRender, logComponentErrored, + logIOInfo, } from './ReactFlightPerformanceTrack'; import { @@ -2769,6 +2770,8 @@ function initializeIOInfo(response: Response, ioInfo: ReactIOInfo): void { ioInfo.start += response._timeOrigin; // $FlowFixMe[cannot-write] ioInfo.end += response._timeOrigin; + + logIOInfo(ioInfo); } function resolveIOInfo( @@ -2890,6 +2893,7 @@ function flushComponentPerformance( trackIdx, parentEndTime, previousEndTime, + response._rootEnvironmentName, ); } // Since we didn't bump the track this time, we just return the same track. diff --git a/packages/react-client/src/ReactFlightPerformanceTrack.js b/packages/react-client/src/ReactFlightPerformanceTrack.js index ed0f67a4f313e..8284ac8b6e49a 100644 --- a/packages/react-client/src/ReactFlightPerformanceTrack.js +++ b/packages/react-client/src/ReactFlightPerformanceTrack.js @@ -9,7 +9,7 @@ /* eslint-disable react-internal/no-production-logging */ -import type {ReactComponentInfo} from 'shared/ReactTypes'; +import type {ReactComponentInfo, ReactIOInfo} from 'shared/ReactTypes'; import {enableProfilerTimer} from 'shared/ReactFeatureFlags'; @@ -18,6 +18,7 @@ const supportsUserTiming = typeof console !== 'undefined' && typeof console.timeStamp === 'function'; +const IO_TRACK = 'Server Requests ⚛'; const COMPONENTS_TRACK = 'Server Components ⚛'; export function markAllTracksInOrder() { @@ -25,6 +26,14 @@ export function markAllTracksInOrder() { // Ensure we create the Server Component track groups earlier than the Client Scheduler // and Client Components. We can always add the 0 time slot even if it's in the past. // That's still considered for ordering. + console.timeStamp( + 'Server Requests Track', + 0.001, + 0.001, + IO_TRACK, + undefined, + 'primary-light', + ); console.timeStamp( 'Server Components Track', 0.001, @@ -166,9 +175,13 @@ export function logDedupedComponentRender( trackIdx: number, startTime: number, endTime: number, + rootEnv: string, ): void { if (supportsUserTiming && endTime >= 0 && trackIdx < 10) { + const env = componentInfo.env; const name = componentInfo.name; + const isPrimaryEnv = env === rootEnv; + const color = isPrimaryEnv ? 'primary-light' : 'secondary-light'; const entryName = name + ' [deduped]'; const debugTask = componentInfo.debugTask; if (__DEV__ && debugTask) { @@ -181,7 +194,7 @@ export function logDedupedComponentRender( endTime, trackNames[trackIdx], COMPONENTS_TRACK, - 'tertiary-light', + color, ), ); } else { @@ -191,7 +204,54 @@ export function logDedupedComponentRender( endTime, trackNames[trackIdx], COMPONENTS_TRACK, - 'tertiary-light', + color, + ); + } + } +} + +function getIOColor( + functionName: string, +): 'tertiary-light' | 'tertiary' | 'tertiary-dark' { + // Add some color variation to be able to distinguish various sources. + switch (functionName.charCodeAt(0) % 3) { + case 0: + return 'tertiary-light'; + case 1: + return 'tertiary'; + default: + return 'tertiary-dark'; + } +} + +export function logIOInfo(ioInfo: ReactIOInfo): void { + const startTime = ioInfo.start; + const endTime = ioInfo.end; + if (supportsUserTiming && endTime >= 0) { + const name = ioInfo.name; + const debugTask = ioInfo.debugTask; + const color = getIOColor(name); + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + name, + startTime < 0 ? 0 : startTime, + endTime, + IO_TRACK, + undefined, + color, + ), + ); + } else { + console.timeStamp( + name, + startTime < 0 ? 0 : startTime, + endTime, + IO_TRACK, + undefined, + color, ); } } From 45da4e055dc7a2b9de6abdae0709e242f8091636 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 3 Jun 2025 16:12:26 -0400 Subject: [PATCH 4/8] [Flight] Track Owner on AsyncInfo and IOInfo (#33395) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #33394. This lets us create async stack traces to the owner that was in context when the I/O was started or awaited. Screenshot 2025-06-01 at 12 31 52 AM This owner might not be the immediate closest parent where the I/O was awaited. --- .../react-client/src/ReactFlightClient.js | 10 +- .../src/ReactFlightAsyncSequence.js | 5 + .../react-server/src/ReactFlightServer.js | 25 +++- .../src/ReactFlightServerConfigDebugNode.js | 5 + .../ReactFlightAsyncDebugInfo-test.js | 140 ++++++++++++++---- packages/shared/ReactTypes.js | 2 + 6 files changed, 154 insertions(+), 33 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 3385a3d632751..9420cbcf9a8d9 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -2616,14 +2616,15 @@ function resolveDebugInfo( initializeFakeTask(response, componentInfoOrAsyncInfo, env); } if (debugInfo.owner === null && response._debugRootOwner != null) { - // $FlowFixMe[prop-missing] By narrowing `owner` to `null`, we narrowed `debugInfo` to `ReactComponentInfo` - const componentInfo: ReactComponentInfo = debugInfo; + const componentInfoOrAsyncInfo: ReactComponentInfo | ReactAsyncInfo = + // $FlowFixMe: By narrowing `owner` to `null`, we narrowed `debugInfo` to `ReactComponentInfo` + debugInfo; // $FlowFixMe[cannot-write] - componentInfo.owner = response._debugRootOwner; + componentInfoOrAsyncInfo.owner = response._debugRootOwner; // We override the stack if we override the owner since the stack where the root JSX // was created on the server isn't very useful but where the request was made is. // $FlowFixMe[cannot-write] - componentInfo.debugStack = response._debugRootStack; + componentInfoOrAsyncInfo.debugStack = response._debugRootStack; } else if (debugInfo.stack !== undefined) { const componentInfoOrAsyncInfo: ReactComponentInfo | ReactAsyncInfo = // $FlowFixMe[incompatible-type] @@ -2764,7 +2765,6 @@ function initializeIOInfo(response: Response, ioInfo: ReactIOInfo): void { initializeFakeTask(response, ioInfo, env); initializeFakeStack(response, ioInfo); } - // TODO: Initialize owner. // Adjust the time to the current environment's time space. // $FlowFixMe[cannot-write] ioInfo.start += response._timeOrigin; diff --git a/packages/react-server/src/ReactFlightAsyncSequence.js b/packages/react-server/src/ReactFlightAsyncSequence.js index 533990d827a53..91a01b0eb4cdc 100644 --- a/packages/react-server/src/ReactFlightAsyncSequence.js +++ b/packages/react-server/src/ReactFlightAsyncSequence.js @@ -7,12 +7,15 @@ * @flow */ +import type {ReactComponentInfo} from 'shared/ReactTypes'; + export const IO_NODE = 0; export const PROMISE_NODE = 1; export const AWAIT_NODE = 2; export type IONode = { tag: 0, + owner: null | ReactComponentInfo, stack: Error, // callsite that spawned the I/O start: number, // start time when the first part of the I/O sequence started end: number, // we typically don't use this. only when there's no promise intermediate. @@ -22,6 +25,7 @@ export type IONode = { export type PromiseNode = { tag: 1, + owner: null | ReactComponentInfo, stack: Error, // callsite that created the Promise start: number, // start time when the Promise was created end: number, // end time when the Promise was resolved. @@ -31,6 +35,7 @@ export type PromiseNode = { export type AwaitNode = { tag: 2, + owner: null | ReactComponentInfo, stack: Error, // callsite that awaited (using await, .then(), Promise.all(), ...) start: -1.1, // not used. We use the timing of the awaited promise. end: -1.1, // not used. diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 6bdf2caf597df..47ef1d42cc62e 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1934,6 +1934,7 @@ function visitAsyncNode( request.pendingChunks++; emitDebugChunk(request, task.id, { awaited: ((ioNode: any): ReactIOInfo), // This is deduped by this reference. + owner: node.owner, stack: stack, }); } @@ -3523,6 +3524,7 @@ function emitIOInfoChunk( name: string, start: number, end: number, + owner: ?ReactComponentInfo, stack: ?ReactStackTrace, ): void { if (!__DEV__) { @@ -3560,8 +3562,15 @@ function emitIOInfoChunk( name: name, start: relativeStartTimestamp, end: relativeEndTimestamp, - stack: stack, }; + if (stack != null) { + // $FlowFixMe[cannot-write] + debugIOInfo.stack = stack; + } + if (owner != null) { + // $FlowFixMe[cannot-write] + debugIOInfo.owner = owner; + } // $FlowFixMe[incompatible-type] stringify can return null const json: string = stringify(debugIOInfo, replacer); const row = id.toString(16) + ':J' + json + '\n'; @@ -3577,12 +3586,18 @@ function outlineIOInfo(request: Request, ioInfo: ReactIOInfo): void { // We can't serialize the ConsoleTask/Error objects so we need to omit them before serializing. request.pendingChunks++; const id = request.nextChunkId++; + const owner = ioInfo.owner; + // Ensure the owner is already outlined. + if (owner != null) { + outlineComponentInfo(request, owner); + } emitIOInfoChunk( request, id, ioInfo.name, ioInfo.start, ioInfo.end, + owner, ioInfo.stack, ); request.writtenObjects.set(ioInfo, serializeByValueID(id)); @@ -3612,10 +3627,15 @@ function serializeIONode( name = name.slice(7); } } + const owner = ioNode.owner; + // Ensure the owner is already outlined. + if (owner != null) { + outlineComponentInfo(request, owner); + } request.pendingChunks++; const id = request.nextChunkId++; - emitIOInfoChunk(request, id, name, ioNode.start, ioNode.end, stack); + emitIOInfoChunk(request, id, name, ioNode.start, ioNode.end, owner, stack); const ref = serializeByValueID(id); request.writtenObjects.set(ioNode, ref); return ref; @@ -4141,6 +4161,7 @@ function forwardDebugInfo( const debugAsyncInfo: Omit = { awaited: ioInfo, + owner: debugInfo[i].owner, stack: debugInfo[i].stack, }; emitDebugChunk(request, id, debugAsyncInfo); diff --git a/packages/react-server/src/ReactFlightServerConfigDebugNode.js b/packages/react-server/src/ReactFlightServerConfigDebugNode.js index b1e2dd4f27e9c..5412a5c410ff2 100644 --- a/packages/react-server/src/ReactFlightServerConfigDebugNode.js +++ b/packages/react-server/src/ReactFlightServerConfigDebugNode.js @@ -15,6 +15,7 @@ import type { } from './ReactFlightAsyncSequence'; import {IO_NODE, PROMISE_NODE, AWAIT_NODE} from './ReactFlightAsyncSequence'; +import {resolveOwner} from './flight/ReactFlightCurrentOwner'; import {createHook, executionAsyncId} from 'async_hooks'; import {enableAsyncDebugInfo} from 'shared/ReactFeatureFlags'; @@ -46,6 +47,7 @@ export function initAsyncDebugInfo(): void { // so that we can later pick the best stack trace in user space. node = ({ tag: AWAIT_NODE, + owner: resolveOwner(), stack: new Error(), start: -1.1, end: -1.1, @@ -55,6 +57,7 @@ export function initAsyncDebugInfo(): void { } else { node = ({ tag: PROMISE_NODE, + owner: resolveOwner(), stack: new Error(), start: performance.now(), end: -1.1, // Set when we resolve. @@ -74,6 +77,7 @@ export function initAsyncDebugInfo(): void { // We have begun a new I/O sequence. node = ({ tag: IO_NODE, + owner: resolveOwner(), stack: new Error(), // This is only used if no native promises are used. start: performance.now(), end: -1.1, // Only set when pinged. @@ -84,6 +88,7 @@ export function initAsyncDebugInfo(): void { // We have begun a new I/O sequence after the await. node = ({ tag: IO_NODE, + owner: resolveOwner(), stack: new Error(), start: performance.now(), end: -1.1, // Only set when pinged. diff --git a/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js b/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js index 7cf6e24fdae07..72857556b4c10 100644 --- a/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js +++ b/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js @@ -39,6 +39,9 @@ function normalizeIOInfo(ioInfo) { if (ioInfo.stack) { copy.stack = normalizeStack(ioInfo.stack); } + if (ioInfo.owner) { + copy.owner = normalizeDebugInfo(ioInfo.owner); + } if (typeof ioInfo.start === 'number') { copy.start = 0; } @@ -160,9 +163,9 @@ describe('ReactFlightAsyncDebugInfo', () => { [ "Object.", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 130, + 133, 109, - 117, + 120, 50, ], ], @@ -171,49 +174,83 @@ describe('ReactFlightAsyncDebugInfo', () => { "awaited": { "end": 0, "name": "delay", + "owner": { + "env": "Server", + "key": null, + "name": "Component", + "owner": null, + "props": {}, + "stack": [ + [ + "Object.", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 133, + 109, + 120, + 50, + ], + ], + }, "stack": [ [ "delay", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 112, + 115, 12, - 111, + 114, 3, ], [ "getData", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 119, + 122, 13, - 118, + 121, 5, ], [ "Component", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 126, + 129, 26, - 125, + 128, 5, ], ], "start": 0, }, + "owner": { + "env": "Server", + "key": null, + "name": "Component", + "owner": null, + "props": {}, + "stack": [ + [ + "Object.", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 133, + 109, + 120, + 50, + ], + ], + }, "stack": [ [ "getData", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 119, + 122, 13, - 118, + 121, 5, ], [ "Component", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 126, + 129, 26, - 125, + 128, 5, ], ], @@ -222,49 +259,83 @@ describe('ReactFlightAsyncDebugInfo', () => { "awaited": { "end": 0, "name": "delay", + "owner": { + "env": "Server", + "key": null, + "name": "Component", + "owner": null, + "props": {}, + "stack": [ + [ + "Object.", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 133, + 109, + 120, + 50, + ], + ], + }, "stack": [ [ "delay", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 112, + 115, 12, - 111, + 114, 3, ], [ "getData", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 120, + 123, 21, - 118, + 121, 5, ], [ "Component", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 126, + 129, 20, - 125, + 128, 5, ], ], "start": 0, }, + "owner": { + "env": "Server", + "key": null, + "name": "Component", + "owner": null, + "props": {}, + "stack": [ + [ + "Object.", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 133, + 109, + 120, + 50, + ], + ], + }, "stack": [ [ "getData", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 121, + 124, 21, - 118, + 121, 5, ], [ "Component", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 126, + 129, 20, - 125, + 128, 5, ], ], @@ -323,9 +394,9 @@ describe('ReactFlightAsyncDebugInfo', () => { [ "Object.", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 293, + 364, 109, - 280, + 351, 67, ], ], @@ -334,13 +405,30 @@ describe('ReactFlightAsyncDebugInfo', () => { "awaited": { "end": 0, "name": "setTimeout", + "owner": { + "env": "Server", + "key": null, + "name": "Component", + "owner": null, + "props": {}, + "stack": [ + [ + "Object.", + "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", + 364, + 109, + 351, + 67, + ], + ], + }, "stack": [ [ "Component", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 283, + 354, 7, - 281, + 352, 5, ], ], diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index 15863a69cab39..6d4d2104022bc 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -234,6 +234,7 @@ export type ReactIOInfo = { +name: string, // the name of the async function being called (e.g. "fetch") +start: number, // the start time +end: number, // the end time (this might be different from the time the await was unblocked) + +owner?: null | ReactComponentInfo, +stack?: null | ReactStackTrace, // Stashed Data for the Specific Execution Environment. Not part of the transport protocol +debugStack?: null | Error, @@ -242,6 +243,7 @@ export type ReactIOInfo = { export type ReactAsyncInfo = { +awaited: ReactIOInfo, + +owner?: null | ReactComponentInfo, +stack?: null | ReactStackTrace, // Stashed Data for the Specific Execution Environment. Not part of the transport protocol +debugStack?: null | Error, From 157ac578ded11352330dbdfb8cf339b28c6a16d6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 3 Jun 2025 17:28:46 -0400 Subject: [PATCH 5/8] [Flight] Include env in ReactAsyncInfo and ReactIOInfo (#33400) Stacked on #33395. This lets us keep track of which environment this was fetched and awaited. Currently the IO and await is in the same environment. It's just kept when forwarded. Once we support forwarding information from a Promise fetched from another environment and awaited in this environment then the await can end up being in a different environment. There's a question of when the await is inside Flight itself such as when you return a promise fetched from another environment whether that should mean that the await is in the current environment. I don't think so since the original stack trace is the best stack trace. It's only if you `await` it in user space in this environment first that this might happen and even then it should only be considered if there wasn't a better await earlier or if reading from the other environment was itself I/O. The timing of *when* we read `environmentName()` is a little interesting here too. --- .../react-client/src/ReactFlightClient.js | 6 ++-- .../src/ReactFlightPerformanceTrack.js | 10 +++++-- .../react-server/src/ReactFlightServer.js | 30 ++++++++++++++++++- .../ReactFlightAsyncDebugInfo-test.js | 18 +++++++---- packages/shared/ReactTypes.js | 2 ++ 5 files changed, 52 insertions(+), 14 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 9420cbcf9a8d9..38849f65b55b4 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -2758,9 +2758,7 @@ function resolveConsoleEntry( function initializeIOInfo(response: Response, ioInfo: ReactIOInfo): void { const env = - // TODO: Pass env through I/O info. - // ioInfo.env !== undefined ? ioInfo.env : - response._rootEnvironmentName; + ioInfo.env === undefined ? response._rootEnvironmentName : ioInfo.env; if (ioInfo.stack !== undefined) { initializeFakeTask(response, ioInfo, env); initializeFakeStack(response, ioInfo); @@ -2771,7 +2769,7 @@ function initializeIOInfo(response: Response, ioInfo: ReactIOInfo): void { // $FlowFixMe[cannot-write] ioInfo.end += response._timeOrigin; - logIOInfo(ioInfo); + logIOInfo(ioInfo, response._rootEnvironmentName); } function resolveIOInfo( diff --git a/packages/react-client/src/ReactFlightPerformanceTrack.js b/packages/react-client/src/ReactFlightPerformanceTrack.js index 8284ac8b6e49a..7ee214ac22a21 100644 --- a/packages/react-client/src/ReactFlightPerformanceTrack.js +++ b/packages/react-client/src/ReactFlightPerformanceTrack.js @@ -224,11 +224,15 @@ function getIOColor( } } -export function logIOInfo(ioInfo: ReactIOInfo): void { +export function logIOInfo(ioInfo: ReactIOInfo, rootEnv: string): void { const startTime = ioInfo.start; const endTime = ioInfo.end; if (supportsUserTiming && endTime >= 0) { const name = ioInfo.name; + const env = ioInfo.env; + const isPrimaryEnv = env === rootEnv; + const entryName = + isPrimaryEnv || env === undefined ? name : name + ' [' + env + ']'; const debugTask = ioInfo.debugTask; const color = getIOColor(name); if (__DEV__ && debugTask) { @@ -236,7 +240,7 @@ export function logIOInfo(ioInfo: ReactIOInfo): void { // $FlowFixMe[method-unbinding] console.timeStamp.bind( console, - name, + entryName, startTime < 0 ? 0 : startTime, endTime, IO_TRACK, @@ -246,7 +250,7 @@ export function logIOInfo(ioInfo: ReactIOInfo): void { ); } else { console.timeStamp( - name, + entryName, startTime < 0 ? 0 : startTime, endTime, IO_TRACK, diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 47ef1d42cc62e..11cb4128f929b 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1930,10 +1930,14 @@ function visitAsyncNode( } // Outline the IO node. serializeIONode(request, ioNode); + // We log the environment at the time when the last promise pigned ping which may + // be later than what the environment was when we actually started awaiting. + const env = (0, request.environmentName)(); // Then emit a reference to us awaiting it in the current task. request.pendingChunks++; emitDebugChunk(request, task.id, { awaited: ((ioNode: any): ReactIOInfo), // This is deduped by this reference. + env: env, owner: node.owner, stack: stack, }); @@ -1969,8 +1973,12 @@ function emitAsyncSequence( } serializeIONode(request, awaitedNode); request.pendingChunks++; + // We log the environment at the time when we ping which may be later than what the + // environment was when we actually started awaiting. + const env = (0, request.environmentName)(); emitDebugChunk(request, task.id, { awaited: ((awaitedNode: any): ReactIOInfo), // This is deduped by this reference. + env: env, }); } } @@ -3524,6 +3532,7 @@ function emitIOInfoChunk( name: string, start: number, end: number, + env: ?string, owner: ?ReactComponentInfo, stack: ?ReactStackTrace, ): void { @@ -3563,6 +3572,10 @@ function emitIOInfoChunk( start: relativeStartTimestamp, end: relativeEndTimestamp, }; + if (env != null) { + // $FlowFixMe[cannot-write] + debugIOInfo.env = env; + } if (stack != null) { // $FlowFixMe[cannot-write] debugIOInfo.stack = stack; @@ -3597,6 +3610,7 @@ function outlineIOInfo(request: Request, ioInfo: ReactIOInfo): void { ioInfo.name, ioInfo.start, ioInfo.end, + ioInfo.env, owner, ioInfo.stack, ); @@ -3633,9 +3647,22 @@ function serializeIONode( outlineComponentInfo(request, owner); } + // We log the environment at the time when we serialize the I/O node. + // The environment name may have changed from when the I/O was actually started. + const env = (0, request.environmentName)(); + request.pendingChunks++; const id = request.nextChunkId++; - emitIOInfoChunk(request, id, name, ioNode.start, ioNode.end, owner, stack); + emitIOInfoChunk( + request, + id, + name, + ioNode.start, + ioNode.end, + env, + owner, + stack, + ); const ref = serializeByValueID(id); request.writtenObjects.set(ioNode, ref); return ref; @@ -4161,6 +4188,7 @@ function forwardDebugInfo( const debugAsyncInfo: Omit = { awaited: ioInfo, + env: debugInfo[i].env, owner: debugInfo[i].owner, stack: debugInfo[i].stack, }; diff --git a/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js b/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js index 72857556b4c10..68a9cdb3b1558 100644 --- a/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js +++ b/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js @@ -173,6 +173,7 @@ describe('ReactFlightAsyncDebugInfo', () => { { "awaited": { "end": 0, + "env": "Server", "name": "delay", "owner": { "env": "Server", @@ -219,6 +220,7 @@ describe('ReactFlightAsyncDebugInfo', () => { ], "start": 0, }, + "env": "Server", "owner": { "env": "Server", "key": null, @@ -258,6 +260,7 @@ describe('ReactFlightAsyncDebugInfo', () => { { "awaited": { "end": 0, + "env": "Server", "name": "delay", "owner": { "env": "Server", @@ -304,6 +307,7 @@ describe('ReactFlightAsyncDebugInfo', () => { ], "start": 0, }, + "env": "Server", "owner": { "env": "Server", "key": null, @@ -394,9 +398,9 @@ describe('ReactFlightAsyncDebugInfo', () => { [ "Object.", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 364, + 368, 109, - 351, + 355, 67, ], ], @@ -404,6 +408,7 @@ describe('ReactFlightAsyncDebugInfo', () => { { "awaited": { "end": 0, + "env": "Server", "name": "setTimeout", "owner": { "env": "Server", @@ -415,9 +420,9 @@ describe('ReactFlightAsyncDebugInfo', () => { [ "Object.", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 364, + 368, 109, - 351, + 355, 67, ], ], @@ -426,14 +431,15 @@ describe('ReactFlightAsyncDebugInfo', () => { [ "Component", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 354, + 358, 7, - 352, + 356, 5, ], ], "start": 0, }, + "env": "Server", }, { "time": 0, diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index 6d4d2104022bc..b2b2f25fbaa93 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -234,6 +234,7 @@ export type ReactIOInfo = { +name: string, // the name of the async function being called (e.g. "fetch") +start: number, // the start time +end: number, // the end time (this might be different from the time the await was unblocked) + +env?: string, // the environment where this I/O was spawned. +owner?: null | ReactComponentInfo, +stack?: null | ReactStackTrace, // Stashed Data for the Specific Execution Environment. Not part of the transport protocol @@ -243,6 +244,7 @@ export type ReactIOInfo = { export type ReactAsyncInfo = { +awaited: ReactIOInfo, + +env?: string, // the environment where this was awaited. This might not be the same as where it was spawned. +owner?: null | ReactComponentInfo, +stack?: null | ReactStackTrace, // Stashed Data for the Specific Execution Environment. Not part of the transport protocol From 9cc74fec749bcca2e0f5d1e41aa612b2135641ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 3 Jun 2025 17:29:41 -0400 Subject: [PATCH 6/8] [Flight] Emit the time we awaited something inside a Server Component (#33402) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Stacked on #33400. Screenshot 2025-06-01 at 10 27 47 PM This is emitted with the start/end time and stack of the "await". Which may be different than the thing that started the I/O. These awaits aren't quite as simple as just every await since you can start a sequence in parallel there can actually be multiple overlapping awaits and there can be CPU work interleaved with the await on the same component. ```js function getData() { await fetch(...); await fetch(...); } const promise = getData(); doWork(); await promise; ``` This has two "I/O" awaits but those are actually happening in parallel with `doWork()`. Since these also could have started before we started rendering this sequence (e.g. a component) we have to clamp it so that we don't consider awaits that start before the component. What we're conceptually trying to convey is the time this component was blocked due to that I/O resource. Whether it's blocked from completing the last result or if it's blocked from issuing a waterfall request. --- .../react-client/src/ReactFlightClient.js | 33 +++++++++++-- .../src/ReactFlightPerformanceTrack.js | 48 ++++++++++++++++++- .../src/ReactFlightAsyncSequence.js | 4 +- .../react-server/src/ReactFlightServer.js | 32 +++++++++++++ .../src/ReactFlightServerConfigDebugNode.js | 10 ++-- .../ReactFlightAsyncDebugInfo-test.js | 30 +++++++++--- 6 files changed, 138 insertions(+), 19 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 38849f65b55b4..bc5f017b59ff8 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -79,6 +79,7 @@ import { logDedupedComponentRender, logComponentErrored, logIOInfo, + logComponentAwait, } from './ReactFlightPerformanceTrack'; import { @@ -680,7 +681,7 @@ function getIOInfoTaskName(ioInfo: ReactIOInfo): string { } function getAsyncInfoTaskName(asyncInfo: ReactAsyncInfo): string { - return 'await'; // We could be smarter about this and give it a name like `then` or `Promise.all`. + return 'await ' + getIOInfoTaskName(asyncInfo.awaited); } function getServerComponentTaskName(componentInfo: ReactComponentInfo): string { @@ -2971,9 +2972,12 @@ function flushComponentPerformance( for (let i = debugInfo.length - 1; i >= 0; i--) { const info = debugInfo[i]; if (typeof info.time === 'number') { - endTime = info.time; - if (endTime > childrenEndTime) { - childrenEndTime = endTime; + if (info.time > childrenEndTime) { + childrenEndTime = info.time; + } + if (endTime === 0) { + // Last timestamp is the end of the last component. + endTime = info.time; } } if (typeof info.name === 'string' && i > 0) { @@ -3011,8 +3015,29 @@ function flushComponentPerformance( } // Track the root most component of the result for deduping logging. result.component = componentInfo; + // Set the end time of the previous component to the start of the previous. + endTime = startTime; } isLastComponent = false; + } else if (info.awaited && i > 0 && i < debugInfo.length - 2) { + // $FlowFixMe: Refined. + const asyncInfo: ReactAsyncInfo = info; + const startTimeInfo = debugInfo[i - 1]; + const endTimeInfo = debugInfo[i + 1]; + if ( + typeof startTimeInfo.time === 'number' && + typeof endTimeInfo.time === 'number' + ) { + const awaitStartTime = startTimeInfo.time; + const awaitEndTime = endTimeInfo.time; + logComponentAwait( + asyncInfo, + trackIdx, + awaitStartTime, + awaitEndTime, + response._rootEnvironmentName, + ); + } } } } diff --git a/packages/react-client/src/ReactFlightPerformanceTrack.js b/packages/react-client/src/ReactFlightPerformanceTrack.js index 7ee214ac22a21..faa5cf9650d9c 100644 --- a/packages/react-client/src/ReactFlightPerformanceTrack.js +++ b/packages/react-client/src/ReactFlightPerformanceTrack.js @@ -9,7 +9,11 @@ /* eslint-disable react-internal/no-production-logging */ -import type {ReactComponentInfo, ReactIOInfo} from 'shared/ReactTypes'; +import type { + ReactComponentInfo, + ReactIOInfo, + ReactAsyncInfo, +} from 'shared/ReactTypes'; import {enableProfilerTimer} from 'shared/ReactFeatureFlags'; @@ -224,6 +228,48 @@ function getIOColor( } } +export function logComponentAwait( + asyncInfo: ReactAsyncInfo, + trackIdx: number, + startTime: number, + endTime: number, + rootEnv: string, +): void { + if (supportsUserTiming && endTime > 0) { + const env = asyncInfo.env; + const name = asyncInfo.awaited.name; + const isPrimaryEnv = env === rootEnv; + const color = getIOColor(name); + const entryName = + 'await ' + + (isPrimaryEnv || env === undefined ? name : name + ' [' + env + ']'); + const debugTask = asyncInfo.debugTask; + if (__DEV__ && debugTask) { + debugTask.run( + // $FlowFixMe[method-unbinding] + console.timeStamp.bind( + console, + entryName, + startTime < 0 ? 0 : startTime, + endTime, + trackNames[trackIdx], + COMPONENTS_TRACK, + color, + ), + ); + } else { + console.timeStamp( + entryName, + startTime < 0 ? 0 : startTime, + endTime, + trackNames[trackIdx], + COMPONENTS_TRACK, + color, + ); + } + } +} + export function logIOInfo(ioInfo: ReactIOInfo, rootEnv: string): void { const startTime = ioInfo.start; const endTime = ioInfo.end; diff --git a/packages/react-server/src/ReactFlightAsyncSequence.js b/packages/react-server/src/ReactFlightAsyncSequence.js index 91a01b0eb4cdc..b36fb551091ee 100644 --- a/packages/react-server/src/ReactFlightAsyncSequence.js +++ b/packages/react-server/src/ReactFlightAsyncSequence.js @@ -37,8 +37,8 @@ export type AwaitNode = { tag: 2, owner: null | ReactComponentInfo, stack: Error, // callsite that awaited (using await, .then(), Promise.all(), ...) - start: -1.1, // not used. We use the timing of the awaited promise. - end: -1.1, // not used. + start: number, // when we started blocking. This might be later than the I/O started. + end: number, // when we unblocked. This might be later than the I/O resolved if there's CPU time. awaited: null | AsyncSequence, // the promise we were waiting on previous: null | AsyncSequence, // the sequence that was blocking us from awaiting in the first place }; diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 11cb4128f929b..e5c6043652c85 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -1919,6 +1919,24 @@ function visitAsyncNode( if (awaited !== null) { const ioNode = visitAsyncNode(request, task, awaited, cutOff, visited); if (ioNode !== null) { + if (node.end < 0) { + // If we haven't defined an end time, use the resolve of the inner Promise. + // This can happen because the ping gets invoked before the await gets resolved. + if (ioNode.end < node.start) { + // If we're awaiting a resolved Promise it could have finished before we started. + node.end = node.start; + } else { + node.end = ioNode.end; + } + } + if (node.end < cutOff) { + // This was already resolved when we started this sequence. It must have been + // part of a different component. + // TODO: Think of some other way to exclude irrelevant data since if we awaited + // a cached promise, we should still log this component as being dependent on that data. + return null; + } + const stack = filterStackTrace( request, parseStackTrace(node.stack, 1), @@ -1933,6 +1951,15 @@ function visitAsyncNode( // We log the environment at the time when the last promise pigned ping which may // be later than what the environment was when we actually started awaiting. const env = (0, request.environmentName)(); + if (node.start <= cutOff) { + // If this was an await that started before this sequence but finished after, + // then we clamp it to the start of this sequence. We don't need to emit a time + // TODO: Typically we'll already have a previous time stamp with the cutOff time + // so we shouldn't need to emit another one. But not always. + emitTimingChunk(request, task.id, cutOff); + } else { + emitTimingChunk(request, task.id, node.start); + } // Then emit a reference to us awaiting it in the current task. request.pendingChunks++; emitDebugChunk(request, task.id, { @@ -1941,6 +1968,7 @@ function visitAsyncNode( owner: node.owner, stack: stack, }); + emitTimingChunk(request, task.id, node.end); } } // If we had awaited anything we would have written it now. @@ -1976,10 +2004,14 @@ function emitAsyncSequence( // We log the environment at the time when we ping which may be later than what the // environment was when we actually started awaiting. const env = (0, request.environmentName)(); + // If we don't have any thing awaited, the time we started awaiting was internal + // when we yielded after rendering. The cutOff time is basically that. + emitTimingChunk(request, task.id, cutOff); emitDebugChunk(request, task.id, { awaited: ((awaitedNode: any): ReactIOInfo), // This is deduped by this reference. env: env, }); + emitTimingChunk(request, task.id, awaitedNode.end); } } diff --git a/packages/react-server/src/ReactFlightServerConfigDebugNode.js b/packages/react-server/src/ReactFlightServerConfigDebugNode.js index 5412a5c410ff2..7a86231ed9a5c 100644 --- a/packages/react-server/src/ReactFlightServerConfigDebugNode.js +++ b/packages/react-server/src/ReactFlightServerConfigDebugNode.js @@ -49,8 +49,8 @@ export function initAsyncDebugInfo(): void { tag: AWAIT_NODE, owner: resolveOwner(), stack: new Error(), - start: -1.1, - end: -1.1, + start: performance.now(), + end: -1.1, // set when resolved. awaited: trigger, // The thing we're awaiting on. Might get overrriden when we resolve. previous: current === undefined ? null : current, // The path that led us here. }: AwaitNode); @@ -118,10 +118,8 @@ export function initAsyncDebugInfo(): void { 'A Promise should never be an IO_NODE. This is a bug in React.', ); } - if (resolvedNode.tag === PROMISE_NODE) { - // Log the end time when we resolved the promise. - resolvedNode.end = performance.now(); - } + // Log the end time when we resolved the promise. + resolvedNode.end = performance.now(); const currentAsyncId = executionAsyncId(); if (asyncId !== currentAsyncId) { // If the promise was not resolved by itself, then that means that diff --git a/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js b/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js index 68a9cdb3b1558..da2fc898b93b5 100644 --- a/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js +++ b/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js @@ -170,6 +170,9 @@ describe('ReactFlightAsyncDebugInfo', () => { ], ], }, + { + "time": 0, + }, { "awaited": { "end": 0, @@ -257,6 +260,12 @@ describe('ReactFlightAsyncDebugInfo', () => { ], ], }, + { + "time": 0, + }, + { + "time": 0, + }, { "awaited": { "end": 0, @@ -347,6 +356,9 @@ describe('ReactFlightAsyncDebugInfo', () => { { "time": 0, }, + { + "time": 0, + }, ] `); } @@ -398,13 +410,16 @@ describe('ReactFlightAsyncDebugInfo', () => { [ "Object.", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 368, + 380, 109, - 355, + 367, 67, ], ], }, + { + "time": 0, + }, { "awaited": { "end": 0, @@ -420,9 +435,9 @@ describe('ReactFlightAsyncDebugInfo', () => { [ "Object.", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 368, + 380, 109, - 355, + 367, 67, ], ], @@ -431,9 +446,9 @@ describe('ReactFlightAsyncDebugInfo', () => { [ "Component", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 358, + 370, 7, - 356, + 368, 5, ], ], @@ -444,6 +459,9 @@ describe('ReactFlightAsyncDebugInfo', () => { { "time": 0, }, + { + "time": 0, + }, ] `); } From 154008172573d64519ebbc23da611a27073b0a8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 3 Jun 2025 17:30:31 -0400 Subject: [PATCH 7/8] [Flight] Encode Async I/O Tasks using the Enclosing Line/Column (#33403) Stacked on #33402. There's a bug in Chrome Performance tracking which uses the enclosing line/column instead of the callsite in stacks. For our fake eval:ed functions that represents functions on the server, we can position the enclosing function body at the position of the callsite to simulate getting the right line. Unfortunately, that doesn't give us exactly the right callsite when it's used for other purposes that uses the callsite like console logs and error reporting and stacks inside breakpoints. So I don't think we want to always do this. For ReactAsyncInfo/ReactIOInfo, the only thing we're going to use the fake task for is the Performance tracking, so it doesn't have any downsides until Chrome fixes the bug and we'd have to revert it. Therefore this PR uses that techniques only for those entries. We could do this for Server Components too but we're going to use those for other things too like console logs. I don't think it's worth duplicating the Task objects. That would also make it inconsistent with Client Components. For Client Components, we could in theory also generate fake evals but that would be way slower since there's so many of them and currently we rely on the native implementation for those. So doesn't seem worth fixing. But since we can at least fix it for RSC I/O/awaits we can do this hack. --- .../react-client/src/ReactFlightClient.js | 43 ++++++++++++++++--- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index bc5f017b59ff8..41421edb2a32a 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -813,7 +813,13 @@ function createElement( console, getTaskName(type), ); - const callStack = buildFakeCallStack(response, stack, env, createTaskFn); + const callStack = buildFakeCallStack( + response, + stack, + env, + false, + createTaskFn, + ); // This owner should ideally have already been initialized to avoid getting // user stack frames on the stack. const ownerTask = @@ -2134,6 +2140,7 @@ function resolveErrorDev( response, stack, env, + false, // $FlowFixMe[incompatible-use] Error.bind( null, @@ -2196,6 +2203,7 @@ function resolvePostponeDev( response, stack, env, + false, // $FlowFixMe[incompatible-use] Error.bind(null, reason || ''), ); @@ -2404,12 +2412,17 @@ function buildFakeCallStack( response: Response, stack: ReactStackTrace, environmentName: string, + useEnclosingLine: boolean, innerCall: () => T, ): () => T { let callStack = innerCall; for (let i = 0; i < stack.length; i++) { const frame = stack[i]; - const frameKey = frame.join('-') + '-' + environmentName; + const frameKey = + frame.join('-') + + '-' + + environmentName + + (useEnclosingLine ? '-e' : '-n'); let fn = fakeFunctionCache.get(frameKey); if (fn === undefined) { const [name, filename, line, col, enclosingLine, enclosingCol] = frame; @@ -2423,8 +2436,8 @@ function buildFakeCallStack( sourceMap, line, col, - enclosingLine, - enclosingCol, + useEnclosingLine ? line : enclosingLine, + useEnclosingLine ? col : enclosingCol, environmentName, ); // TODO: This cache should technically live on the response since the _debugFindSourceMapURL @@ -2470,6 +2483,15 @@ function initializeFakeTask( // If it's null, we can't initialize a task. return null; } + + // Workaround for a bug where Chrome Performance tracking uses the enclosing line/column + // instead of the callsite. For ReactAsyncInfo/ReactIOInfo, the only thing we're going + // to use the fake task for is the Performance tracking so we encode the enclosing line/ + // column at the callsite to get a better line number. We could do this for Components too + // but we're going to use those for other things too like console logs and it's not worth + // duplicating. If this bug is every fixed in Chrome, this should be set to false. + const useEnclosingLine = debugInfo.key === undefined; + const stack = debugInfo.stack; const env: string = debugInfo.env == null ? response._rootEnvironmentName : debugInfo.env; @@ -2486,6 +2508,7 @@ function initializeFakeTask( stack, '"use ' + childEnvironmentName.toLowerCase() + '"', env, + useEnclosingLine, ); } else { const cachedEntry = debugInfo.debugTask; @@ -2510,6 +2533,7 @@ function initializeFakeTask( stack, taskName, env, + useEnclosingLine, )); } } @@ -2520,9 +2544,16 @@ function buildFakeTask( stack: ReactStackTrace, taskName: string, env: string, + useEnclosingLine: boolean, ): ConsoleTask { const createTaskFn = (console: any).createTask.bind(console, taskName); - const callStack = buildFakeCallStack(response, stack, env, createTaskFn); + const callStack = buildFakeCallStack( + response, + stack, + env, + useEnclosingLine, + createTaskFn, + ); if (ownerTask === null) { const rootTask = getRootTask(response, env); if (rootTask != null) { @@ -2545,6 +2576,7 @@ const createFakeJSXCallStack = { response, stack, environmentName, + false, fakeJSXCallSite, ); return callStackForError(); @@ -2681,6 +2713,7 @@ const replayConsoleWithCallStack = { response, stackTrace, env, + false, bindToConsole(methodName, args, env), ); if (owner != null) { From d742611ce40545127032f4e221c78bf9f70eb437 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Tue, 3 Jun 2025 17:40:30 -0400 Subject: [PATCH 8/8] Replace Implicit Options on SuspenseList with Explicit Options (#33424) We want to change the defaults for `revealOrder` and `tail` on SuspenseList. This is an intermediate step to allow experimental users to upgrade. To explicitly specify these options I added `revealOrder="independent"` and `tail="visible"`. I then added warnings if `undefined` or `null` is passed. You must now always explicitly specify them. However, semantics are still preserved for now until the next step. We also want to change the rendering order of the `children` prop for `revealOrder="backwards"`. As an intermediate step I first added `revealOrder="unstable_legacy-backwards"` option. This will only be temporary until all users can switch to the new `"backwards"` semantics once we flip it in the next step. I also clarified the types that the directional props requires iterable children but not iterable inside of those. Rows with multiple items can be modeled as explicit fragments. --- .eslintrc.js | 1 + fixtures/ssr/src/components/LargeContent.js | 2 +- .../src/__tests__/ReactDOMFizzServer-test.js | 2 +- .../ReactDOMFizzStaticBrowser-test.js | 2 +- .../ReactDOMFizzSuspenseList-test.js | 84 +++++- .../src/__tests__/ReactDOMFloat-test.js | 2 +- ...DOMServerPartialHydration-test.internal.js | 2 +- .../ReactDOMServerSuspense-test.internal.js | 2 +- .../__tests__/ReactWrongReturnPointer-test.js | 2 +- .../react-reconciler/src/ReactChildFiber.js | 4 +- .../src/ReactFiberBeginWork.js | 68 +++-- .../src/ReactFiberSuspenseComponent.js | 7 +- .../__tests__/ReactContextPropagation-test.js | 2 +- .../src/__tests__/ReactErrorStacks-test.js | 2 +- .../src/__tests__/ReactSuspenseList-test.js | 240 ++++++++++++++++-- .../ReactSuspenseyCommitPhase-test.js | 2 +- packages/react-server/src/ReactFizzServer.js | 25 +- packages/shared/ReactTypes.js | 20 +- 18 files changed, 397 insertions(+), 72 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index 1bb4e868d0694..8ffc5a732c877 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -611,6 +611,7 @@ module.exports = { TimeoutID: 'readonly', WheelEventHandler: 'readonly', FinalizationRegistry: 'readonly', + Exclude: 'readonly', Omit: 'readonly', Keyframe: 'readonly', PropertyIndexedKeyframes: 'readonly', diff --git a/fixtures/ssr/src/components/LargeContent.js b/fixtures/ssr/src/components/LargeContent.js index 7c4a6cf2258ef..f5c8adb03e233 100644 --- a/fixtures/ssr/src/components/LargeContent.js +++ b/fixtures/ssr/src/components/LargeContent.js @@ -6,7 +6,7 @@ import React, { export default function LargeContent() { return ( - +

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Mauris diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index d5f635f964934..57124ec6e0c0e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1289,7 +1289,7 @@ describe('ReactDOMFizzServer', () => { function App({showMore}) { return (

- + {a} {b} {showMore ? ( diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js index 478bed90a3a0d..c306b1369b174 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js @@ -2254,7 +2254,7 @@ describe('ReactDOMFizzStaticBrowser', () => { function App() { return (
- + diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js index 977d2dbf154f4..74db51d242960 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js @@ -197,6 +197,70 @@ describe('ReactDOMFizzSuspenseList', () => { ); }); + // @gate enableSuspenseList + it('independently with revealOrder="independent"', async () => { + const A = createAsyncText('A'); + const B = createAsyncText('B'); + const C = createAsyncText('C'); + + function Foo() { + return ( + + ); + } + + await A.resolve(); + + await serverAct(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(); + pipe(writable); + }); + + assertLog(['A', 'Suspend! [B]', 'Suspend! [C]', 'Loading B', 'Loading C']); + + expect(getVisibleChildren(container)).toEqual( +
+ A + Loading B + Loading C +
, + ); + + await serverAct(() => C.resolve()); + assertLog(['C']); + + expect(getVisibleChildren(container)).toEqual( +
+ A + Loading B + C +
, + ); + + await serverAct(() => B.resolve()); + assertLog(['B']); + + expect(getVisibleChildren(container)).toEqual( +
+ A + B + C +
, + ); + }); + // @gate enableSuspenseList it('displays all "together"', async () => { const A = createAsyncText('A'); @@ -452,7 +516,7 @@ describe('ReactDOMFizzSuspenseList', () => { }); // @gate enableSuspenseList - it('displays all "together" in nested SuspenseLists where the inner is default', async () => { + it('displays all "together" in nested SuspenseLists where the inner is "independent"', async () => { const A = createAsyncText('A'); const B = createAsyncText('B'); const C = createAsyncText('C'); @@ -464,7 +528,7 @@ describe('ReactDOMFizzSuspenseList', () => { }> - + }> @@ -523,7 +587,7 @@ describe('ReactDOMFizzSuspenseList', () => { function Foo() { return (
- + }> @@ -586,7 +650,7 @@ describe('ReactDOMFizzSuspenseList', () => { }); // @gate enableSuspenseList - it('displays each items in "backwards" order', async () => { + it('displays each items in "backwards" order in legacy mode', async () => { const A = createAsyncText('A'); const B = createAsyncText('B'); const C = createAsyncText('C'); @@ -594,7 +658,7 @@ describe('ReactDOMFizzSuspenseList', () => { function Foo() { return (
- + }> @@ -665,8 +729,10 @@ describe('ReactDOMFizzSuspenseList', () => { function Foo() { return (
- - + + }> @@ -736,7 +802,7 @@ describe('ReactDOMFizzSuspenseList', () => { function Foo() { return (
- + }> @@ -791,7 +857,7 @@ describe('ReactDOMFizzSuspenseList', () => { function Foo() { return (
- + }> diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index 6436f1898b798..b8a4e5b86ae91 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -5755,7 +5755,7 @@ body { - + diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 8cace332cd552..5b719ddf4acc5 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -2362,7 +2362,7 @@ describe('ReactDOMServerPartialHydration', () => { function App({showMore}) { return ( - + {a} {b} {showMore ? ( diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js index 8a959a294987c..6e6f9bb09261e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSuspense-test.internal.js @@ -123,7 +123,7 @@ describe('ReactDOMServerSuspense', () => { // @gate enableSuspenseList it('server renders a SuspenseList component and its children', async () => { const example = ( - +
A
 diff --git a/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js b/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js index e221f01b92d65..0c73410f2edf0 100644 --- a/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js +++ b/packages/react-dom/src/__tests__/ReactWrongReturnPointer-test.js @@ -172,7 +172,7 @@ test('regression (#20932): return pointer is correct before entering deleted tre function App() { return ( - + }> diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index fcb2406552899..f574162b41b0c 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -2097,7 +2097,9 @@ export function validateSuspenseListChildren( ) { if (__DEV__) { if ( - (revealOrder === 'forwards' || revealOrder === 'backwards') && + (revealOrder === 'forwards' || + revealOrder === 'backwards' || + revealOrder === 'unstable_legacy-backwards') && children !== undefined && children !== null && children !== false diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 7b86962f778fe..a9316163156f5 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -337,7 +337,7 @@ if (__DEV__) { didWarnAboutContextTypes = ({}: {[string]: boolean}); didWarnAboutGetDerivedStateOnFunctionComponent = ({}: {[string]: boolean}); didWarnAboutReassigningProps = false; - didWarnAboutRevealOrder = ({}: {[empty]: boolean}); + didWarnAboutRevealOrder = ({}: {[string]: boolean}); didWarnAboutTailOptions = ({}: {[string]: boolean}); didWarnAboutDefaultPropsOnFunctionComponent = ({}: {[string]: boolean}); didWarnAboutClassNameOnViewTransition = ({}: {[string]: boolean}); @@ -3225,19 +3225,32 @@ function findLastContentRow(firstChild: null | Fiber): null | Fiber { function validateRevealOrder(revealOrder: SuspenseListRevealOrder) { if (__DEV__) { + const cacheKey = revealOrder == null ? 'null' : revealOrder; if ( - revealOrder !== undefined && revealOrder !== 'forwards' && - revealOrder !== 'backwards' && + revealOrder !== 'unstable_legacy-backwards' && revealOrder !== 'together' && - !didWarnAboutRevealOrder[revealOrder] + revealOrder !== 'independent' && + !didWarnAboutRevealOrder[cacheKey] ) { - didWarnAboutRevealOrder[revealOrder] = true; - if (typeof revealOrder === 'string') { + didWarnAboutRevealOrder[cacheKey] = true; + if (revealOrder == null) { + console.error( + 'The default for the prop is changing. ' + + 'To be future compatible you must explictly specify either ' + + '"independent" (the current default), "together", "forwards" or "legacy_unstable-backwards".', + ); + } else if (revealOrder === 'backwards') { + console.error( + 'The rendering order of is changing. ' + + 'To be future compatible you must specify revealOrder="legacy_unstable-backwards" instead.', + ); + } else if (typeof revealOrder === 'string') { switch (revealOrder.toLowerCase()) { case 'together': case 'forwards': - case 'backwards': { + case 'backwards': + case 'independent': { console.error( '"%s" is not a valid value for revealOrder on . ' + 'Use lowercase "%s" instead.', @@ -3259,7 +3272,7 @@ function validateRevealOrder(revealOrder: SuspenseListRevealOrder) { default: console.error( '"%s" is not a supported revealOrder on . ' + - 'Did you mean "together", "forwards" or "backwards"?', + 'Did you mean "independent", "together", "forwards" or "backwards"?', revealOrder, ); break; @@ -3267,7 +3280,7 @@ function validateRevealOrder(revealOrder: SuspenseListRevealOrder) { } else { console.error( '%s is not a supported value for revealOrder on . ' + - 'Did you mean "together", "forwards" or "backwards"?', + 'Did you mean "independent", "together", "forwards" or "backwards"?', revealOrder, ); } @@ -3280,16 +3293,38 @@ function validateTailOptions( revealOrder: SuspenseListRevealOrder, ) { if (__DEV__) { - if (tailMode !== undefined && !didWarnAboutTailOptions[tailMode]) { - if (tailMode !== 'collapsed' && tailMode !== 'hidden') { - didWarnAboutTailOptions[tailMode] = true; + const cacheKey = tailMode == null ? 'null' : tailMode; + if (!didWarnAboutTailOptions[cacheKey]) { + if (tailMode == null) { + if ( + revealOrder === 'forwards' || + revealOrder === 'backwards' || + revealOrder === 'unstable_legacy-backwards' + ) { + didWarnAboutTailOptions[cacheKey] = true; + console.error( + 'The default for the prop is changing. ' + + 'To be future compatible you must explictly specify either ' + + '"visible" (the current default), "collapsed" or "hidden".', + ); + } + } else if ( + tailMode !== 'visible' && + tailMode !== 'collapsed' && + tailMode !== 'hidden' + ) { + didWarnAboutTailOptions[cacheKey] = true; console.error( '"%s" is not a supported value for tail on . ' + - 'Did you mean "collapsed" or "hidden"?', + 'Did you mean "visible", "collapsed" or "hidden"?', tailMode, ); - } else if (revealOrder !== 'forwards' && revealOrder !== 'backwards') { - didWarnAboutTailOptions[tailMode] = true; + } else if ( + revealOrder !== 'forwards' && + revealOrder !== 'backwards' && + revealOrder !== 'unstable_legacy-backwards' + ) { + didWarnAboutTailOptions[cacheKey] = true; console.error( ' is only valid if revealOrder is ' + '"forwards" or "backwards". ' + @@ -3414,7 +3449,8 @@ function updateSuspenseListComponent( ); break; } - case 'backwards': { + case 'backwards': + case 'unstable_legacy-backwards': { // We're going to find the first row that has existing content. // At the same time we're going to reverse the list of everything // we pass in the meantime. That's going to be our tail in reverse diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js index f7ae78ed6d313..2542d660c4c47 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js @@ -75,9 +75,12 @@ export function findFirstSuspended(row: Fiber): null | Fiber { } } else if ( node.tag === SuspenseListComponent && - // revealOrder undefined can't be trusted because it don't + // Independent revealOrder can't be trusted because it doesn't // keep track of whether it suspended or not. - node.memoizedProps.revealOrder !== undefined + (node.memoizedProps.revealOrder === 'forwards' || + node.memoizedProps.revealOrder === 'backwards' || + node.memoizedProps.revealOrder === 'unstable_legacy-backwards' || + node.memoizedProps.revealOrder === 'together') ) { const didSuspend = (node.flags & DidCapture) !== NoFlags; if (didSuspend) { diff --git a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js index 6ed19ba6d505b..0a338c8aa8710 100644 --- a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js +++ b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js @@ -677,7 +677,7 @@ describe('ReactLazyContextPropagation', () => { setContext = setValue; const children = React.useMemo( () => ( - + diff --git a/packages/react-reconciler/src/__tests__/ReactErrorStacks-test.js b/packages/react-reconciler/src/__tests__/ReactErrorStacks-test.js index 1b8beba58b07e..9e4e9694a6a88 100644 --- a/packages/react-reconciler/src/__tests__/ReactErrorStacks-test.js +++ b/packages/react-reconciler/src/__tests__/ReactErrorStacks-test.js @@ -255,7 +255,7 @@ describe('ReactFragment', () => { onCaughtError, }).render( - + , diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js index f9efb330cf891..bfbf165dbaba6 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js @@ -79,7 +79,7 @@ describe('ReactSuspenseList', () => { }); assertConsoleErrorDev([ '"something" is not a supported revealOrder on ' + - '. Did you mean "together", "forwards" or "backwards"?' + + '. Did you mean "independent", "together", "forwards" or "backwards"?' + '\n in SuspenseList (at **)' + '\n in Foo (at **)', ]); @@ -131,7 +131,11 @@ describe('ReactSuspenseList', () => { // @gate enableSuspenseList it('warns if a single element is passed to a "forwards" list', async () => { function Foo({children}) { - return {children}; + return ( + + {children} + + ); } ReactNoop.render(); @@ -166,7 +170,7 @@ describe('ReactSuspenseList', () => { it('warns if a single fragment is passed to a "backwards" list', async () => { function Foo() { return ( - + <>{[]} ); @@ -176,7 +180,7 @@ describe('ReactSuspenseList', () => { ReactNoop.render(); }); assertConsoleErrorDev([ - 'A single row was passed to a . ' + + 'A single row was passed to a . ' + 'This is not useful since it needs multiple rows. ' + 'Did you mean to pass multiple children or an array?' + '\n in SuspenseList (at **)' + @@ -188,7 +192,7 @@ describe('ReactSuspenseList', () => { it('warns if a nested array is passed to a "forwards" list', async () => { function Foo({items}) { return ( - + {items.map(name => ( {name} @@ -214,7 +218,7 @@ describe('ReactSuspenseList', () => { }); // @gate enableSuspenseList - it('shows content independently by default', async () => { + it('warns if no revealOrder is specified', async () => { const A = createAsyncText('A'); const B = createAsyncText('B'); const C = createAsyncText('C'); @@ -250,6 +254,86 @@ describe('ReactSuspenseList', () => { 'Suspend! [C]', ]); + assertConsoleErrorDev([ + 'The default for the prop is changing. ' + + 'To be future compatible you must explictly specify either ' + + '"independent" (the current default), "together", "forwards" or "legacy_unstable-backwards".' + + '\n in SuspenseList (at **)' + + '\n in Foo (at **)', + ]); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + Loading B + Loading C + , + ); + + await act(() => C.resolve()); + assertLog( + gate('alwaysThrottleRetries') + ? ['Suspend! [B]', 'C', 'Suspend! [B]'] + : ['C'], + ); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + Loading B + C + , + ); + + await act(() => B.resolve()); + assertLog(['B']); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + B + C + , + ); + }); + + // @gate enableSuspenseList + it('shows content independently with revealOrder="independent"', async () => { + const A = createAsyncText('A'); + const B = createAsyncText('B'); + const C = createAsyncText('C'); + + function Foo() { + return ( + + }> + + + }> + + + }> + + + + ); + } + + await A.resolve(); + + ReactNoop.render(); + + await waitForAll([ + 'A', + 'Suspend! [B]', + 'Loading B', + 'Suspend! [C]', + 'Loading C', + // pre-warming + 'Suspend! [B]', + 'Suspend! [C]', + ]); + expect(ReactNoop).toMatchRenderedOutput( <> A @@ -564,7 +648,7 @@ describe('ReactSuspenseList', () => { }); // @gate enableSuspenseList - it('displays all "together" in nested SuspenseLists where the inner is default', async () => { + it('displays all "together" in nested SuspenseLists where the inner is "independent"', async () => { const A = createAsyncText('A'); const B = createAsyncText('B'); const C = createAsyncText('C'); @@ -575,7 +659,7 @@ describe('ReactSuspenseList', () => { }> - + }> @@ -897,7 +981,7 @@ describe('ReactSuspenseList', () => { function Foo() { return ( - + }> @@ -955,6 +1039,85 @@ describe('ReactSuspenseList', () => { ); }); + // @gate enableSuspenseList + it('warns if revealOrder="backwards" is specified', async () => { + const A = createAsyncText('A'); + const B = createAsyncText('B'); + const C = createAsyncText('C'); + + function Foo() { + return ( + + }> + + + }> + + + }> + + + + ); + } + + await A.resolve(); + + ReactNoop.render(); + + await waitForAll([ + 'Suspend! [C]', + 'Loading C', + 'Loading B', + 'Loading A', + // pre-warming + 'Suspend! [C]', + ]); + + assertConsoleErrorDev([ + 'The rendering order of is changing. ' + + 'To be future compatible you must specify ' + + 'revealOrder="legacy_unstable-backwards" instead.' + + '\n in SuspenseList (at **)' + + '\n in Foo (at **)', + ]); + + expect(ReactNoop).toMatchRenderedOutput( + <> + Loading A + Loading B + Loading C + , + ); + + await act(() => C.resolve()); + assertLog([ + 'C', + 'Suspend! [B]', + // pre-warming + 'Suspend! [B]', + ]); + + expect(ReactNoop).toMatchRenderedOutput( + <> + Loading A + Loading B + C + , + ); + + await act(() => B.resolve()); + assertLog(['B', 'A']); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + B + C + , + ); + }); + // @gate enableSuspenseList it('displays each items in "backwards" order', async () => { const A = createAsyncText('A'); @@ -963,7 +1126,7 @@ describe('ReactSuspenseList', () => { function Foo() { return ( - + }> @@ -1037,7 +1200,7 @@ describe('ReactSuspenseList', () => { function Foo({items}) { return ( - + {items.map(([key, Component]) => ( }> @@ -1222,7 +1385,7 @@ describe('ReactSuspenseList', () => { function Foo({items}) { return ( - + {items.map(([key, Component]) => ( }> @@ -1400,7 +1563,7 @@ describe('ReactSuspenseList', () => { it('switches to rendering fallbacks if the tail takes long CPU time', async () => { function Foo() { return ( - + }> @@ -1535,6 +1698,29 @@ describe('ReactSuspenseList', () => { ); }); + // @gate enableSuspenseList + it('warns if no tail option is specified', async () => { + function Foo() { + return ( + + A + B + + ); + } + + await act(() => { + ReactNoop.render(); + }); + assertConsoleErrorDev([ + 'The default for the prop is changing. ' + + 'To be future compatible you must explictly specify either ' + + '"visible" (the current default), "collapsed" or "hidden".' + + '\n in SuspenseList (at **)' + + '\n in Foo (at **)', + ]); + }); + // @gate enableSuspenseList it('warns if an unsupported tail option is used', async () => { function Foo() { @@ -1551,7 +1737,7 @@ describe('ReactSuspenseList', () => { }); assertConsoleErrorDev([ '"collapse" is not a supported value for tail on ' + - '. Did you mean "collapsed" or "hidden"?' + + '. Did you mean "visible", "collapsed" or "hidden"?' + '\n in SuspenseList (at **)' + '\n in Foo (at **)', ]); @@ -1796,7 +1982,7 @@ describe('ReactSuspenseList', () => { function Foo({items}) { return ( - + {items.map(([key, Component]) => ( }> @@ -2154,7 +2340,7 @@ describe('ReactSuspenseList', () => { function Foo() { return ( - + }> @@ -2255,7 +2441,7 @@ describe('ReactSuspenseList', () => { function Foo({showB}) { return ( - + }> @@ -2321,7 +2507,7 @@ describe('ReactSuspenseList', () => { function Foo() { return (
- + @@ -2673,7 +2859,7 @@ describe('ReactSuspenseList', () => { function App() { Scheduler.log('App'); return ( - + }> @@ -2760,7 +2946,7 @@ describe('ReactSuspenseList', () => { Scheduler.log('App'); return ( - + }> @@ -2936,7 +3122,7 @@ describe('ReactSuspenseList', () => { // Several layers of Bailout wrappers help verify we're // marking updates all the way to the propagation root. return ( - + @@ -3029,7 +3215,7 @@ describe('ReactSuspenseList', () => { function Repro({update}) { return ( - + {update && ( }> @@ -3128,7 +3314,7 @@ describe('ReactSuspenseList', () => { } function Foo() { return ( - + ); @@ -3184,7 +3370,11 @@ describe('ReactSuspenseList', () => { }; function Foo() { - return {iterable}; + return ( + + {iterable} + + ); } await act(() => { @@ -3270,7 +3460,7 @@ describe('ReactSuspenseList', () => { it('warns if a nested async iterable is passed to a "forwards" list', async () => { function Foo({items}) { return ( - + {items}
Tail
 diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js index cad1a011817f6..2e252acbf3be7 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js @@ -345,7 +345,7 @@ describe('ReactSuspenseyCommitPhase', () => { it('demonstrate current behavior when used with SuspenseList (not ideal)', async () => { function App() { return ( - + }> diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index a18d8d16748f9..579edf25c9102 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -1799,7 +1799,7 @@ function renderSuspenseListRows( task: Task, keyPath: KeyNode, rows: Array, - revealOrder: 'forwards' | 'backwards', + revealOrder: 'forwards' | 'backwards' | 'unstable_legacy-backwards', ): void { // This is a fork of renderChildrenArray that's aware of tracking rows. const prevKeyPath = task.keyPath; @@ -1827,7 +1827,11 @@ function renderSuspenseListRows( // Since we are going to resume into a slot whose order was already // determined by the prerender, we can safely resume it even in reverse // render order. - const i = revealOrder !== 'backwards' ? n : totalChildren - 1 - n; + const i = + revealOrder !== 'backwards' && + revealOrder !== 'unstable_legacy-backwards' + ? n + : totalChildren - 1 - n; const node = rows[i]; task.row = previousSuspenseListRow = createSuspenseListRow( previousSuspenseListRow, @@ -1852,7 +1856,11 @@ function renderSuspenseListRows( // Since we are going to resume into a slot whose order was already // determined by the prerender, we can safely resume it even in reverse // render order. - const i = revealOrder !== 'backwards' ? n : totalChildren - 1 - n; + const i = + revealOrder !== 'backwards' && + revealOrder !== 'unstable_legacy-backwards' + ? n + : totalChildren - 1 - n; const node = rows[i]; if (__DEV__) { warnForMissingKey(request, task, node); @@ -1869,7 +1877,10 @@ function renderSuspenseListRows( } } else { task = ((task: any): RenderTask); // Refined - if (revealOrder !== 'backwards') { + if ( + revealOrder !== 'backwards' && + revealOrder !== 'unstable_legacy-backwards' + ) { // Forwards direction for (let i = 0; i < totalChildren; i++) { const node = rows[i]; @@ -1973,7 +1984,11 @@ function renderSuspenseList( const revealOrder: SuspenseListRevealOrder = props.revealOrder; // TODO: Support tail hidden/collapsed modes. // const tailMode: SuspenseListTailMode = props.tail; - if (revealOrder === 'forwards' || revealOrder === 'backwards') { + if ( + revealOrder === 'forwards' || + revealOrder === 'backwards' || + revealOrder === 'unstable_legacy-backwards' + ) { // For ordered reveal, we need to produce rows from the children. if (isArray(children)) { renderSuspenseListRows(request, task, keyPath, children, revealOrder); diff --git a/packages/shared/ReactTypes.js b/packages/shared/ReactTypes.js index b2b2f25fbaa93..ea9d3d1d0789a 100644 --- a/packages/shared/ReactTypes.js +++ b/packages/shared/ReactTypes.js @@ -308,20 +308,32 @@ export type SuspenseProps = { export type SuspenseListRevealOrder = | 'forwards' | 'backwards' + | 'unstable_legacy-backwards' | 'together' + | 'independent' | void; -export type SuspenseListTailMode = 'collapsed' | 'hidden' | void; +export type SuspenseListTailMode = 'visible' | 'collapsed' | 'hidden' | void; + +// A SuspenseList row cannot include a nested Array since it's an easy mistake to not realize it +// is treated as a single row. A Fragment can be used to intentionally have multiple children as +// a single row. +type SuspenseListRow = Exclude< + ReactNodeList, + Iterable | AsyncIterable, +>; type DirectionalSuspenseListProps = { - children?: ReactNodeList, - revealOrder: 'forwards' | 'backwards', + // Directional SuspenseList are defined by an array of children or multiple slots to JSX + // It does not allow a single element child. + children?: Iterable | AsyncIterable, // Note: AsyncIterable is experimental. + revealOrder: 'forwards' | 'backwards' | 'unstable_legacy-backwards', tail?: SuspenseListTailMode, }; type NonDirectionalSuspenseListProps = { children?: ReactNodeList, - revealOrder?: 'together' | void, + revealOrder?: 'independent' | 'together' | void, tail?: void, };