From 26cf2804802f3d32c4d8f9db73ddea12ad6c1670 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Fri, 31 Oct 2025 12:58:18 -0400 Subject: [PATCH] Switch the default revealOrder to "forwards" and tail "hidden" on SuspenseList (#35018) We have warned about this for a while now so we can make the switch. Often when you reach for SuspenseList, you mean forwards. It doesn't make sense to have the default to just be a noop. While "together" is another useful mode that's more like a Group so isn't so associated with the default as List. So we're switching it. However, tail=hidden isn't as obvious of a default it does allow for a convenient pattern for streaming in list of items by default. This doesn't yet switch the rendering order of "backwards". That's coming in a follow up. --- .../ReactDOMFizzSuspenseList-test.js | 35 +++-- .../react-reconciler/src/ReactChildFiber.js | 3 +- .../src/ReactFiberBeginWork.js | 80 +++++------ .../src/ReactFiberCompleteWork.js | 57 ++++---- .../src/ReactFiberSuspenseComponent.js | 5 +- .../src/__tests__/ReactSuspenseList-test.js | 125 ++++++++++-------- packages/react-server/src/ReactFizzServer.js | 8 +- 7 files changed, 165 insertions(+), 148 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js index 74db51d242..f434bf3cda 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js @@ -134,7 +134,7 @@ describe('ReactDOMFizzSuspenseList', () => { } // @gate enableSuspenseList - it('shows content independently by default', async () => { + it('shows content forwards by default', async () => { const A = createAsyncText('A'); const B = createAsyncText('B'); const C = createAsyncText('C'); @@ -157,14 +157,32 @@ describe('ReactDOMFizzSuspenseList', () => { ); } - await A.resolve(); + await C.resolve(); await serverAct(async () => { const {pipe} = ReactDOMFizzServer.renderToPipeableStream(); pipe(writable); }); - assertLog(['A', 'Suspend! [B]', 'Suspend! [C]', 'Loading B', 'Loading C']); + 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(
@@ -174,17 +192,6 @@ describe('ReactDOMFizzSuspenseList', () => {
, ); - await serverAct(() => C.resolve()); - assertLog(['C']); - - expect(getVisibleChildren(container)).toEqual( -
- A - Loading B - C -
, - ); - await serverAct(() => B.resolve()); assertLog(['B']); diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index f0d9c2e012..8daf7fde4b 100644 --- a/packages/react-reconciler/src/ReactChildFiber.js +++ b/packages/react-reconciler/src/ReactChildFiber.js @@ -2104,7 +2104,8 @@ export function validateSuspenseListChildren( ) { if (__DEV__) { if ( - (revealOrder === 'forwards' || + (revealOrder == null || + revealOrder === 'forwards' || revealOrder === 'backwards' || revealOrder === 'unstable_legacy-backwards') && children !== undefined && diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 7251e52dd6..32ecd33edc 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -3245,6 +3245,7 @@ function validateRevealOrder(revealOrder: SuspenseListRevealOrder) { if (__DEV__) { const cacheKey = revealOrder == null ? 'null' : revealOrder; if ( + revealOrder != null && revealOrder !== 'forwards' && revealOrder !== 'unstable_legacy-backwards' && revealOrder !== 'together' && @@ -3252,13 +3253,7 @@ function validateRevealOrder(revealOrder: SuspenseListRevealOrder) { !didWarnAboutRevealOrder[cacheKey] ) { 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') { + if (revealOrder === 'backwards') { console.error( 'The rendering order of is changing. ' + 'To be future compatible you must specify revealOrder="legacy_unstable-backwards" instead.', @@ -3314,18 +3309,7 @@ function validateTailOptions( 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".', - ); - } + // The default tail is now "hidden". } else if ( tailMode !== 'visible' && tailMode !== 'collapsed' && @@ -3338,6 +3322,7 @@ function validateTailOptions( tailMode, ); } else if ( + revealOrder != null && revealOrder !== 'forwards' && revealOrder !== 'backwards' && revealOrder !== 'unstable_legacy-backwards' @@ -3345,7 +3330,7 @@ function validateTailOptions( didWarnAboutTailOptions[cacheKey] = true; console.error( ' is only valid if revealOrder is ' + - '"forwards" or "backwards". ' + + '"forwards" (default) or "backwards". ' + 'Did you mean to specify revealOrder="forwards"?', tailMode, ); @@ -3449,30 +3434,6 @@ function updateSuspenseListComponent( workInProgress.memoizedState = null; } else { switch (revealOrder) { - case 'forwards': { - const lastContentRow = findLastContentRow(workInProgress.child); - let tail; - if (lastContentRow === null) { - // The whole list is part of the tail. - // TODO: We could fast path by just rendering the tail now. - tail = workInProgress.child; - workInProgress.child = null; - } else { - // Disconnect the tail rows after the content row. - // We're going to render them separately later. - tail = lastContentRow.sibling; - lastContentRow.sibling = null; - } - initSuspenseListRenderState( - workInProgress, - false, // isBackwards - tail, - lastContentRow, - tailMode, - treeForkCount, - ); - break; - } case 'backwards': case 'unstable_legacy-backwards': { // We're going to find the first row that has existing content. @@ -3517,10 +3478,37 @@ function updateSuspenseListComponent( ); break; } - default: { - // The default reveal order is the same as not having + case 'independent': { + // The "independent" reveal order is the same as not having // a boundary. workInProgress.memoizedState = null; + break; + } + // The default is now forwards. + case 'forwards': + default: { + const lastContentRow = findLastContentRow(workInProgress.child); + let tail; + if (lastContentRow === null) { + // The whole list is part of the tail. + // TODO: We could fast path by just rendering the tail now. + tail = workInProgress.child; + workInProgress.child = null; + } else { + // Disconnect the tail rows after the content row. + // We're going to render them separately later. + tail = lastContentRow.sibling; + lastContentRow.sibling = null; + } + initSuspenseListRenderState( + workInProgress, + false, // isBackwards + tail, + lastContentRow, + tailMode, + treeForkCount, + ); + break; } } } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index d3abcb466e..5ccf1b2ac7 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -698,30 +698,8 @@ function cutOffTailIfNeeded( return; } switch (renderState.tailMode) { - case 'hidden': { - // Any insertions at the end of the tail list after this point - // should be invisible. If there are already mounted boundaries - // anything before them are not considered for collapsing. - // Therefore we need to go through the whole tail to find if - // there are any. - let tailNode = renderState.tail; - let lastTailNode = null; - while (tailNode !== null) { - if (tailNode.alternate !== null) { - lastTailNode = tailNode; - } - tailNode = tailNode.sibling; - } - // Next we're simply going to delete all insertions after the - // last rendered item. - if (lastTailNode === null) { - // All remaining items in the tail are insertions. - renderState.tail = null; - } else { - // Detach the insertion after the last node that was already - // inserted. - lastTailNode.sibling = null; - } + case 'visible': { + // Everything should remain as it was. break; } case 'collapsed': { @@ -756,6 +734,34 @@ function cutOffTailIfNeeded( } break; } + // Hidden is now the default. + case 'hidden': + default: { + // Any insertions at the end of the tail list after this point + // should be invisible. If there are already mounted boundaries + // anything before them are not considered for collapsing. + // Therefore we need to go through the whole tail to find if + // there are any. + let tailNode = renderState.tail; + let lastTailNode = null; + while (tailNode !== null) { + if (tailNode.alternate !== null) { + lastTailNode = tailNode; + } + tailNode = tailNode.sibling; + } + // Next we're simply going to delete all insertions after the + // last rendered item. + if (lastTailNode === null) { + // All remaining items in the tail are insertions. + renderState.tail = null; + } else { + // Detach the insertion after the last node that was already + // inserted. + lastTailNode.sibling = null; + } + break; + } } } @@ -1795,7 +1801,8 @@ function completeWork( // This might have been modified. if ( renderState.tail === null && - renderState.tailMode === 'hidden' && + renderState.tailMode !== 'collapsed' && + renderState.tailMode !== 'visible' && !renderedTail.alternate && !getIsHydrating() // We don't cut it if we're hydrating. ) { diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js index 64ab4f29fc..695c369715 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js @@ -79,10 +79,7 @@ export function findFirstSuspended(row: Fiber): null | Fiber { node.tag === SuspenseListComponent && // Independent revealOrder can't be trusted because it doesn't // keep track of whether it suspended or not. - (node.memoizedProps.revealOrder === 'forwards' || - node.memoizedProps.revealOrder === 'backwards' || - node.memoizedProps.revealOrder === 'unstable_legacy-backwards' || - node.memoizedProps.revealOrder === 'together') + node.memoizedProps.revealOrder !== 'independent' ) { const didSuspend = (node.flags & DidCapture) !== NoFlags; if (didSuspend) { diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js index bfbf165dba..34600b7a67 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js @@ -218,7 +218,7 @@ describe('ReactSuspenseList', () => { }); // @gate enableSuspenseList - it('warns if no revealOrder is specified', async () => { + it('behaves as revealOrder=forwards by default', async () => { const A = createAsyncText('A'); const B = createAsyncText('B'); const C = createAsyncText('C'); @@ -239,54 +239,36 @@ describe('ReactSuspenseList', () => { ); } - await A.resolve(); - ReactNoop.render(); - await waitForAll([ - 'A', - 'Suspend! [B]', - 'Loading B', - 'Suspend! [C]', - 'Loading C', - // pre-warming - 'Suspend! [B]', - 'Suspend! [C]', - ]); + await waitForAll(['Suspend! [A]', 'Loading A']); - 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(null); + + await A.resolve(); + + await waitForAll(['A', 'Suspend! [B]', 'Loading B']); + + // Incremental loading is suspended. + jest.advanceTimersByTime(500); + + expect(ReactNoop).toMatchRenderedOutput(A); + + await act(() => B.resolve()); + assertLog(['B', 'Suspend! [C]', 'Loading C']); + + // Incremental loading is suspended. + jest.advanceTimersByTime(500); expect(ReactNoop).toMatchRenderedOutput( <> A - Loading B - Loading C + B , ); 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']); + assertLog(['C']); expect(ReactNoop).toMatchRenderedOutput( <> @@ -1699,26 +1681,65 @@ describe('ReactSuspenseList', () => { }); // @gate enableSuspenseList - it('warns if no tail option is specified', async () => { + it('behaves as tail=hidden if no tail option is specified', async () => { + const A = createAsyncText('A'); + const B = createAsyncText('B'); + const C = createAsyncText('C'); + 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 **)', - ]); + ReactNoop.render(); + + await waitForAll(['Suspend! [A]', 'Loading A']); + + expect(ReactNoop).toMatchRenderedOutput(null); + + await A.resolve(); + + await waitForAll(['A', 'Suspend! [B]', 'Loading B']); + + // Incremental loading is suspended. + jest.advanceTimersByTime(500); + + expect(ReactNoop).toMatchRenderedOutput(A); + + await act(() => B.resolve()); + assertLog(['B', 'Suspend! [C]', 'Loading C']); + + // Incremental loading is suspended. + jest.advanceTimersByTime(500); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + B + , + ); + + await act(() => C.resolve()); + assertLog(['C']); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + B + C + , + ); }); // @gate enableSuspenseList @@ -1758,7 +1779,7 @@ describe('ReactSuspenseList', () => { }); assertConsoleErrorDev([ ' is only valid if ' + - 'revealOrder is "forwards" or "backwards". ' + + 'revealOrder is "forwards" (default) or "backwards". ' + 'Did you mean to specify revealOrder="forwards"?' + '\n in SuspenseList (at **)' + '\n in Foo (at **)', diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js index 07408d64e8..156bacbe14 100644 --- a/packages/react-server/src/ReactFizzServer.js +++ b/packages/react-server/src/ReactFizzServer.js @@ -1913,7 +1913,7 @@ function renderSuspenseListRows( task: Task, keyPath: KeyNode, rows: Array, - revealOrder: 'forwards' | 'backwards' | 'unstable_legacy-backwards', + revealOrder: void | 'forwards' | 'backwards' | 'unstable_legacy-backwards', ): void { // This is a fork of renderChildrenArray that's aware of tracking rows. const prevKeyPath = task.keyPath; @@ -2098,11 +2098,7 @@ function renderSuspenseList( const revealOrder: SuspenseListRevealOrder = props.revealOrder; // TODO: Support tail hidden/collapsed modes. // const tailMode: SuspenseListTailMode = props.tail; - if ( - revealOrder === 'forwards' || - revealOrder === 'backwards' || - revealOrder === 'unstable_legacy-backwards' - ) { + if (revealOrder !== 'independent' && revealOrder !== 'together') { // For ordered reveal, we need to produce rows from the children. if (isArray(children)) { renderSuspenseListRows(request, task, keyPath, children, revealOrder);