From 8bb74ce4aac2a98a69060a571b254c8df5102562 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 30 Oct 2025 10:07:32 -0400 Subject: [PATCH] Switch the default revealOrder to "forwards" and tail "hidden" --- .../ReactDOMFizzSuspenseList-test.js | 21 ++- .../react-reconciler/src/ReactChildFiber.js | 3 +- .../src/ReactFiberBeginWork.js | 80 +++++------- .../src/ReactFiberCompleteWork.js | 29 +++-- .../src/ReactFiberSuspenseComponent.js | 5 +- .../src/__tests__/ReactSuspenseList-test.js | 121 ++++++++++-------- packages/react-server/src/ReactFizzServer.js | 8 +- 7 files changed, 142 insertions(+), 125 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzSuspenseList-test.js index 74db51d242960..f434bf3cda8ec 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,31 +157,38 @@ 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(
- A + Loading A Loading B Loading C
, ); - await serverAct(() => C.resolve()); - assertLog(['C']); + await serverAct(() => A.resolve()); + assertLog(['A']); expect(getVisibleChildren(container)).toEqual(
A Loading B - C + Loading C
, ); diff --git a/packages/react-reconciler/src/ReactChildFiber.js b/packages/react-reconciler/src/ReactChildFiber.js index f0d9c2e012e8e..8daf7fde4b4e0 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 7251e52dd6108..32ecd33edca65 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 d3abcb466e585..5ccf1b2ac7ac8 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -698,7 +698,11 @@ function cutOffTailIfNeeded( return; } switch (renderState.tailMode) { - case 'hidden': { + case 'visible': { + // Everything should remain as it was. + break; + } + case 'collapsed': { // 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. @@ -716,7 +720,13 @@ function cutOffTailIfNeeded( // last rendered item. if (lastTailNode === null) { // All remaining items in the tail are insertions. - renderState.tail = null; + if (!hasRenderedATailFallback && renderState.tail !== null) { + // We suspended during the head. We want to show at least one + // row at the tail. So we'll keep on and cut off the rest. + renderState.tail.sibling = null; + } else { + renderState.tail = null; + } } else { // Detach the insertion after the last node that was already // inserted. @@ -724,7 +734,9 @@ function cutOffTailIfNeeded( } break; } - case 'collapsed': { + // 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. @@ -742,13 +754,7 @@ function cutOffTailIfNeeded( // last rendered item. if (lastTailNode === null) { // All remaining items in the tail are insertions. - if (!hasRenderedATailFallback && renderState.tail !== null) { - // We suspended during the head. We want to show at least one - // row at the tail. So we'll keep on and cut off the rest. - renderState.tail.sibling = null; - } else { - renderState.tail = null; - } + renderState.tail = null; } else { // Detach the insertion after the last node that was already // inserted. @@ -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 64ab4f29fcd14..695c36971546d 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 bfbf165dbaba6..34600b7a67283 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', () => { ); } + ReactNoop.render(); + + await waitForAll(['Suspend! [A]', 'Loading A']); + + expect(ReactNoop).toMatchRenderedOutput(null); + await A.resolve(); - ReactNoop.render(); + await waitForAll(['A', 'Suspend! [B]', 'Loading B']); - await waitForAll([ - 'A', - 'Suspend! [B]', - 'Loading B', - 'Suspend! [C]', - 'Loading C', - // pre-warming - 'Suspend! [B]', - 'Suspend! [C]', - ]); + // Incremental loading is suspended. + jest.advanceTimersByTime(500); - assertConsoleErrorDev([ - 'The default for the prop is changing. ' + - 'To be future compatible you must explictly specify either ' + - '"independent" (the current default), "together", "forwards" or "legacy_unstable-backwards".' + - '\n in SuspenseList (at **)' + - '\n in Foo (at **)', - ]); + expect(ReactNoop).toMatchRenderedOutput(A); - expect(ReactNoop).toMatchRenderedOutput( - <> - A - Loading B - Loading C - , - ); + await act(() => B.resolve()); + assertLog(['B', 'Suspend! [C]', 'Loading C']); - await act(() => C.resolve()); - assertLog( - gate('alwaysThrottleRetries') - ? ['Suspend! [B]', 'C', 'Suspend! [B]'] - : ['C'], - ); + // Incremental loading is suspended. + jest.advanceTimersByTime(500); expect(ReactNoop).toMatchRenderedOutput( <> A - Loading B - C + B , ); - await act(() => B.resolve()); - assertLog(['B']); + await act(() => C.resolve()); + 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 07408d64e8817..156bacbe1480c 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);