From ac2c1a5a5840d5e043d1d7a12a356f226e285c02 Mon Sep 17 00:00:00 2001 From: Hendrik Liebau Date: Thu, 25 Sep 2025 15:13:15 +0200 Subject: [PATCH 1/3] [Flight] Ensure blocked debug info is handled properly (#34524) This PR ensures that server components are reliably included in the DevTools component tree, even if debug info is received delayed, e.g. when using a debug channel. The fix consists of three parts: - We must not unset the debug chunk before all debug info entries are resolved. - We must ensure that the "RSC Stream" IO debug info entry is pushed last, after all other entries were resolved. - We need to transfer the debug info from blocked element chunks onto the lazy node and the element. Ideally, we wouldn't even create a lazy node for blocked elements that are at the root of the JSON payload, because that would basically wrap a lazy in a lazy. This optimization that ensures that everything around the blocked element can proceed is only needed for nested elements. However, we also need it for resolving deduped references in blocked root elements, unless we adapt that logic, which would be a bigger lift. When reloading the Flight fixture, the component tree is now displayed deterministically. Previously, it would sometimes omit synchronous server components. complete --------- Co-authored-by: Sebastian Markbage --- .../react-client/src/ReactFlightClient.js | 212 ++++++++++++------ .../src/__tests__/ReactFlight-test.js | 55 ++--- .../__tests__/ReactFlightDOMBrowser-test.js | 147 ++++++++++++ .../src/__tests__/ReactFlightDOMEdge-test.js | 16 +- .../src/__tests__/ReactFlightDOMNode-test.js | 24 +- 5 files changed, 341 insertions(+), 113 deletions(-) diff --git a/packages/react-client/src/ReactFlightClient.js b/packages/react-client/src/ReactFlightClient.js index 6e96f2f0378ec..701d9df33cdd9 100644 --- a/packages/react-client/src/ReactFlightClient.js +++ b/packages/react-client/src/ReactFlightClient.js @@ -499,10 +499,44 @@ function createErrorChunk( return new ReactPromise(ERRORED, null, error); } +function moveDebugInfoFromChunkToInnerValue( + chunk: InitializedChunk, + value: T, +): void { + // Remove the debug info from the initialized chunk, and add it to the inner + // value instead. This can be a React element, an array, or an uninitialized + // Lazy. + const resolvedValue = resolveLazy(value); + if ( + typeof resolvedValue === 'object' && + resolvedValue !== null && + (isArray(resolvedValue) || + typeof resolvedValue[ASYNC_ITERATOR] === 'function' || + resolvedValue.$$typeof === REACT_ELEMENT_TYPE || + resolvedValue.$$typeof === REACT_LAZY_TYPE) + ) { + const debugInfo = chunk._debugInfo.splice(0); + if (isArray(resolvedValue._debugInfo)) { + // $FlowFixMe[method-unbinding] + resolvedValue._debugInfo.unshift.apply( + resolvedValue._debugInfo, + debugInfo, + ); + } else { + Object.defineProperty((resolvedValue: any), '_debugInfo', { + configurable: false, + enumerable: false, + writable: true, + value: debugInfo, + }); + } + } +} + function wakeChunk( listeners: Array mixed)>, value: T, - chunk: SomeChunk, + chunk: InitializedChunk, ): void { for (let i = 0; i < listeners.length; i++) { const listener = listeners[i]; @@ -512,6 +546,10 @@ function wakeChunk( fulfillReference(listener, value, chunk); } } + + if (__DEV__) { + moveDebugInfoFromChunkToInnerValue(chunk, value); + } } function rejectChunk( @@ -649,7 +687,6 @@ function triggerErrorOnChunk( } try { initializeDebugChunk(response, chunk); - chunk._debugChunk = null; if (initializingHandler !== null) { if (initializingHandler.errored) { // Ignore error parsing debug info, we'll report the original error instead. @@ -932,9 +969,9 @@ function initializeModelChunk(chunk: ResolvedModelChunk): void { } if (__DEV__) { - // Lazily initialize any debug info and block the initializing chunk on any unresolved entries. + // Initialize any debug info and block the initializing chunk on any + // unresolved entries. initializeDebugChunk(response, chunk); - chunk._debugChunk = null; } try { @@ -946,7 +983,14 @@ function initializeModelChunk(chunk: ResolvedModelChunk): void { if (resolveListeners !== null) { cyclicChunk.value = null; cyclicChunk.reason = null; - wakeChunk(resolveListeners, value, cyclicChunk); + for (let i = 0; i < resolveListeners.length; i++) { + const listener = resolveListeners[i]; + if (typeof listener === 'function') { + listener(value); + } else { + fulfillReference(listener, value, cyclicChunk); + } + } } if (initializingHandler !== null) { if (initializingHandler.errored) { @@ -963,6 +1007,10 @@ function initializeModelChunk(chunk: ResolvedModelChunk): void { const initializedChunk: InitializedChunk = (chunk: any); initializedChunk.status = INITIALIZED; initializedChunk.value = value; + + if (__DEV__) { + moveDebugInfoFromChunkToInnerValue(initializedChunk, value); + } } catch (error) { const erroredChunk: ErroredChunk = (chunk: any); erroredChunk.status = ERRORED; @@ -1079,7 +1127,7 @@ function getTaskName(type: mixed): string { function initializeElement( response: Response, element: any, - lazyType: null | LazyComponent< + lazyNode: null | LazyComponent< React$Element, SomeChunk>, >, @@ -1151,15 +1199,33 @@ function initializeElement( initializeFakeStack(response, owner); } - // In case the JSX runtime has validated the lazy type as a static child, we - // need to transfer this information to the element. - if ( - lazyType && - lazyType._store && - lazyType._store.validated && - !element._store.validated - ) { - element._store.validated = lazyType._store.validated; + if (lazyNode !== null) { + // In case the JSX runtime has validated the lazy type as a static child, we + // need to transfer this information to the element. + if ( + lazyNode._store && + lazyNode._store.validated && + !element._store.validated + ) { + element._store.validated = lazyNode._store.validated; + } + + // If the lazy node is initialized, we move its debug info to the inner + // value. + if (lazyNode._payload.status === INITIALIZED && lazyNode._debugInfo) { + const debugInfo = lazyNode._debugInfo.splice(0); + if (element._debugInfo) { + // $FlowFixMe[method-unbinding] + element._debugInfo.unshift.apply(element._debugInfo, debugInfo); + } else { + Object.defineProperty(element, '_debugInfo', { + configurable: false, + enumerable: false, + writable: true, + value: debugInfo, + }); + } + } } // TODO: We should be freezing the element but currently, we might write into @@ -1279,13 +1345,13 @@ function createElement( createBlockedChunk(response); handler.value = element; handler.chunk = blockedChunk; - const lazyType = createLazyChunkWrapper(blockedChunk, validated); + const lazyNode = createLazyChunkWrapper(blockedChunk, validated); if (__DEV__) { // After we have initialized any blocked references, initialize stack etc. - const init = initializeElement.bind(null, response, element, lazyType); + const init = initializeElement.bind(null, response, element, lazyNode); blockedChunk.then(init, init); } - return lazyType; + return lazyNode; } } if (__DEV__) { @@ -1466,7 +1532,7 @@ function fulfillReference( const element: any = handler.value; switch (key) { case '3': - transferReferencedDebugInfo(handler.chunk, fulfilledChunk, mappedValue); + transferReferencedDebugInfo(handler.chunk, fulfilledChunk); element.props = mappedValue; break; case '4': @@ -1482,11 +1548,11 @@ function fulfillReference( } break; default: - transferReferencedDebugInfo(handler.chunk, fulfilledChunk, mappedValue); + transferReferencedDebugInfo(handler.chunk, fulfilledChunk); break; } } else if (__DEV__ && !reference.isDebug) { - transferReferencedDebugInfo(handler.chunk, fulfilledChunk, mappedValue); + transferReferencedDebugInfo(handler.chunk, fulfilledChunk); } handler.deps--; @@ -1808,47 +1874,34 @@ function loadServerReference, T>( return (null: any); } +function resolveLazy(value: any): mixed { + while ( + typeof value === 'object' && + value !== null && + value.$$typeof === REACT_LAZY_TYPE + ) { + const payload: SomeChunk = value._payload; + if (payload.status === INITIALIZED) { + value = payload.value; + continue; + } + break; + } + + return value; +} + function transferReferencedDebugInfo( parentChunk: null | SomeChunk, referencedChunk: SomeChunk, - referencedValue: mixed, ): void { if (__DEV__) { - const referencedDebugInfo = referencedChunk._debugInfo; - // If we have a direct reference to an object that was rendered by a synchronous - // server component, it might have some debug info about how it was rendered. - // We forward this to the underlying object. This might be a React Element or - // an Array fragment. - // If this was a string / number return value we lose the debug info. We choose - // that tradeoff to allow sync server components to return plain values and not - // use them as React Nodes necessarily. We could otherwise wrap them in a Lazy. - if ( - typeof referencedValue === 'object' && - referencedValue !== null && - (isArray(referencedValue) || - typeof referencedValue[ASYNC_ITERATOR] === 'function' || - referencedValue.$$typeof === REACT_ELEMENT_TYPE) - ) { - // We should maybe use a unique symbol for arrays but this is a React owned array. - // $FlowFixMe[prop-missing]: This should be added to elements. - const existingDebugInfo: ?ReactDebugInfo = - (referencedValue._debugInfo: any); - if (existingDebugInfo == null) { - Object.defineProperty((referencedValue: any), '_debugInfo', { - configurable: false, - enumerable: false, - writable: true, - value: referencedDebugInfo.slice(0), // Clone so that pushing later isn't going into the original - }); - } else { - // $FlowFixMe[method-unbinding] - existingDebugInfo.push.apply(existingDebugInfo, referencedDebugInfo); - } - } - // We also add the debug info to the initializing chunk since the resolution of that promise is - // also blocked by the referenced debug info. By adding it to both we can track it even if the array/element - // is extracted, or if the root is rendered as is. + // We add the debug info to the initializing chunk since the resolution of + // that promise is also blocked by the referenced debug info. By adding it + // to both we can track it even if the array/element/lazy is extracted, or + // if the root is rendered as is. if (parentChunk !== null) { + const referencedDebugInfo = referencedChunk._debugInfo; const parentDebugInfo = parentChunk._debugInfo; for (let i = 0; i < referencedDebugInfo.length; ++i) { const debugInfoEntry = referencedDebugInfo[i]; @@ -1999,7 +2052,7 @@ function getOutlinedModel( // If we're resolving the "owner" or "stack" slot of an Element array, we don't call // transferReferencedDebugInfo because this reference is to a debug chunk. } else { - transferReferencedDebugInfo(initializingChunk, chunk, chunkValue); + transferReferencedDebugInfo(initializingChunk, chunk); } return chunkValue; case PENDING: @@ -2709,14 +2762,47 @@ function incrementChunkDebugInfo( } } +function addDebugInfo(chunk: SomeChunk, debugInfo: ReactDebugInfo): void { + const value = resolveLazy(chunk.value); + if ( + typeof value === 'object' && + value !== null && + (isArray(value) || + typeof value[ASYNC_ITERATOR] === 'function' || + value.$$typeof === REACT_ELEMENT_TYPE || + value.$$typeof === REACT_LAZY_TYPE) + ) { + if (isArray(value._debugInfo)) { + // $FlowFixMe[method-unbinding] + value._debugInfo.push.apply(value._debugInfo, debugInfo); + } else { + Object.defineProperty((value: any), '_debugInfo', { + configurable: false, + enumerable: false, + writable: true, + value: debugInfo, + }); + } + } else { + // $FlowFixMe[method-unbinding] + chunk._debugInfo.push.apply(chunk._debugInfo, debugInfo); + } +} + function resolveChunkDebugInfo( streamState: StreamState, chunk: SomeChunk, ): void { if (__DEV__ && enableAsyncDebugInfo) { - // Push the currently resolving chunk's debug info representing the stream on the Promise - // that was waiting on the stream. - chunk._debugInfo.push({awaited: streamState._debugInfo}); + // Add the currently resolving chunk's debug info representing the stream + // to the Promise that was waiting on the stream, or its underlying value. + const debugInfo: ReactDebugInfo = [{awaited: streamState._debugInfo}]; + if (chunk.status === PENDING || chunk.status === BLOCKED) { + const boundAddDebugInfo = addDebugInfo.bind(null, chunk, debugInfo); + chunk.then(boundAddDebugInfo, boundAddDebugInfo); + } else { + addDebugInfo(chunk, debugInfo); + } } } @@ -2909,7 +2995,8 @@ function resolveStream>( const resolveListeners = chunk.value; if (__DEV__) { - // Lazily initialize any debug info and block the initializing chunk on any unresolved entries. + // Initialize any debug info and block the initializing chunk on any + // unresolved entries. if (chunk._debugChunk != null) { const prevHandler = initializingHandler; const prevChunk = initializingChunk; @@ -2923,7 +3010,6 @@ function resolveStream>( } try { initializeDebugChunk(response, chunk); - chunk._debugChunk = null; if (initializingHandler !== null) { if (initializingHandler.errored) { // Ignore error parsing debug info, we'll report the original error instead. @@ -2947,7 +3033,7 @@ function resolveStream>( resolvedChunk.value = stream; resolvedChunk.reason = controller; if (resolveListeners !== null) { - wakeChunk(resolveListeners, chunk.value, chunk); + wakeChunk(resolveListeners, chunk.value, (chunk: any)); } } diff --git a/packages/react-client/src/__tests__/ReactFlight-test.js b/packages/react-client/src/__tests__/ReactFlight-test.js index 0baee5a1f5098..b0f539bf2572c 100644 --- a/packages/react-client/src/__tests__/ReactFlight-test.js +++ b/packages/react-client/src/__tests__/ReactFlight-test.js @@ -327,8 +327,8 @@ describe('ReactFlight', () => { const transport = ReactNoopFlightServer.render(root); await act(async () => { - const promise = ReactNoopFlightClient.read(transport); - expect(getDebugInfo(promise)).toEqual( + const result = await ReactNoopFlightClient.read(transport); + expect(getDebugInfo(result)).toEqual( __DEV__ ? [ {time: 12}, @@ -346,7 +346,7 @@ describe('ReactFlight', () => { ] : undefined, ); - ReactNoop.render(await promise); + ReactNoop.render(result); }); expect(ReactNoop).toMatchRenderedOutput(Hello, Seb Smith); @@ -1378,9 +1378,7 @@ describe('ReactFlight', () => { environmentName: 'Server', }, ], - findSourceMapURLCalls: [ - [__filename, 'Server'], - [__filename, 'Server'], + findSourceMapURLCalls: expect.arrayContaining([ // TODO: What should we request here? The outer () or the inner (inspected-page.html)? ['inspected-page.html:29:11), ', 'Server'], [ @@ -1389,8 +1387,7 @@ describe('ReactFlight', () => { ], ['file:///testing.js', 'Server'], ['', 'Server'], - [__filename, 'Server'], - ], + ]), }); } else { expect(errors.map(getErrorForJestMatcher)).toEqual([ @@ -2785,8 +2782,8 @@ describe('ReactFlight', () => { ); await act(async () => { - const promise = ReactNoopFlightClient.read(transport); - expect(getDebugInfo(promise)).toEqual( + const result = await ReactNoopFlightClient.read(transport); + expect(getDebugInfo(result)).toEqual( __DEV__ ? [ {time: gate(flags => flags.enableAsyncDebugInfo) ? 22 : 20}, @@ -2803,11 +2800,10 @@ describe('ReactFlight', () => { ] : undefined, ); - const result = await promise; const thirdPartyChildren = await result.props.children[1]; // We expect the debug info to be transferred from the inner stream to the outer. - expect(getDebugInfo(thirdPartyChildren[0])).toEqual( + expect(getDebugInfo(await thirdPartyChildren[0])).toEqual( __DEV__ ? [ {time: gate(flags => flags.enableAsyncDebugInfo) ? 54 : 22}, // Clamped to the start @@ -2910,8 +2906,8 @@ describe('ReactFlight', () => { ); await act(async () => { - const promise = ReactNoopFlightClient.read(transport); - expect(getDebugInfo(promise)).toEqual( + const result = await ReactNoopFlightClient.read(transport); + expect(getDebugInfo(result)).toEqual( __DEV__ ? [ {time: 16}, @@ -2924,17 +2920,10 @@ describe('ReactFlight', () => { transport: expect.arrayContaining([]), }, }, - { - time: 16, - }, - { - time: 16, - }, {time: 31}, ] : undefined, ); - const result = await promise; const thirdPartyFragment = await result.props.children; expect(getDebugInfo(thirdPartyFragment)).toEqual( __DEV__ @@ -2949,15 +2938,7 @@ describe('ReactFlight', () => { children: {}, }, }, - { - time: 33, - }, - { - time: 33, - }, - { - time: 33, - }, + {time: 33}, ] : undefined, ); @@ -3013,8 +2994,8 @@ describe('ReactFlight', () => { ); await act(async () => { - const promise = ReactNoopFlightClient.read(transport); - expect(getDebugInfo(promise)).toEqual( + const result = await ReactNoopFlightClient.read(transport); + expect(getDebugInfo(result)).toEqual( __DEV__ ? [ {time: 16}, @@ -3040,7 +3021,6 @@ describe('ReactFlight', () => { ] : undefined, ); - const result = await promise; ReactNoop.render(result); }); @@ -3891,15 +3871,6 @@ describe('ReactFlight', () => { { time: 13, }, - { - time: 14, - }, - { - time: 15, - }, - { - time: 16, - }, ]); } else { expect(root._debugInfo).toBe(undefined); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js index a49e268ebf040..49cc28535e92d 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js @@ -27,6 +27,7 @@ let webpackMap; let webpackServerMap; let act; let serverAct; +let getDebugInfo; let React; let ReactDOM; let ReactDOMClient; @@ -48,6 +49,10 @@ describe('ReactFlightDOMBrowser', () => { ReactServerScheduler = require('scheduler'); patchMessageChannel(ReactServerScheduler); serverAct = require('internal-test-utils').serverAct; + getDebugInfo = require('internal-test-utils').getDebugInfo.bind(null, { + ignoreProps: true, + useFixedTime: true, + }); // Simulate the condition resolution @@ -1767,6 +1772,9 @@ describe('ReactFlightDOMBrowser', () => { webpackMap, ), ); + + // Snapshot updates change this formatting, so we let prettier ignore it. + // prettier-ignore const response = await ReactServerDOMClient.createFromReadableStream(stream); @@ -2906,4 +2914,143 @@ describe('ReactFlightDOMBrowser', () => { '
HiSebbie
', ); }); + + it('should fully resolve debug info when transported through a (slow) debug channel', async () => { + function Paragraph({children}) { + return ReactServer.createElement('p', null, children); + } + + let debugReadableStreamController; + + const debugReadableStream = new ReadableStream({ + start(controller) { + debugReadableStreamController = controller; + }, + }); + + const stream = await serverAct(() => + ReactServerDOMServer.renderToReadableStream( + { + root: ReactServer.createElement( + ReactServer.Fragment, + null, + ReactServer.createElement(Paragraph, null, 'foo'), + ReactServer.createElement(Paragraph, null, 'bar'), + ), + }, + webpackMap, + { + debugChannel: { + writable: new WritableStream({ + write(chunk) { + debugReadableStreamController.enqueue(chunk); + }, + close() { + debugReadableStreamController.close(); + }, + }), + }, + }, + ), + ); + + function ClientRoot({response}) { + const {root} = use(response); + return root; + } + + const [slowDebugStream1, slowDebugStream2] = + createDelayedStream(debugReadableStream).tee(); + + const response = ReactServerDOMClient.createFromReadableStream(stream, { + debugChannel: {readable: slowDebugStream1}, + }); + + const container = document.createElement('div'); + const clientRoot = ReactDOMClient.createRoot(container); + + await act(() => { + clientRoot.render(); + }); + + if (__DEV__) { + const debugStreamReader = slowDebugStream2.getReader(); + while (true) { + const {done} = await debugStreamReader.read(); + if (done) { + break; + } + // Allow the client to process each debug chunk as it arrives. + await act(() => {}); + } + } + + expect(container.innerHTML).toBe('

foo

bar

'); + + if ( + __DEV__ && + gate( + flags => + flags.enableComponentPerformanceTrack && flags.enableAsyncDebugInfo, + ) + ) { + const result = await response; + const firstParagraph = result.root[0]; + + expect(getDebugInfo(firstParagraph)).toMatchInlineSnapshot(` + [ + { + "time": 0, + }, + { + "env": "Server", + "key": null, + "name": "Paragraph", + "props": {}, + "stack": [ + [ + "", + "/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js", + 2937, + 27, + 2931, + 34, + ], + [ + "serverAct", + "/packages/internal-test-utils/internalAct.js", + 270, + 19, + 231, + 1, + ], + [ + "Object.", + "/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMBrowser-test.js", + 2931, + 18, + 2918, + 89, + ], + ], + }, + { + "time": 0, + }, + { + "awaited": { + "byteSize": 0, + "end": 0, + "name": "RSC stream", + "owner": null, + "start": 0, + "value": { + "value": "stream", + }, + }, + }, + ] + `); + } + }); }); diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js index 55b434ce3eeff..7aaf4150db087 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMEdge-test.js @@ -1240,7 +1240,7 @@ describe('ReactFlightDOMEdge', () => { env: 'Server', }); if (gate(flags => flags.enableAsyncDebugInfo)) { - expect(lazyWrapper._debugInfo).toEqual([ + expect(greeting._debugInfo).toEqual([ {time: 12}, greetInfo, {time: 13}, @@ -1259,7 +1259,7 @@ describe('ReactFlightDOMEdge', () => { } // The owner that created the span was the outer server component. // We expect the debug info to be referentially equal to the owner. - expect(greeting._owner).toBe(lazyWrapper._debugInfo[1]); + expect(greeting._owner).toBe(greeting._debugInfo[1]); } else { expect(lazyWrapper._debugInfo).toBe(undefined); expect(greeting._owner).toBe(undefined); @@ -1930,11 +1930,19 @@ describe('ReactFlightDOMEdge', () => { if (__DEV__) { expect(normalizeCodeLocInfo(componentStack)).toBe( - '\n in Component\n in Suspense\n in body\n in html\n in ClientRoot (at **)', + '\n in Component\n' + + ' in Suspense\n' + + ' in body\n' + + ' in html\n' + + ' in App (at **)\n' + + ' in ClientRoot (at **)', ); } else { expect(normalizeCodeLocInfo(componentStack)).toBe( - '\n in Suspense\n in body\n in html\n in ClientRoot (at **)', + '\n in Suspense\n' + + ' in body\n' + + ' in html\n' + + ' in ClientRoot (at **)', ); } diff --git a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js index f069b23b293c0..59df3c24d6f40 100644 --- a/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js +++ b/packages/react-server-dom-webpack/src/__tests__/ReactFlightDOMNode-test.js @@ -722,11 +722,19 @@ describe('ReactFlightDOMNode', () => { if (__DEV__) { expect(normalizeCodeLocInfo(componentStack)).toBe( - '\n in Component (at **)\n in Suspense\n in body\n in html\n in ClientRoot (at **)', + '\n in Component (at **)\n' + + ' in Suspense\n' + + ' in body\n' + + ' in html\n' + + ' in App (at **)\n' + + ' in ClientRoot (at **)', ); } else { expect(normalizeCodeLocInfo(componentStack)).toBe( - '\n in Suspense\n in body\n in html\n in ClientRoot (at **)', + '\n in Suspense\n' + + ' in body\n' + + ' in html\n' + + ' in ClientRoot (at **)', ); } @@ -861,11 +869,19 @@ describe('ReactFlightDOMNode', () => { if (__DEV__) { expect(normalizeCodeLocInfo(componentStack)).toBe( - '\n in Component (at **)\n in Suspense\n in body\n in html\n in ClientRoot (at **)', + '\n in Component (at **)\n' + + ' in Suspense\n' + + ' in body\n' + + ' in html\n' + + ' in App (at **)\n' + + ' in ClientRoot (at **)', ); } else { expect(normalizeCodeLocInfo(componentStack)).toBe( - '\n in Suspense\n in body\n in html\n in ClientRoot (at **)', + '\n in Suspense\n' + + ' in body\n' + + ' in html\n' + + ' in ClientRoot (at **)', ); } From 6eb5d67e9c4c5c456783dbbd454d79016c43b07d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 25 Sep 2025 09:38:41 -0400 Subject: [PATCH 2/3] [Fizz] Outline a Suspense Boundary if it has Suspensey CSS or Images (#34552) We should favor outlining a boundary if it contains Suspensey CSS or Suspensey Images since then we can load that content separately and not block the main content. This also allows us to animate the reveal. For example this should be able to animate the reveal even though the actual HTML content isn't large in this case it's worth outlining so that the JS runtime can choose to animate this reveal. ```js ``` For Suspensey Images, in Fizz, we currently only implement the suspensey semantics when a View Transition is running. Therefore the outlining only applies if it appears inside a Suspense boundary which might animate. Otherwise there's no point in outlining. It is also only if the Suspense boundary itself might animate its appear and not just any ViewTransition. So the effect is very conservative. For CSS it applies even without ViewTransition though, since it can help unblock the main content faster. --- .../view-transition/src/components/Page.js | 2 +- .../src/server/ReactFizzConfigDOM.js | 68 ++++++++++++++++--- .../src/server/ReactFizzConfigDOMLegacy.js | 6 ++ .../react-markup/src/ReactFizzConfigMarkup.js | 5 ++ .../src/ReactNoopServer.js | 3 + packages/react-server/src/ReactFizzServer.js | 15 +++- .../src/forks/ReactFizzConfig.custom.js | 1 + 7 files changed, 87 insertions(+), 13 deletions(-) diff --git a/fixtures/view-transition/src/components/Page.js b/fixtures/view-transition/src/components/Page.js index 658ed686293c7..d411bb2453069 100644 --- a/fixtures/view-transition/src/components/Page.js +++ b/fixtures/view-transition/src/components/Page.js @@ -238,8 +238,8 @@ export default function Page({url, navigate}) { + {show ? : null} - {show ? : null} diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js index eb8c2786152d8..a93c32a947f10 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js @@ -782,13 +782,14 @@ const HTML_COLGROUP_MODE = 9; type InsertionMode = 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9; -const NO_SCOPE = /* */ 0b000000; -const NOSCRIPT_SCOPE = /* */ 0b000001; -const PICTURE_SCOPE = /* */ 0b000010; -const FALLBACK_SCOPE = /* */ 0b000100; -const EXIT_SCOPE = /* */ 0b001000; // A direct Instance below a Suspense fallback is the only thing that can "exit" -const ENTER_SCOPE = /* */ 0b010000; // A direct Instance below Suspense content is the only thing that can "enter" -const UPDATE_SCOPE = /* */ 0b100000; // Inside a scope that applies "update" ViewTransitions if anything mutates here. +const NO_SCOPE = /* */ 0b0000000; +const NOSCRIPT_SCOPE = /* */ 0b0000001; +const PICTURE_SCOPE = /* */ 0b0000010; +const FALLBACK_SCOPE = /* */ 0b0000100; +const EXIT_SCOPE = /* */ 0b0001000; // A direct Instance below a Suspense fallback is the only thing that can "exit" +const ENTER_SCOPE = /* */ 0b0010000; // A direct Instance below Suspense content is the only thing that can "enter" +const UPDATE_SCOPE = /* */ 0b0100000; // Inside a scope that applies "update" ViewTransitions if anything mutates here. +const APPEARING_SCOPE = /* */ 0b1000000; // Below Suspense content subtree which might appear in an "enter" animation or "shared" animation. // Everything not listed here are tracked for the whole subtree as opposed to just // until the next Instance. @@ -987,11 +988,20 @@ export function getSuspenseContentFormatContext( resumableState: ResumableState, parentContext: FormatContext, ): FormatContext { + const viewTransition = getSuspenseViewTransition( + parentContext.viewTransition, + ); + let subtreeScope = parentContext.tagScope | ENTER_SCOPE; + if (viewTransition !== null && viewTransition.share !== 'none') { + // If we have a ViewTransition wrapping Suspense then the appearing animation + // will be applied just like an "enter" below. Mark it as animating. + subtreeScope |= APPEARING_SCOPE; + } return createFormatContext( parentContext.insertionMode, parentContext.selectedValue, - parentContext.tagScope | ENTER_SCOPE, - getSuspenseViewTransition(parentContext.viewTransition), + subtreeScope, + viewTransition, ); } @@ -1063,6 +1073,9 @@ export function getViewTransitionFormatContext( } else { subtreeScope &= ~UPDATE_SCOPE; } + if (enter !== 'none') { + subtreeScope |= APPEARING_SCOPE; + } return createFormatContext( parentContext.insertionMode, parentContext.selectedValue, @@ -3289,6 +3302,7 @@ function pushImg( props: Object, resumableState: ResumableState, renderState: RenderState, + hoistableState: null | HoistableState, formatContext: FormatContext, ): null { const pictureOrNoScriptTagInScope = @@ -3321,6 +3335,19 @@ function pushImg( ) { // We have a suspensey image and ought to preload it to optimize the loading of display blocking // resumableState. + + if (hoistableState !== null) { + // Mark this boundary's state as having suspensey images. + // Only do that if we have a ViewTransition that might trigger a parent Suspense boundary + // to animate its appearing. Since that's the only case we'd actually apply suspensey images + // for SSR reveals. + const isInSuspenseWithEnterViewTransition = + formatContext.tagScope & APPEARING_SCOPE; + if (isInSuspenseWithEnterViewTransition) { + hoistableState.suspenseyImages = true; + } + } + const sizes = typeof props.sizes === 'string' ? props.sizes : undefined; const key = getImageResourceKey(src, srcSet, sizes); @@ -4255,7 +4282,14 @@ export function pushStartInstance( return pushStartPreformattedElement(target, props, type, formatContext); } case 'img': { - return pushImg(target, props, resumableState, renderState, formatContext); + return pushImg( + target, + props, + resumableState, + renderState, + hoistableState, + formatContext, + ); } // Omitted close tags case 'base': @@ -6125,6 +6159,7 @@ type StylesheetResource = { export type HoistableState = { styles: Set, stylesheets: Set, + suspenseyImages: boolean, }; export type StyleQueue = { @@ -6138,6 +6173,7 @@ export function createHoistableState(): HoistableState { return { styles: new Set(), stylesheets: new Set(), + suspenseyImages: false, }; } @@ -6995,6 +7031,18 @@ export function hoistHoistables( ): void { childState.styles.forEach(hoistStyleQueueDependency, parentState); childState.stylesheets.forEach(hoistStylesheetDependency, parentState); + if (childState.suspenseyImages) { + // If the child has suspensey images, the parent now does too if it's inlined. + // Similarly, if a SuspenseList row has a suspensey image then effectively + // the next row should be blocked on it as well since the next row can't show + // earlier. In practice, since the child will be outlined this transferring + // may never matter but is conceptually correct. + parentState.suspenseyImages = true; + } +} + +export function hasSuspenseyContent(hoistableState: HoistableState): boolean { + return hoistableState.stylesheets.size > 0 || hoistableState.suspenseyImages; } // This function is called at various times depending on whether we are rendering diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js index 6ab54af00f7b7..d48e9a8dd932e 100644 --- a/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js +++ b/packages/react-dom-bindings/src/server/ReactFizzConfigDOMLegacy.js @@ -10,6 +10,7 @@ import type { RenderState as BaseRenderState, ResumableState, + HoistableState, StyleQueue, Resource, HeadersDescriptor, @@ -325,5 +326,10 @@ export function writePreambleStart( ); } +export function hasSuspenseyContent(hoistableState: HoistableState): boolean { + // Never outline. + return false; +} + export type TransitionStatus = FormStatus; export const NotPendingTransition: TransitionStatus = NotPending; diff --git a/packages/react-markup/src/ReactFizzConfigMarkup.js b/packages/react-markup/src/ReactFizzConfigMarkup.js index fee02f320fcb5..7dbe5592f3372 100644 --- a/packages/react-markup/src/ReactFizzConfigMarkup.js +++ b/packages/react-markup/src/ReactFizzConfigMarkup.js @@ -242,5 +242,10 @@ export function writeCompletedRoot( return true; } +export function hasSuspenseyContent(hoistableState: HoistableState): boolean { + // Never outline. + return false; +} + export type TransitionStatus = FormStatus; export const NotPendingTransition: TransitionStatus = NotPending; diff --git a/packages/react-noop-renderer/src/ReactNoopServer.js b/packages/react-noop-renderer/src/ReactNoopServer.js index 59f0fafa5d21f..1793180cc7658 100644 --- a/packages/react-noop-renderer/src/ReactNoopServer.js +++ b/packages/react-noop-renderer/src/ReactNoopServer.js @@ -324,6 +324,9 @@ const ReactNoopServer = ReactFizzServer({ writeHoistablesForBoundary() {}, writePostamble() {}, hoistHoistables(parent: HoistableState, child: HoistableState) {}, + hasSuspenseyContent(hoistableState: HoistableState): boolean { + return false; + }, createHoistableState(): HoistableState { return null; }, diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 7d54dff3d080a..b8184a1983708 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -99,6 +99,7 @@ import { hoistPreambleState, isPreambleReady, isPreambleContext, + hasSuspenseyContent, } from './ReactFizzConfig'; import { constructClassInstance, @@ -461,7 +462,7 @@ function isEligibleForOutlining( // The larger this limit is, the more we can save on preparing fallbacks in case we end up // outlining. return ( - boundary.byteSize > 500 && + (boundary.byteSize > 500 || hasSuspenseyContent(boundary.contentState)) && // For boundaries that can possibly contribute to the preamble we don't want to outline // them regardless of their size since the fallbacks should only be emitted if we've // errored the boundary. @@ -5748,8 +5749,13 @@ function flushSegment( return writeEndPendingSuspenseBoundary(destination, request.renderState); } else if ( + // We don't outline when we're emitting partially completed boundaries optimistically + // because it doesn't make sense to outline something if its parent is going to be + // blocked on something later in the stream anyway. + !flushingPartialBoundaries && isEligibleForOutlining(request, boundary) && - flushedByteSize + boundary.byteSize > request.progressiveChunkSize + (flushedByteSize + boundary.byteSize > request.progressiveChunkSize || + hasSuspenseyContent(boundary.contentState)) ) { // Inlining this boundary would make the current sequence being written too large // and block the parent for too long. Instead, it will be emitted separately so that we @@ -5980,6 +5986,8 @@ function flushPartiallyCompletedSegment( } } +let flushingPartialBoundaries = false; + function flushCompletedQueues( request: Request, destination: Destination, @@ -6095,6 +6103,7 @@ function flushCompletedQueues( // Next we emit any segments of any boundaries that are partially complete // but not deeply complete. + flushingPartialBoundaries = true; const partialBoundaries = request.partialBoundaries; for (i = 0; i < partialBoundaries.length; i++) { const boundary = partialBoundaries[i]; @@ -6106,6 +6115,7 @@ function flushCompletedQueues( } } partialBoundaries.splice(0, i); + flushingPartialBoundaries = false; // Next we check the completed boundaries again. This may have had // boundaries added to it in case they were too larged to be inlined. @@ -6123,6 +6133,7 @@ function flushCompletedQueues( } largeBoundaries.splice(0, i); } finally { + flushingPartialBoundaries = false; if ( request.allPendingTasks === 0 && request.clientRenderedBoundaries.length === 0 && diff --git a/packages/react-server/src/forks/ReactFizzConfig.custom.js b/packages/react-server/src/forks/ReactFizzConfig.custom.js index 981e390ea4dc6..aa8ea94b57917 100644 --- a/packages/react-server/src/forks/ReactFizzConfig.custom.js +++ b/packages/react-server/src/forks/ReactFizzConfig.custom.js @@ -104,4 +104,5 @@ export const writeHoistablesForBoundary = $$$config.writeHoistablesForBoundary; export const writePostamble = $$$config.writePostamble; export const hoistHoistables = $$$config.hoistHoistables; export const createHoistableState = $$$config.createHoistableState; +export const hasSuspenseyContent = $$$config.hasSuspenseyContent; export const emitEarlyPreloads = $$$config.emitEarlyPreloads; From b0c1dc01ecbfaf81aa69f760b29dd76c02600792 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 25 Sep 2025 12:05:47 -0400 Subject: [PATCH 3/3] [Flight] Add approximate parent context for FormatContext (#34601) Flight doesn't have any semantically sound notion of a parent context. That's why we removed Server Context. Each root can really start anywhere in the tree when you refetch subtrees. Additionally when you dedupe elements they can end up in multiple different parent contexts. However, we do have a DEV only version of this with debugTask being tracked for the nearest parent element to track the context of properties inside of it. To apply certain DOM specific hints and optimizations when you render host components we need some information of the context. This is usually very local so doesn't suffer from the likelihood that you refetch in the middle. We'll also only use this information for optimistic hints and not hard semantics so getting it wrong isn't terrible. ``` ``` For example, in these cases we should exclude preloading the image but we have to know if that's the scope we're in. We can easily get this wrong if they're split or even if they're wrapped in client components that we don't know about like: ``` ``` However, getting it wrong in either direction is not the end of the world. It's about covering the common cases well. --- .../src/server/ReactFlightServerConfigDOM.js | 14 ++++++ .../react-server/src/ReactFlightServer.js | 45 +++++++++++++++++++ .../forks/ReactFlightServerConfig.custom.js | 14 ++++++ .../ReactFlightServerConfig.dom-legacy.js | 14 ++++++ .../forks/ReactFlightServerConfig.markup.js | 14 ++++++ 5 files changed, 101 insertions(+) diff --git a/packages/react-dom-bindings/src/server/ReactFlightServerConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFlightServerConfigDOM.js index 9cded881352af..75ed81bf6fe42 100644 --- a/packages/react-dom-bindings/src/server/ReactFlightServerConfigDOM.js +++ b/packages/react-dom-bindings/src/server/ReactFlightServerConfigDOM.js @@ -61,3 +61,17 @@ export type Hints = Set; export function createHints(): Hints { return new Set(); } + +export opaque type FormatContext = null; + +export function createRootFormatContext(): FormatContext { + return null; +} + +export function getChildFormatContext( + parentContext: FormatContext, + type: string, + props: Object, +): FormatContext { + return parentContext; +} diff --git a/packages/react-server/src/ReactFlightServer.js b/packages/react-server/src/ReactFlightServer.js index 66203af1fefdb..d170787dc7997 100644 --- a/packages/react-server/src/ReactFlightServer.js +++ b/packages/react-server/src/ReactFlightServer.js @@ -49,6 +49,7 @@ import type { Hints, HintCode, HintModel, + FormatContext, } from './ReactFlightServerConfig'; import type {ThenableState} from './ReactFlightThenable'; import type { @@ -88,6 +89,8 @@ import { supportsRequestStorage, requestStorage, createHints, + createRootFormatContext, + getChildFormatContext, initAsyncDebugInfo, markAsyncSequenceRootTask, getCurrentAsyncSequence, @@ -525,6 +528,7 @@ type Task = { toJSON: (key: string, value: ReactClientValue) => ReactJSONValue, keyPath: null | string, // parent server component keys implicitSlot: boolean, // true if the root server component of this sequence had a null key + formatContext: FormatContext, // an approximate parent context from host components thenableState: ThenableState | null, timed: boolean, // Profiling-only. Whether we need to track the completion time of this task. time: number, // Profiling-only. The last time stamp emitted for this task. @@ -758,6 +762,7 @@ function RequestInstance( model, null, false, + createRootFormatContext(), abortSet, timeOrigin, null, @@ -980,6 +985,7 @@ function serializeThenable( (thenable: any), // will be replaced by the value before we retry. used for debug info. task.keyPath, // the server component sequence continues through Promise-as-a-child. task.implicitSlot, + task.formatContext, request.abortableTasks, enableProfilerTimer && (enableComponentPerformanceTrack || enableAsyncDebugInfo) @@ -1102,6 +1108,7 @@ function serializeReadableStream( task.model, task.keyPath, task.implicitSlot, + task.formatContext, request.abortableTasks, enableProfilerTimer && (enableComponentPerformanceTrack || enableAsyncDebugInfo) @@ -1197,6 +1204,7 @@ function serializeAsyncIterable( task.model, task.keyPath, task.implicitSlot, + task.formatContext, request.abortableTasks, enableProfilerTimer && (enableComponentPerformanceTrack || enableAsyncDebugInfo) @@ -2028,6 +2036,7 @@ function deferTask(request: Request, task: Task): ReactJSONValue { task.model, // the currently rendering element task.keyPath, // unlike outlineModel this one carries along context task.implicitSlot, + task.formatContext, request.abortableTasks, enableProfilerTimer && (enableComponentPerformanceTrack || enableAsyncDebugInfo) @@ -2048,6 +2057,7 @@ function outlineTask(request: Request, task: Task): ReactJSONValue { task.model, // the currently rendering element task.keyPath, // unlike outlineModel this one carries along context task.implicitSlot, + task.formatContext, request.abortableTasks, enableProfilerTimer && (enableComponentPerformanceTrack || enableAsyncDebugInfo) @@ -2214,6 +2224,22 @@ function renderElement( } } } + } else if (typeof type === 'string') { + const parentFormatContext = task.formatContext; + const newFormatContext = getChildFormatContext( + parentFormatContext, + type, + props, + ); + if (parentFormatContext !== newFormatContext && props.children != null) { + // We've entered a new context. We need to create another Task which has + // the new context set up since it's not safe to push/pop in the middle of + // a tree. Additionally this means that any deduping within this tree now + // assumes the new context even if it's reused outside in a different context. + // We'll rely on this to dedupe the value later as we discover it again + // inside the returned element's tree. + outlineModelWithFormatContext(request, props.children, newFormatContext); + } } // For anything else, try it on the client instead. // We don't know if the client will support it or not. This might error on the @@ -2530,6 +2556,7 @@ function createTask( model: ReactClientValue, keyPath: null | string, implicitSlot: boolean, + formatContext: FormatContext, abortSet: Set, lastTimestamp: number, // Profiling-only debugOwner: null | ReactComponentInfo, // DEV-only @@ -2554,6 +2581,7 @@ function createTask( model, keyPath, implicitSlot, + formatContext: formatContext, ping: () => pingTask(request, task), toJSON: function ( this: @@ -2819,11 +2847,26 @@ function serializeDebugClientReference( } function outlineModel(request: Request, value: ReactClientValue): number { + return outlineModelWithFormatContext( + request, + value, + // For deduped values we don't know which context it will be reused in + // so we have to assume that it's the root context. + createRootFormatContext(), + ); +} + +function outlineModelWithFormatContext( + request: Request, + value: ReactClientValue, + formatContext: FormatContext, +): number { const newTask = createTask( request, value, null, // The way we use outlining is for reusing an object. false, // It makes no sense for that use case to be contextual. + formatContext, // Except for FormatContext we optimistically use it. request.abortableTasks, enableProfilerTimer && (enableComponentPerformanceTrack || enableAsyncDebugInfo) @@ -3071,6 +3114,7 @@ function serializeBlob(request: Request, blob: Blob): string { model, null, false, + createRootFormatContext(), request.abortableTasks, enableProfilerTimer && (enableComponentPerformanceTrack || enableAsyncDebugInfo) @@ -3208,6 +3252,7 @@ function renderModel( task.model, task.keyPath, task.implicitSlot, + task.formatContext, request.abortableTasks, enableProfilerTimer && (enableComponentPerformanceTrack || enableAsyncDebugInfo) diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.custom.js b/packages/react-server/src/forks/ReactFlightServerConfig.custom.js index fbb0168eee631..a7f0ee3d991b0 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.custom.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.custom.js @@ -32,3 +32,17 @@ export const componentStorage: AsyncLocalStorage = export function createHints(): any { return null; } + +export type FormatContext = null; + +export function createRootFormatContext(): FormatContext { + return null; +} + +export function getChildFormatContext( + parentContext: FormatContext, + type: string, + props: Object, +): FormatContext { + return parentContext; +} diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.dom-legacy.js b/packages/react-server/src/forks/ReactFlightServerConfig.dom-legacy.js index bc599ac0b4011..2b4d2e1e809ea 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.dom-legacy.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.dom-legacy.js @@ -32,3 +32,17 @@ export const componentStorage: AsyncLocalStorage = export function createHints(): any { return null; } + +export type FormatContext = null; + +export function createRootFormatContext(): FormatContext { + return null; +} + +export function getChildFormatContext( + parentContext: FormatContext, + type: string, + props: Object, +): FormatContext { + return parentContext; +} diff --git a/packages/react-server/src/forks/ReactFlightServerConfig.markup.js b/packages/react-server/src/forks/ReactFlightServerConfig.markup.js index 59a1bac1eb491..ca8c4670834ff 100644 --- a/packages/react-server/src/forks/ReactFlightServerConfig.markup.js +++ b/packages/react-server/src/forks/ReactFlightServerConfig.markup.js @@ -19,6 +19,20 @@ export function createHints(): Hints { return null; } +export type FormatContext = null; + +export function createRootFormatContext(): FormatContext { + return null; +} + +export function getChildFormatContext( + parentContext: FormatContext, + type: string, + props: Object, +): FormatContext { + return parentContext; +} + export const supportsRequestStorage = false; export const requestStorage: AsyncLocalStorage = (null: any);