From c97a6d54d13246aa323f313490119d2a7d4c261b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 18 May 2025 17:18:01 -0400 Subject: [PATCH 1/8] Test --- .../ReactDOMFizzSuspenseList-test.js | 327 ++++++++++++++++++ 1 file changed, 327 insertions(+) create mode 100644 packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js new file mode 100644 index 0000000000000..90ced7d677dbb --- /dev/null +++ b/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js @@ -0,0 +1,327 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + * @jest-environment ./scripts/jest/ReactDOMServerIntegrationEnvironment + */ + +'use strict'; +import { + insertNodesAndExecuteScripts, + getVisibleChildren, +} from '../test-utils/FizzTestUtils'; + +let JSDOM; +let React; +let Suspense; +let SuspenseList; +let assertLog; +let Scheduler; +let ReactDOMFizzServer; +let Stream; +let document; +let writable; +let container; +let buffer = ''; +let hasErrored = false; +let fatalError = undefined; + +describe('ReactDOMFizSuspenseList', () => { + beforeEach(() => { + jest.resetModules(); + JSDOM = require('jsdom').JSDOM; + React = require('react'); + assertLog = require('internal-test-utils').assertLog; + ReactDOMFizzServer = require('react-dom/server'); + Stream = require('stream'); + + Suspense = React.Suspense; + SuspenseList = React.unstable_SuspenseList; + + Scheduler = require('scheduler'); + + // Test Environment + const jsdom = new JSDOM( + '
', + { + runScripts: 'dangerously', + }, + ); + document = jsdom.window.document; + container = document.getElementById('container'); + global.window = jsdom.window; + // The Fizz runtime assumes requestAnimationFrame exists so we need to polyfill it. + global.requestAnimationFrame = global.window.requestAnimationFrame = cb => + setTimeout(cb); + + buffer = ''; + hasErrored = false; + + writable = new Stream.PassThrough(); + writable.setEncoding('utf8'); + writable.on('data', chunk => { + buffer += chunk; + }); + writable.on('error', error => { + hasErrored = true; + fatalError = error; + }); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + async function serverAct(callback) { + await callback(); + // Await one turn around the event loop. + // This assumes that we'll flush everything we have so far. + await new Promise(resolve => { + setImmediate(resolve); + }); + if (hasErrored) { + throw fatalError; + } + // JSDOM doesn't support stream HTML parser so we need to give it a proper fragment. + // We also want to execute any scripts that are embedded. + // We assume that we have now received a proper fragment of HTML. + const bufferedContent = buffer; + buffer = ''; + const temp = document.createElement('body'); + temp.innerHTML = bufferedContent; + await insertNodesAndExecuteScripts(temp, container, null); + jest.runAllTimers(); + } + + function Text(props) { + Scheduler.log(props.text); + return {props.text}; + } + + function createAsyncText(text) { + let resolved = false; + const Component = function () { + if (!resolved) { + Scheduler.log('Suspend! [' + text + ']'); + throw promise; + } + return ; + }; + const promise = new Promise(resolve => { + Component.resolve = function () { + resolved = true; + return resolve(); + }; + }); + return Component; + } + + // @gate enableSuspenseList + it('shows content independently by default', 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 each items in "forwards" order', async () => { + const A = createAsyncText('A'); + const B = createAsyncText('B'); + const C = createAsyncText('C'); + + function Foo() { + return ( +
+ + }> + + + }> + + + }> + + + +
+ ); + } + + await C.resolve(); + + await serverAct(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(); + pipe(writable); + }); + + assertLog([ + 'Suspend! [A]', + 'Suspend! [B]', // TODO: Defer rendering the content after fallback if previous suspended, + 'C', + 'Loading A', + 'Loading B', + 'Loading C', + ]); + + expect(getVisibleChildren(container)).toEqual( +
+ Loading A + Loading B + Loading C +
, + ); + + await serverAct(() => A.resolve()); + assertLog(['A']); + + expect(getVisibleChildren(container)).toEqual( +
+ A + Loading B + Loading C +
, + ); + + await serverAct(() => B.resolve()); + assertLog(['B']); + + expect(getVisibleChildren(container)).toEqual( +
+ A + B + C +
, + ); + }); + + // @gate enableSuspenseList + it('displays each items in "backwards" order', 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([ + 'Suspend! [C]', + 'Suspend! [B]', // TODO: Defer rendering the content after fallback if previous suspended, + 'A', + 'Loading C', + 'Loading B', + 'Loading A', + ]); + + expect(getVisibleChildren(container)).toEqual( +
+ Loading A + Loading B + Loading C +
, + ); + + await serverAct(() => C.resolve()); + assertLog(['C']); + + expect(getVisibleChildren(container)).toEqual( +
+ Loading A + Loading B + C +
, + ); + + await serverAct(() => B.resolve()); + assertLog(['B']); + + expect(getVisibleChildren(container)).toEqual( +
+ A + B + C +
, + ); + }); +}); From 8aac798b390420c9967a6a1b70327ee12dd8aae0 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sat, 17 May 2025 18:27:23 -0400 Subject: [PATCH 2/8] Track the row that we're rendering into as a context on the task --- packages/react-server/src/ReactFizzServer.js | 166 ++++++++++++++++++- 1 file changed, 158 insertions(+), 8 deletions(-) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 247e21076e5b6..1ce7b69675f4b 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -24,6 +24,8 @@ import type { ViewTransitionProps, ActivityProps, SuspenseProps, + SuspenseListProps, + SuspenseListRevealOrder, } from 'shared/ReactTypes'; import type {LazyComponent as LazyComponentType} from 'react/src/ReactLazy'; import type { @@ -231,6 +233,12 @@ type LegacyContext = { [key: string]: any, }; +type SuspenseListRow = { + pendingTasks: number, // The number of tasks, previous rows and inner suspense boundaries blocking this row. + boundaries: Array, // The boundaries in this row waiting to be unblocked by the previous row. + next: null | SuspenseListRow, // The next row blocked by this one. +}; + const CLIENT_RENDERED = 4; // if it errors or infinitely suspends type SuspenseBoundary = { @@ -268,11 +276,12 @@ type RenderTask = { formatContext: FormatContext, // the format's specific context (e.g. HTML/SVG/MathML) context: ContextSnapshot, // the current new context that this task is executing in treeContext: TreeContext, // the current tree context that this task is executing in + row: null | SuspenseListRow, // the current SuspenseList row that this is rendering inside componentStack: null | ComponentStackNode, // stack frame description of the currently rendering component thenableState: null | ThenableState, legacyContext: LegacyContext, // the current legacy context that this task is executing in debugTask: null | ConsoleTask, // DEV only - // DON'T ANY MORE FIELDS. We at 16 already which otherwise requires converting to a constructor. + // DON'T ANY MORE FIELDS. We at 16 in prod already which otherwise requires converting to a constructor. // Consider splitting into multiple objects or consolidating some fields. }; @@ -298,12 +307,11 @@ type ReplayTask = { formatContext: FormatContext, // the format's specific context (e.g. HTML/SVG/MathML) context: ContextSnapshot, // the current new context that this task is executing in treeContext: TreeContext, // the current tree context that this task is executing in + row: null | SuspenseListRow, // the current SuspenseList row that this is rendering inside componentStack: null | ComponentStackNode, // stack frame description of the currently rendering component thenableState: null | ThenableState, legacyContext: LegacyContext, // the current legacy context that this task is executing in debugTask: null | ConsoleTask, // DEV only - // DON'T ANY MORE FIELDS. We at 16 already which otherwise requires converting to a constructor. - // Consider splitting into multiple objects or consolidating some fields. }; export type Task = RenderTask | ReplayTask; @@ -542,6 +550,7 @@ export function createRequest( rootContextSnapshot, emptyTreeContext, null, + null, emptyContextObject, null, ); @@ -647,6 +656,7 @@ export function resumeRequest( rootContextSnapshot, emptyTreeContext, null, + null, emptyContextObject, null, ); @@ -674,6 +684,7 @@ export function resumeRequest( rootContextSnapshot, emptyTreeContext, null, + null, emptyContextObject, null, ); @@ -782,6 +793,7 @@ function createRenderTask( formatContext: FormatContext, context: ContextSnapshot, treeContext: TreeContext, + row: null | SuspenseListRow, componentStack: null | ComponentStackNode, legacyContext: LegacyContext, debugTask: null | ConsoleTask, @@ -806,6 +818,7 @@ function createRenderTask( formatContext, context, treeContext, + row, componentStack, thenableState, }: any); @@ -832,6 +845,7 @@ function createReplayTask( formatContext: FormatContext, context: ContextSnapshot, treeContext: TreeContext, + row: null | SuspenseListRow, componentStack: null | ComponentStackNode, legacyContext: LegacyContext, debugTask: null | ConsoleTask, @@ -857,6 +871,7 @@ function createReplayTask( formatContext, context, treeContext, + row, componentStack, thenableState, }: any); @@ -1145,17 +1160,20 @@ function renderSuspenseBoundary( // so we can just render through it. const prevKeyPath = someTask.keyPath; const prevContext = someTask.formatContext; + const prevRow = someTask.row; someTask.keyPath = keyPath; someTask.formatContext = getSuspenseContentFormatContext( request.resumableState, prevContext, ); + someTask.row = null; const content: ReactNodeList = props.children; try { renderNode(request, someTask, content, -1); } finally { someTask.keyPath = prevKeyPath; someTask.formatContext = prevContext; + someTask.row = prevRow; } return; } @@ -1164,6 +1182,7 @@ function renderSuspenseBoundary( const prevKeyPath = task.keyPath; const prevContext = task.formatContext; + const prevRow = task.row; const parentBoundary = task.blockedBoundary; const parentPreamble = task.blockedPreamble; const parentHoistableState = task.hoistableState; @@ -1290,6 +1309,7 @@ function renderSuspenseBoundary( ), task.context, task.treeContext, + null, // The row gets reset inside the Suspense boundary. task.componentStack, !disableLegacyContext ? task.legacyContext : emptyContextObject, __DEV__ ? task.debugTask : null, @@ -1318,6 +1338,7 @@ function renderSuspenseBoundary( request.resumableState, prevContext, ); + task.row = null; contentRootSegment.status = RENDERING; try { @@ -1405,6 +1426,7 @@ function renderSuspenseBoundary( task.blockedSegment = parentSegment; task.keyPath = prevKeyPath; task.formatContext = prevContext; + task.row = prevRow; } const fallbackKeyPath = [keyPath[0], 'Suspense Fallback', keyPath[2]]; @@ -1427,6 +1449,7 @@ function renderSuspenseBoundary( ), task.context, task.treeContext, + task.row, task.componentStack, !disableLegacyContext ? task.legacyContext : emptyContextObject, __DEV__ ? task.debugTask : null, @@ -1451,6 +1474,7 @@ function replaySuspenseBoundary( ): void { const prevKeyPath = task.keyPath; const prevContext = task.formatContext; + const prevRow = task.row; const previousReplaySet: ReplaySet = task.replay; const parentBoundary = task.blockedBoundary; @@ -1490,6 +1514,7 @@ function replaySuspenseBoundary( request.resumableState, prevContext, ); + task.row = null; task.replay = {nodes: childNodes, slots: childSlots, pendingTasks: 1}; try { @@ -1566,6 +1591,7 @@ function replaySuspenseBoundary( task.replay = previousReplaySet; task.keyPath = prevKeyPath; task.formatContext = prevContext; + task.row = prevRow; } const fallbackKeyPath = [keyPath[0], 'Suspense Fallback', keyPath[2]]; @@ -1593,6 +1619,7 @@ function replaySuspenseBoundary( ), task.context, task.treeContext, + task.row, task.componentStack, !disableLegacyContext ? task.legacyContext : emptyContextObject, __DEV__ ? task.debugTask : null, @@ -1604,6 +1631,130 @@ function replaySuspenseBoundary( request.pingedTasks.push(suspendedFallbackTask); } +function renderSuspenseListRows( + request: Request, + task: Task, + keyPath: KeyNode, + rows: Array, + revealOrder: 'forwards' | 'backwards', +): void { + const prevKeyPath = task.keyPath; + task.keyPath = keyPath; + renderChildrenArray(request, task, rows, -1); + task.keyPath = prevKeyPath; +} + +function renderSuspenseList( + request: Request, + task: Task, + keyPath: KeyNode, + props: SuspenseListProps, +): void { + const children: any = props.children; + const revealOrder: SuspenseListRevealOrder = props.revealOrder; + // TODO: Support tail hidden/collapsed modes. + // const tailMode: SuspenseListTailMode = props.tail; + if (revealOrder === 'forwards' || revealOrder === 'backwards') { + // For ordered reveal, we need to produce rows from the children. + if (isArray(children)) { + renderSuspenseListRows(request, task, keyPath, children, revealOrder); + return; + } + const iteratorFn = getIteratorFn(children); + if (iteratorFn) { + const iterator = iteratorFn.call(children); + if (iterator) { + if (__DEV__) { + validateIterable(task, children, -1, iterator, iteratorFn); + } + // TODO: We currently use the same id algorithm as regular nodes + // but we need a new algorithm for SuspenseList that doesn't require + // a full set to be loaded up front to support Async Iterable. + // When we have that, we shouldn't buffer anymore. + let step = iterator.next(); + if (!step.done) { + const rows = []; + do { + rows.push(step.value); + step = iterator.next(); + } while (!step.done); + renderSuspenseListRows(request, task, keyPath, children, revealOrder); + } + return; + } + } + if ( + enableAsyncIterableChildren && + typeof (children: any)[ASYNC_ITERATOR] === 'function' + ) { + const iterator: AsyncIterator = (children: any)[ + ASYNC_ITERATOR + ](); + if (iterator) { + if (__DEV__) { + validateAsyncIterable(task, (children: any), -1, iterator); + } + // TODO: Update the task.children to be the iterator to avoid asking + // for new iterators, but we currently warn for rendering these + // so needs some refactoring to deal with the warning. + + // Restore the thenable state before resuming. + const prevThenableState = task.thenableState; + task.thenableState = null; + prepareToUseThenableState(prevThenableState); + + // We need to know how many total rows are in this set, so that we + // can allocate enough id slots to acommodate them. So we must exhaust + // the iterator before we start recursively rendering the rows. + // TODO: This is not great but I think it's inherent to the id + // generation algorithm. + + const rows = []; + + let done = false; + + if (iterator === children) { + // If it's an iterator we need to continue reading where we left + // off. We can do that by reading the first few rows from the previous + // thenable state. + // $FlowFixMe + let step = readPreviousThenableFromState(); + while (step !== undefined) { + if (step.done) { + done = true; + break; + } + rows.push(step.value); + step = readPreviousThenableFromState(); + } + } + + if (!done) { + let step = unwrapThenable(iterator.next()); + while (!step.done) { + rows.push(step.value); + step = unwrapThenable(iterator.next()); + } + } + renderSuspenseListRows(request, task, keyPath, rows, revealOrder); + return; + } + } + // This case will warn on the client. + renderSuspenseListRows(request, task, keyPath, [children], revealOrder); + return; + } + + if (revealOrder === 'together') { + // TODO + } + // For other reveal order modes, we just render it as a fragment. + const prevKeyPath = task.keyPath; + task.keyPath = keyPath; + renderNodeDestructive(request, task, children, -1); + task.keyPath = prevKeyPath; +} + function renderPreamble( request: Request, task: Task, @@ -1634,6 +1785,7 @@ function renderPreamble( task.formatContext, task.context, task.treeContext, + task.row, task.componentStack, !disableLegacyContext ? task.legacyContext : emptyContextObject, __DEV__ ? task.debugTask : null, @@ -2383,11 +2535,7 @@ function renderElement( return; } case REACT_SUSPENSE_LIST_TYPE: { - // TODO: SuspenseList should control the boundaries. - const prevKeyPath = task.keyPath; - task.keyPath = keyPath; - renderNodeDestructive(request, task, props.children, -1); - task.keyPath = prevKeyPath; + renderSuspenseList(request, task, keyPath, props); return; } case REACT_VIEW_TRANSITION_TYPE: { @@ -3537,6 +3685,7 @@ function spawnNewSuspendedReplayTask( task.formatContext, task.context, task.treeContext, + task.row, task.componentStack, !disableLegacyContext ? task.legacyContext : emptyContextObject, __DEV__ ? task.debugTask : null, @@ -3578,6 +3727,7 @@ function spawnNewSuspendedRenderTask( task.formatContext, task.context, task.treeContext, + task.row, task.componentStack, !disableLegacyContext ? task.legacyContext : emptyContextObject, __DEV__ ? task.debugTask : null, From 95ebd58a5af4b443875bf4cb69f2b89939e46e9c Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 19 May 2025 00:28:03 -0400 Subject: [PATCH 3/8] Track SuspenseListRow per row rendered Backwards row are rendered in reverse order but flushed in the right order using the segments. --- packages/react-server/src/ReactFizzServer.js | 190 ++++++++++++++++++- 1 file changed, 185 insertions(+), 5 deletions(-) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 1ce7b69675f4b..87b02aacfa097 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -1631,6 +1631,41 @@ function replaySuspenseBoundary( request.pingedTasks.push(suspendedFallbackTask); } +function finishSuspenseListRow(request: Request, row: SuspenseListRow): void { + // This row finished. Now we have to unblock all the next rows that were blocked on this. + // We do this in a loop to avoid stash overflow for very long lists that get unblocked. + let unblockedRow = row.next; + while (unblockedRow !== null) { + unblockedRow.pendingTasks--; + if (unblockedRow.pendingTasks > 0) { + // Still blocked. + break; + } + const unblockedBoundaries = unblockedRow.boundaries; + for (let i = 0; i < unblockedBoundaries.length; i++) { + finishedTask(request, unblockedBoundaries[i], null); + } + unblockedRow = unblockedRow.next; + } +} + +function createSuspenseListRow( + previousRow: null | SuspenseListRow, +): SuspenseListRow { + const newRow: SuspenseListRow = { + pendingTasks: 1, // At first the row is blocked on attempting rendering itself. + boundaries: [], + next: null, + }; + if (previousRow !== null && previousRow.pendingTasks > 0) { + // If the previous row is not done yet, we add ourselves to be blocked on it. + // When it finishes, we'll decrement our pending tasks. + newRow.pendingTasks++; + previousRow.next = newRow; + } + return newRow; +} + function renderSuspenseListRows( request: Request, task: Task, @@ -1638,10 +1673,157 @@ function renderSuspenseListRows( rows: Array, revealOrder: 'forwards' | 'backwards', ): void { + // This is a fork of renderChildrenArray that's aware of tracking rows. const prevKeyPath = task.keyPath; - task.keyPath = keyPath; - renderChildrenArray(request, task, rows, -1); + const previousComponentStack = task.componentStack; + let previousDebugTask = null; + if (__DEV__) { + previousDebugTask = task.debugTask; + // We read debugInfo from task.node.props.children instead of rows because it + // might have been an unwrapped iterable so we read from the original node. + pushServerComponentStack(task, (task.node: any).props.children._debugInfo); + } + + const prevTreeContext = task.treeContext; + const prevRow = task.row; + const totalChildren = rows.length; + + if (task.replay !== null) { + // Replay + // First we need to check if we have any resume slots at this level. + const resumeSlots = task.replay.slots; + if (resumeSlots !== null && typeof resumeSlots === 'object') { + let previousSuspenseListRow: null | SuspenseListRow = null; + for (let n = 0; n < totalChildren; n++) { + // 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 node = rows[i]; + task.row = previousSuspenseListRow = createSuspenseListRow( + previousSuspenseListRow, + ); + task.treeContext = pushTreeContext(prevTreeContext, totalChildren, i); + const resumeSegmentID = resumeSlots[i]; + // TODO: If this errors we should still continue with the next sibling. + if (typeof resumeSegmentID === 'number') { + resumeNode(request, task, resumeSegmentID, node, i); + // We finished rendering this node, so now we can consume this + // slot. This must happen after in case we rerender this task. + delete resumeSlots[i]; + } else { + renderNode(request, task, node, i); + } + if (--previousSuspenseListRow.pendingTasks === 0) { + finishSuspenseListRow(request, previousSuspenseListRow); + } + } + } else { + let previousSuspenseListRow: null | SuspenseListRow = null; + for (let n = 0; n < totalChildren; n++) { + // 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 node = rows[i]; + if (__DEV__) { + warnForMissingKey(request, task, node); + } + task.row = previousSuspenseListRow = createSuspenseListRow( + previousSuspenseListRow, + ); + task.treeContext = pushTreeContext(prevTreeContext, totalChildren, i); + renderNode(request, task, node, i); + if (--previousSuspenseListRow.pendingTasks === 0) { + finishSuspenseListRow(request, previousSuspenseListRow); + } + } + } + } else { + task = ((task: any): RenderTask); // Refined + if (revealOrder !== 'backwards') { + // Forwards direction + let previousSuspenseListRow: null | SuspenseListRow = null; + for (let i = 0; i < totalChildren; i++) { + const node = rows[i]; + if (__DEV__) { + warnForMissingKey(request, task, node); + } + task.row = previousSuspenseListRow = createSuspenseListRow( + previousSuspenseListRow, + ); + task.treeContext = pushTreeContext(prevTreeContext, totalChildren, i); + renderNode(request, task, node, i); + if (--previousSuspenseListRow.pendingTasks === 0) { + finishSuspenseListRow(request, previousSuspenseListRow); + } + } + } else { + // For backwards direction we need to do things a bit differently. + // We give each row its own segment so that we can render the content in + // reverse order but still emit it in the right order when we flush. + const parentSegment = task.blockedSegment; + const childIndex = parentSegment.children.length; + const insertionIndex = parentSegment.chunks.length; + let previousSuspenseListRow: null | SuspenseListRow = null; + for (let i = totalChildren - 1; i >= 0; i--) { + const node = rows[i]; + task.row = previousSuspenseListRow = createSuspenseListRow( + previousSuspenseListRow, + ); + task.treeContext = pushTreeContext(prevTreeContext, totalChildren, i); + const newSegment = createPendingSegment( + request, + insertionIndex, + null, + task.formatContext, + // Assume we are text embedded at the trailing edges + i === 0 ? parentSegment.lastPushedText : true, + true, + ); + // Insert in the beginning of the sequence, which will insert before any previous rows. + parentSegment.children.splice(childIndex, 0, newSegment); + task.blockedSegment = newSegment; + if (__DEV__) { + warnForMissingKey(request, task, node); + } + try { + renderNode(request, task, node, i); + pushSegmentFinale( + newSegment.chunks, + request.renderState, + newSegment.lastPushedText, + newSegment.textEmbedded, + ); + newSegment.status = COMPLETED; + finishedSegment(request, task.blockedBoundary, newSegment); + if (--previousSuspenseListRow.pendingTasks === 0) { + finishSuspenseListRow(request, previousSuspenseListRow); + } + } catch (thrownValue: mixed) { + if (request.status === ABORTING) { + newSegment.status = ABORTED; + } else { + newSegment.status = ERRORED; + } + throw thrownValue; + } + } + task.blockedSegment = parentSegment; + // Reset lastPushedText for current Segment since the new Segments "consumed" it + parentSegment.lastPushedText = false; + } + } + + // Because this context is always set right before rendering every child, we + // only need to reset it to the previous value at the very end. + task.treeContext = prevTreeContext; + task.row = prevRow; task.keyPath = prevKeyPath; + if (__DEV__) { + task.componentStack = previousComponentStack; + task.debugTask = previousDebugTask; + } } function renderSuspenseList( @@ -1740,9 +1922,7 @@ function renderSuspenseList( return; } } - // This case will warn on the client. - renderSuspenseListRows(request, task, keyPath, [children], revealOrder); - return; + // This case will warn on the client. It's the same as independent revealOrder. } if (revealOrder === 'together') { From e74fb07833df95e4d1ade3bd31ef4721c5c66e11 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 19 May 2025 12:18:37 -0400 Subject: [PATCH 4/8] Block rows from completing when there's still incomplete tasks within them Not covered by Suspense boundaries. While these already block the parent, it is needed to know whether we might discover more Suspense boundaries later. --- packages/react-server/src/ReactFizzServer.js | 67 +++++++++++++++----- 1 file changed, 51 insertions(+), 16 deletions(-) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 87b02aacfa097..19a9d7992326b 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -235,7 +235,7 @@ type LegacyContext = { type SuspenseListRow = { pendingTasks: number, // The number of tasks, previous rows and inner suspense boundaries blocking this row. - boundaries: Array, // The boundaries in this row waiting to be unblocked by the previous row. + boundaries: null | Array, // The boundaries in this row waiting to be unblocked by the previous row. (null means this row is not blocked) next: null | SuspenseListRow, // The next row blocked by this one. }; @@ -804,6 +804,9 @@ function createRenderTask( } else { blockedBoundary.pendingTasks++; } + if (row !== null) { + row.pendingTasks++; + } const task: RenderTask = ({ replay: null, node, @@ -856,6 +859,9 @@ function createReplayTask( } else { blockedBoundary.pendingTasks++; } + if (row !== null) { + row.pendingTasks++; + } replay.pendingTasks++; const task: ReplayTask = ({ replay, @@ -1636,15 +1642,21 @@ function finishSuspenseListRow(request: Request, row: SuspenseListRow): void { // We do this in a loop to avoid stash overflow for very long lists that get unblocked. let unblockedRow = row.next; while (unblockedRow !== null) { + // Unblocking the boundaries will decrement the count of this row but we keep it above + // zero so they never finish this row recursively. + const unblockedBoundaries = unblockedRow.boundaries; + if (unblockedBoundaries !== null) { + unblockedRow.boundaries = null; + for (let i = 0; i < unblockedBoundaries.length; i++) { + finishedTask(request, unblockedBoundaries[i], null, null); + } + } + // Instead we decrement at the end to keep it all in this loop. unblockedRow.pendingTasks--; if (unblockedRow.pendingTasks > 0) { // Still blocked. break; } - const unblockedBoundaries = unblockedRow.boundaries; - for (let i = 0; i < unblockedBoundaries.length; i++) { - finishedTask(request, unblockedBoundaries[i], null); - } unblockedRow = unblockedRow.next; } } @@ -1654,13 +1666,14 @@ function createSuspenseListRow( ): SuspenseListRow { const newRow: SuspenseListRow = { pendingTasks: 1, // At first the row is blocked on attempting rendering itself. - boundaries: [], + boundaries: null, next: null, }; if (previousRow !== null && previousRow.pendingTasks > 0) { // If the previous row is not done yet, we add ourselves to be blocked on it. // When it finishes, we'll decrement our pending tasks. newRow.pendingTasks++; + newRow.boundaries = []; previousRow.next = newRow; } return newRow; @@ -4216,10 +4229,19 @@ function erroredReplay( function erroredTask( request: Request, boundary: Root | SuspenseBoundary, + row: null | SuspenseListRow, error: mixed, errorInfo: ThrownInfo, debugTask: null | ConsoleTask, ) { + if (row !== null) { + if (--row.pendingTasks === 0) { + finishSuspenseListRow(request, row); + } + } + + request.allPendingTasks--; + // Report the error to a global handler. let errorDigest; // We don't handle halts here because we only halt when prerendering and @@ -4271,7 +4293,6 @@ function erroredTask( } } - request.allPendingTasks--; if (request.allPendingTasks === 0) { completeAll(request); } @@ -4286,7 +4307,7 @@ function abortTaskSoft(this: Request, task: Task): void { const segment = task.blockedSegment; if (segment !== null) { segment.status = ABORTED; - finishedTask(request, boundary, segment); + finishedTask(request, boundary, task.row, segment); } } @@ -4399,6 +4420,13 @@ function abortTask(task: Task, request: Request, error: mixed): void { segment.status = ABORTED; } + const row = task.row; + if (row !== null) { + if (--row.pendingTasks === 0) { + finishSuspenseListRow(request, row); + } + } + const errorInfo = getThrownInfo(task.componentStack); if (boundary === null) { @@ -4421,7 +4449,7 @@ function abortTask(task: Task, request: Request, error: mixed): void { // we just need to mark it as postponed. logPostpone(request, postponeInstance.message, errorInfo, null); trackPostpone(request, trackedPostpones, task, segment); - finishedTask(request, null, segment); + finishedTask(request, null, row, segment); } else { const fatal = new Error( 'The render was aborted with postpone when the shell is incomplete. Reason: ' + @@ -4440,7 +4468,7 @@ function abortTask(task: Task, request: Request, error: mixed): void { // We log the error but we still resolve the prerender logRecoverableError(request, error, errorInfo, null); trackPostpone(request, trackedPostpones, task, segment); - finishedTask(request, null, segment); + finishedTask(request, null, row, segment); } else { logRecoverableError(request, error, errorInfo, null); fatalError(request, error, errorInfo, null); @@ -4512,7 +4540,7 @@ function abortTask(task: Task, request: Request, error: mixed): void { abortTask(fallbackTask, request, error), ); boundary.fallbackAbortableTasks.clear(); - return finishedTask(request, boundary, segment); + return finishedTask(request, boundary, row, segment); } } boundary.status = CLIENT_RENDERED; @@ -4529,7 +4557,7 @@ function abortTask(task: Task, request: Request, error: mixed): void { logPostpone(request, postponeInstance.message, errorInfo, null); if (request.trackedPostpones !== null && segment !== null) { trackPostpone(request, request.trackedPostpones, task, segment); - finishedTask(request, task.blockedBoundary, segment); + finishedTask(request, task.blockedBoundary, row, segment); // If this boundary was still pending then we haven't already cancelled its fallbacks. // We'll need to abort the fallbacks, which will also error that parent boundary. @@ -4685,8 +4713,14 @@ function finishedSegment( function finishedTask( request: Request, boundary: Root | SuspenseBoundary, + row: null | SuspenseListRow, segment: null | Segment, ) { + if (row !== null) { + if (--row.pendingTasks === 0) { + finishSuspenseListRow(request, row); + } + } request.allPendingTasks--; if (boundary === null) { if (segment !== null && segment.parentFlushed) { @@ -4833,7 +4867,7 @@ function retryRenderTask( task.abortSet.delete(task); segment.status = COMPLETED; finishedSegment(request, task.blockedBoundary, segment); - finishedTask(request, task.blockedBoundary, segment); + finishedTask(request, task.blockedBoundary, task.row, segment); } catch (thrownValue: mixed) { resetHooksState(); @@ -4886,7 +4920,7 @@ function retryRenderTask( } trackPostpone(request, trackedPostpones, task, segment); - finishedTask(request, task.blockedBoundary, segment); + finishedTask(request, task.blockedBoundary, task.row, segment); return; } @@ -4920,7 +4954,7 @@ function retryRenderTask( __DEV__ ? task.debugTask : null, ); trackPostpone(request, trackedPostpones, task, segment); - finishedTask(request, task.blockedBoundary, segment); + finishedTask(request, task.blockedBoundary, task.row, segment); return; } } @@ -4932,6 +4966,7 @@ function retryRenderTask( erroredTask( request, task.blockedBoundary, + task.row, x, errorInfo, __DEV__ ? task.debugTask : null, @@ -4979,7 +5014,7 @@ function retryReplayTask(request: Request, task: ReplayTask): void { task.replay.pendingTasks--; task.abortSet.delete(task); - finishedTask(request, task.blockedBoundary, null); + finishedTask(request, task.blockedBoundary, task.row, null); } catch (thrownValue) { resetHooksState(); From 7e8aec6db80e636e970a28dbf2f7728aa000c918 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 19 May 2025 13:04:23 -0400 Subject: [PATCH 5/8] Block a row from completing until a Suspense boundary completes However, if a boundary might get outlined, then we can't consider it complete until it actually gets written. If it's not eligible then we can flush it early to allow for it to be inline. --- packages/react-server/src/ReactFizzServer.js | 71 +++++++++++++++++++- 1 file changed, 70 insertions(+), 1 deletion(-) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 19a9d7992326b..c4d43ed2fd781 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -246,6 +246,7 @@ type SuspenseBoundary = { rootSegmentID: number, parentFlushed: boolean, pendingTasks: number, // when it reaches zero we can show this boundary's content + row: null | SuspenseListRow, // the row that this boundary blocks from completing. completedSegments: Array, // completed but not yet flushed segments. byteSize: number, // used to determine whether to inline children boundaries. fallbackAbortableTasks: Set, // used to cancel task on the fallback if the boundary completes or gets canceled. @@ -750,6 +751,7 @@ function pingTask(request: Request, task: Task): void { function createSuspenseBoundary( request: Request, + row: null | SuspenseListRow, fallbackAbortableTasks: Set, contentPreamble: null | Preamble, fallbackPreamble: null | Preamble, @@ -759,6 +761,7 @@ function createSuspenseBoundary( rootSegmentID: -1, parentFlushed: false, pendingTasks: 0, + row: row, completedSegments: [], byteSize: 0, fallbackAbortableTasks, @@ -776,6 +779,17 @@ function createSuspenseBoundary( boundary.errorStack = null; boundary.errorComponentStack = null; } + if (row !== null) { + // This boundary will block this row from completing. + row.pendingTasks++; + const blockedBoundaries = row.boundaries; + if (blockedBoundaries !== null) { + // Previous rows will block this boundary itself from completing. + request.allPendingTasks++; + boundary.pendingTasks++; + blockedBoundaries.push(boundary); + } + } return boundary; } @@ -1206,12 +1220,19 @@ function renderSuspenseBoundary( if (canHavePreamble(task.formatContext)) { newBoundary = createSuspenseBoundary( request, + task.row, fallbackAbortSet, createPreambleState(), createPreambleState(), ); } else { - newBoundary = createSuspenseBoundary(request, fallbackAbortSet, null, null); + newBoundary = createSuspenseBoundary( + request, + task.row, + fallbackAbortSet, + null, + null, + ); } if (request.trackedPostpones !== null) { newBoundary.trackedContentKeyPath = keyPath; @@ -1366,6 +1387,14 @@ function renderSuspenseBoundary( // the fallback. However, if this boundary ended up big enough to be eligible for outlining // we can't do that because we might still need the fallback if we outline it. if (!isEligibleForOutlining(request, newBoundary)) { + if (prevRow !== null) { + // If we have synchronously completed the boundary and it's not eligible for outlining + // then we don't have to wait for it to be flushed before we unblock future rows. + // This lets us inline small rows in order. + if (--prevRow.pendingTasks === 0) { + finishSuspenseListRow(request, prevRow); + } + } if (request.pendingRootTasks === 0 && task.blockedPreamble) { // The root is complete and this boundary may contribute part of the preamble. // We eagerly attempt to prepare the preamble here because we expect most requests @@ -1494,6 +1523,7 @@ function replaySuspenseBoundary( if (canHavePreamble(task.formatContext)) { resumedBoundary = createSuspenseBoundary( request, + task.row, fallbackAbortSet, createPreambleState(), createPreambleState(), @@ -1501,6 +1531,7 @@ function replaySuspenseBoundary( } else { resumedBoundary = createSuspenseBoundary( request, + task.row, fallbackAbortSet, null, null, @@ -4321,6 +4352,7 @@ function abortRemainingSuspenseBoundary( ): void { const resumedBoundary = createSuspenseBoundary( request, + null, new Set(), null, null, @@ -4769,6 +4801,13 @@ function finishedTask( if (!isEligibleForOutlining(request, boundary)) { boundary.fallbackAbortableTasks.forEach(abortTaskSoft, request); boundary.fallbackAbortableTasks.clear(); + const boundaryRow = boundary.row; + if (boundaryRow !== null) { + // If we aren't eligible for outlining, we don't have to wait until we flush it. + if (--boundaryRow.pendingTasks === 0) { + finishSuspenseListRow(request, boundaryRow); + } + } } if ( @@ -5326,6 +5365,16 @@ function flushSegment( // Emit a client rendered suspense boundary wrapper. // We never queue the inner boundary so we'll never emit its content or partial segments. + const row = boundary.row; + if (row !== null) { + // Since this boundary end up client rendered, we can unblock future suspense list rows. + // This means that they may appear out of order if the future rows succeed but this is + // a client rendered row. + if (--row.pendingTasks === 0) { + finishSuspenseListRow(request, row); + } + } + if (__DEV__) { writeStartClientRenderedSuspenseBoundary( destination, @@ -5414,6 +5463,16 @@ function flushSegment( if (hoistableState) { hoistHoistables(hoistableState, boundary.contentState); } + + const row = boundary.row; + if (row !== null && isEligibleForOutlining(request, boundary)) { + // Once we have written the boundary, we can unblock the row and let future + // rows be written. This may schedule new completed boundaries. + if (--row.pendingTasks === 0) { + finishSuspenseListRow(request, row); + } + } + // We can inline this boundary's content as a complete boundary. writeStartCompletedSuspenseBoundary(destination, request.renderState); @@ -5492,6 +5551,15 @@ function flushCompletedBoundary( } completedSegments.length = 0; + const row = boundary.row; + if (row !== null && isEligibleForOutlining(request, boundary)) { + // Once we have written the boundary, we can unblock the row and let future + // rows be written. This may schedule new completed boundaries. + if (--row.pendingTasks === 0) { + finishSuspenseListRow(request, row); + } + } + writeHoistablesForBoundary( destination, boundary.contentState, @@ -5684,6 +5752,7 @@ function flushCompletedQueues( // Next we check the completed boundaries again. This may have had // boundaries added to it in case they were too larged to be inlined. + // SuspenseListRows might have been unblocked as well. // New ones might be added in this loop. const largeBoundaries = request.completedBoundaries; for (i = 0; i < largeBoundaries.length; i++) { From 8ab5e1f45b220d7ac5ad346658df923ccfb28c49 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 19 May 2025 13:18:38 -0400 Subject: [PATCH 6/8] Use in fixture These are now appearing in order (except the "first paint" thing) --- fixtures/ssr/src/components/LargeContent.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/fixtures/ssr/src/components/LargeContent.js b/fixtures/ssr/src/components/LargeContent.js index a5af3064b4917..7c4a6cf2258ef 100644 --- a/fixtures/ssr/src/components/LargeContent.js +++ b/fixtures/ssr/src/components/LargeContent.js @@ -1,8 +1,12 @@ -import React, {Fragment, Suspense} from 'react'; +import React, { + Fragment, + Suspense, + unstable_SuspenseList as SuspenseList, +} from 'react'; export default function LargeContent() { return ( - +

Lorem ipsum dolor sit amet, consectetur adipiscing elit. Mauris @@ -286,6 +290,6 @@ export default function LargeContent() { interdum a. Proin nec odio in nulla vestibulum.

- + ); } From 662f9b550659cb91ac1c9971daede4c07d6ae6c0 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 19 May 2025 13:38:26 -0400 Subject: [PATCH 7/8] Fix test that's now works properly --- .../src/__tests__/ReactDOMFizzServer-test.js | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 8bb3e2f4b74b9..d6147d60e077c 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1318,10 +1318,8 @@ describe('ReactDOMFizzServer', () => { expect(ref.current).toBe(null); expect(getVisibleChildren(container)).toEqual(
- Loading A - {/* // TODO: This is incorrect. It should be "Loading B" but Fizz SuspenseList - // isn't implemented fully yet. */} - B + {'Loading A'} + {'Loading B'}
, ); @@ -1335,11 +1333,9 @@ describe('ReactDOMFizzServer', () => { // We haven't resolved yet. expect(getVisibleChildren(container)).toEqual(
- Loading A - {/* // TODO: This is incorrect. It should be "Loading B" but Fizz SuspenseList - // isn't implemented fully yet. */} - B - Loading C + {'Loading A'} + {'Loading B'} + {'Loading C'}
, ); From 0c7f03f02a51b20b23ee6a89a105f6b989905044 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Mon, 19 May 2025 15:16:31 -0400 Subject: [PATCH 8/8] Update packages/react-server/src/ReactFizzServer.js Typo Co-authored-by: Sebastian "Sebbie" Silbermann --- packages/react-server/src/ReactFizzServer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index c4d43ed2fd781..b8f5c1be75db0 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -1670,7 +1670,7 @@ function replaySuspenseBoundary( function finishSuspenseListRow(request: Request, row: SuspenseListRow): void { // This row finished. Now we have to unblock all the next rows that were blocked on this. - // We do this in a loop to avoid stash overflow for very long lists that get unblocked. + // We do this in a loop to avoid stack overflow for very long lists that get unblocked. let unblockedRow = row.next; while (unblockedRow !== null) { // Unblocking the boundaries will decrement the count of this row but we keep it above