Bugfix: SuspenseList incorrectly forces a fallback

packages/react-reconciler/src/ReactFiberBeginWork.js

@@ -2167,6 +2167,8 @@ function shouldRemainOnFallback(
  // If we're already showing a fallback, there are cases where we need to
  // remain on that fallback regardless of whether the content has resolved.
  // For example, SuspenseList coordinates when nested content appears.
+ // TODO: For compatibility with offscreen prerendering, this should also check
+ // whether the current fiber (if it exists) was visible in the previous tree.
  if (current !== null) {
  const suspenseState: SuspenseState = current.memoizedState;
  if (suspenseState === null) {

packages/react-reconciler/src/ReactFiberSuspenseContext.js

@@ -45,6 +45,14 @@ export function pushPrimaryTreeSuspenseHandler(handler: Fiber): void {
  const current = handler.alternate;
  const props: SuspenseProps = handler.pendingProps;
 
+ // Shallow Suspense context fields, like ForceSuspenseFallback, should only be
+ // propagated a single level. For example, when ForceSuspenseFallback is set,
+ // it should only force the nearest Suspense boundary into fallback mode.
+ pushSuspenseListContext(
+ handler,
+ setDefaultShallowSuspenseListContext(suspenseStackCursor.current),
+ );
+
  // Experimental feature: Some Suspense boundaries are marked as having an
  // undesirable fallback state. These have special behavior where we only
  // activate the fallback if there's no other boundary on the stack that we can
@@ -100,6 +108,11 @@ export function pushFallbackTreeSuspenseHandler(fiber: Fiber): void {
 
 export function pushOffscreenSuspenseHandler(fiber: Fiber): void {
  if (fiber.tag === OffscreenComponent) {
+ // A SuspenseList context is only pushed here to avoid a push/pop mismatch.
+ // Reuse the current value on the stack.
+ // TODO: We can avoid needing to push here by by forking popSuspenseHandler
+ // into separate functions for Suspense and Offscreen.
+ pushSuspenseListContext(fiber, suspenseStackCursor.current);
  push(suspenseHandlerStackCursor, fiber, fiber);
  if (shellBoundary !== null) {
  // A parent boundary is showing a fallback, so we've already rendered
@@ -122,6 +135,7 @@ export function pushOffscreenSuspenseHandler(fiber: Fiber): void {
 }
 
 export function reuseSuspenseHandlerOnStack(fiber: Fiber) {
+ pushSuspenseListContext(fiber, suspenseStackCursor.current);
  push(suspenseHandlerStackCursor, getSuspenseHandler(), fiber);
 }
 
@@ -135,6 +149,7 @@ export function popSuspenseHandler(fiber: Fiber): void {
  // Popping back into the shell.
  shellBoundary = null;
  }
+ popSuspenseListContext(fiber);
 }
 
 // SuspenseList context

packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js

@@ -3003,4 +3003,108 @@ describe('ReactSuspenseList', () => {
  </>,
  );
  });
+
+ // @gate enableSuspenseList
+ it(
+ 'regression test: SuspenseList should never force boundaries deeper than ' +
+ 'a single level into fallback mode',
+ async () => {
+ const A = createAsyncText('A');
+
+ function UnreachableFallback() {
+ throw new Error('Should never be thrown!');
+ }
+
+ function Repro({update}) {
+ return (
+ <SuspenseList revealOrder="forwards">
+ {update && (
+ <Suspense fallback={<Text text="Loading A..." />}>
+ <A />
+ </Suspense>
+ )}
+ <Suspense fallback={<Text text="Loading B..." />}>
+ {update ? (
+ <Suspense fallback={<UnreachableFallback />}>
+ <Text text="B2" />
+ </Suspense>
+ ) : (
+ <Text text="B1" />
+ )}
+ </Suspense>
+ <Suspense fallback={<Text text="Loading C..." />}>
+ <Text text="C" />
+ </Suspense>
+ {update && (
+ <Suspense fallback={<Text text="Loading D..." />}>
+ <Text text="D" />
+ </Suspense>
+ )}
+ </SuspenseList>
+ );
+ }
+
+ // Initial mount. Only two rows are mounted, B and C.
+ const root = ReactNoop.createRoot();
+ await act(() => root.render(<Repro update={false} />));
+ assertLog(['B1', 'C']);
+ expect(root).toMatchRenderedOutput(
+ <>
+ <span>B1</span>
+ <span>C</span>
+ </>,
+ );
+
+ // During the update, a few things happen simultaneously:
+ // - A new row, A, is inserted into the head. This row suspends.
+ // - The context in row B is replaced. The new content contains a nested
+ // Suspense boundary.
+ // - A new row, D, is inserted into the tail.
+ await act(() => root.render(<Repro update={true} />));
+ assertLog([
+ // During the first pass, the new row, A, suspends. This means any new
+ // rows in the tail should be forced into fallback mode.
+ 'Suspend! [A]',
+ 'Loading A...',
+ 'B2',
+ 'C',
+
+ // A second pass is used to render the fallbacks in the tail.
+ //
+ // Rows B and C were already mounted, so they should not be forced into
+ // fallback mode.
+ //
+ // In the regression that this test was written for, the inner
+ // Suspense boundary around B2 was incorrectly activated. Only the
+ // nearest fallbacks per row should be activated, and only if they
+ // haven't already mounted.
+ 'Loading A...',
+ 'B2',
+ 'C',
+
+ // D is part of the tail, so it should show a fallback.
+ 'Loading D...',
+ ]);
+ expect(root).toMatchRenderedOutput(
+ <>
+ <span>Loading A...</span>
+ <span>B2</span>
+ <span>C</span>
+ <span>Loading D...</span>
+ </>,
+ );
+
+ // Now finish loading A.
+ await act(() => A.resolve());
+ assertLog(['A', 'D']);
+ expect(root).toMatchRenderedOutput(
+ <>
+ <span>A</span>
+ <span>B2</span>
+ <span>C</span>
+ <span>D</span>
+ </>,
+ );
+ },
+ );
 });