Bugfix: SuspenseList incorrectly forces a fallback (#26453)

Fixes a bug in SuspenseList that @kassens found when deploying React to
Meta. In some scenarios, SuspenseList would force the fallback of a
deeply nested Suspense boundary into fallback mode, which should never
happen under any circumstances — SuspenseList should only affect the
nearest descendent Suspense boundaries, without going deeper.

The cause was that the internal ForceSuspenseFallback context flag was
not being properly reset when it reached the nearest Suspense boundary.
It should only be propagated shallowly.

We didn't discover this earlier because the scenario where it happens is
not that common. To trigger the bug, you need to insert a new Suspense
boundary into an already-mounted row of the list. But often when a new
Suspense boundary is rendered, it suspends and shows a fallback, anyway,
because its content hasn't loaded yet.

Another reason we didn't discover this earlier is because there was
another bug that was accidentally masking it, which was fixed by #25922.
When that fix landed, it revealed this bug.

The SuspenseList implementation is complicated but I'm not too concerned
with the current messiness. It's an experimental API, and we intend to
release it soon, but there are some known flaws and missing features
that we need to address first regardless. We'll likely end up rewriting
most of it.

Co-authored-by: Jan Kassens <[email protected]>

DiffTrain build for [51a7c45](51a7c45)

Author: acdlite

compiled/facebook-www/REVISION

@@ -1 +1 @@
-afb3d51dc6310f0dbeffdd303eb3c6895e6f7db0
+51a7c45f8799cab903693fcfdd305ce84ba15273

compiled/facebook-www/React-dev.modern.js

@@ -27,7 +27,7 @@ if (
 }
           "use strict";
 
-var ReactVersion = "18.3.0-www-modern-b3a4de20";
+var ReactVersion = "18.3.0-www-modern-e3dcc95d";
 
 // ATTENTION
 // When adding new symbols to this file,

compiled/facebook-www/ReactART-dev.classic.js

@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
   return self;
 }
 
-var ReactVersion = "18.3.0-www-classic-ac5037f5";
+var ReactVersion = "18.3.0-www-classic-7d98bb40";
 
 var LegacyRoot = 0;
 var ConcurrentRoot = 1;
@@ -7166,7 +7166,14 @@ function getShellBoundary() {
 function pushPrimaryTreeSuspenseHandler(handler) {
   // TODO: Pass as argument
   var current = handler.alternate;
-  var props = handler.pendingProps; // Experimental feature: Some Suspense boundaries are marked as having an
+  var props = 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
   // use instead.
@@ -7218,6 +7225,11 @@ function pushFallbackTreeSuspenseHandler(fiber) {
 }
 function pushOffscreenSuspenseHandler(fiber) {
   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);
@@ -7240,6 +7252,7 @@ function pushOffscreenSuspenseHandler(fiber) {
   }
 }
 function reuseSuspenseHandlerOnStack(fiber) {
+  pushSuspenseListContext(fiber, suspenseStackCursor.current);
   push(suspenseHandlerStackCursor, getSuspenseHandler(), fiber);
 }
 function getSuspenseHandler() {
@@ -7252,6 +7265,8 @@ function popSuspenseHandler(fiber) {
     // Popping back into the shell.
     shellBoundary = null;
   }
+
+  popSuspenseListContext(fiber);
 } // SuspenseList context
 // TODO: Move to a separate module? We may change the SuspenseList
 // implementation to hide/show in the commit phase, anyway.
@@ -14551,6 +14566,8 @@ function shouldRemainOnFallback(current, workInProgress, renderLanes) {
   // 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) {
     var suspenseState = current.memoizedState;
 

compiled/facebook-www/ReactART-dev.modern.js

@@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
   return self;
 }
 
-var ReactVersion = "18.3.0-www-modern-d114403d";
+var ReactVersion = "18.3.0-www-modern-34dd0fe8";
 
 var LegacyRoot = 0;
 var ConcurrentRoot = 1;
@@ -6922,7 +6922,14 @@ function getShellBoundary() {
 function pushPrimaryTreeSuspenseHandler(handler) {
   // TODO: Pass as argument
   var current = handler.alternate;
-  var props = handler.pendingProps; // Experimental feature: Some Suspense boundaries are marked as having an
+  var props = 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
   // use instead.
@@ -6974,6 +6981,11 @@ function pushFallbackTreeSuspenseHandler(fiber) {
 }
 function pushOffscreenSuspenseHandler(fiber) {
   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);
@@ -6996,6 +7008,7 @@ function pushOffscreenSuspenseHandler(fiber) {
   }
 }
 function reuseSuspenseHandlerOnStack(fiber) {
+  pushSuspenseListContext(fiber, suspenseStackCursor.current);
   push(suspenseHandlerStackCursor, getSuspenseHandler(), fiber);
 }
 function getSuspenseHandler() {
@@ -7008,6 +7021,8 @@ function popSuspenseHandler(fiber) {
     // Popping back into the shell.
     shellBoundary = null;
   }
+
+  popSuspenseListContext(fiber);
 } // SuspenseList context
 // TODO: Move to a separate module? We may change the SuspenseList
 // implementation to hide/show in the commit phase, anyway.
@@ -14246,6 +14261,8 @@ function shouldRemainOnFallback(current, workInProgress, renderLanes) {
   // 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) {
     var suspenseState = current.memoizedState;
 

compiled/facebook-www/ReactART-prod.classic.js

@@ -2248,8 +2248,10 @@ function popHiddenContext() {
 var suspenseHandlerStackCursor = createCursor(null),
   shellBoundary = null;
 function pushPrimaryTreeSuspenseHandler(handler) {
-  var current = handler.alternate;
-  !0 !== handler.pendingProps.unstable_avoidThisFallback ||
+  var current = handler.alternate,
+    props = handler.pendingProps;
+  push(suspenseStackCursor, suspenseStackCursor.current & 1);
+  !0 !== props.unstable_avoidThisFallback ||
   (null !== current && null === currentTreeHiddenStackCursor.current)
     ? (push(suspenseHandlerStackCursor, handler),
       null === shellBoundary &&
@@ -2262,20 +2264,26 @@ function pushPrimaryTreeSuspenseHandler(handler) {
 }
 function pushOffscreenSuspenseHandler(fiber) {
   if (22 === fiber.tag) {
-    if ((push(suspenseHandlerStackCursor, fiber), null === shellBoundary)) {
+    if (
+      (push(suspenseStackCursor, suspenseStackCursor.current),
+      push(suspenseHandlerStackCursor, fiber),
+      null === shellBoundary)
+    ) {
       var current = fiber.alternate;
       null !== current &&
         null !== current.memoizedState &&
         (shellBoundary = fiber);
     }
-  } else reuseSuspenseHandlerOnStack();
+  } else reuseSuspenseHandlerOnStack(fiber);
 }
 function reuseSuspenseHandlerOnStack() {
+  push(suspenseStackCursor, suspenseStackCursor.current);
   push(suspenseHandlerStackCursor, suspenseHandlerStackCursor.current);
 }
 function popSuspenseHandler(fiber) {
   pop(suspenseHandlerStackCursor);
   shellBoundary === fiber && (shellBoundary = null);
+  pop(suspenseStackCursor);
 }
 var suspenseStackCursor = createCursor(0);
 function findFirstSuspended(row) {
@@ -3857,12 +3865,12 @@ function updateOffscreenComponent(current, workInProgress, renderLanes) {
     }
     pushTransition(workInProgress, nextProps, nextIsDetached);
     pushHiddenContext(workInProgress, prevState);
-    reuseSuspenseHandlerOnStack();
+    reuseSuspenseHandlerOnStack(workInProgress);
     workInProgress.memoizedState = null;
   } else
     null !== current && pushTransition(workInProgress, null, null),
       reuseHiddenContextOnStack(),
-      reuseSuspenseHandlerOnStack();
+      reuseSuspenseHandlerOnStack(workInProgress);
   reconcileChildren(current, workInProgress, nextChildren, renderLanes);
   return workInProgress.child;
 }
@@ -4232,7 +4240,7 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
     didSuspend = nextProps.fallback;
     if (showFallback)
       return (
-        reuseSuspenseHandlerOnStack(),
+        reuseSuspenseHandlerOnStack(workInProgress),
         (current = mountSuspenseFallbackChildren(
           workInProgress,
           current,
@@ -4263,7 +4271,7 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
       );
     if ("number" === typeof nextProps.unstable_expectedLoadTime)
       return (
-        reuseSuspenseHandlerOnStack(),
+        reuseSuspenseHandlerOnStack(workInProgress),
         (current = mountSuspenseFallbackChildren(
           workInProgress,
           current,
@@ -4294,7 +4302,7 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
       );
   }
   if (showFallback) {
-    reuseSuspenseHandlerOnStack();
+    reuseSuspenseHandlerOnStack(workInProgress);
     showFallback = nextProps.fallback;
     didSuspend = workInProgress.mode;
     JSCompiler_temp = current.child;
@@ -4469,12 +4477,12 @@ function updateDehydratedSuspenseComponent(
       );
     if (null !== workInProgress.memoizedState)
       return (
-        reuseSuspenseHandlerOnStack(),
+        reuseSuspenseHandlerOnStack(workInProgress),
         (workInProgress.child = current.child),
         (workInProgress.flags |= 128),
         null
       );
-    reuseSuspenseHandlerOnStack();
+    reuseSuspenseHandlerOnStack(workInProgress);
     suspenseState = nextProps.fallback;
     didSuspend = workInProgress.mode;
     nextProps = createFiberFromOffscreen(
@@ -9957,19 +9965,19 @@ var slice = Array.prototype.slice,
     };
     return Text;
   })(React.Component),
-  devToolsConfig$jscomp$inline_1157 = {
+  devToolsConfig$jscomp$inline_1168 = {
     findFiberByHostInstance: function () {
       return null;
     },
     bundleType: 0,
-    version: "18.3.0-www-classic-f03f0ae3",
+    version: "18.3.0-www-classic-52e77af5",
     rendererPackageName: "react-art"
   };
-var internals$jscomp$inline_1332 = {
-  bundleType: devToolsConfig$jscomp$inline_1157.bundleType,
-  version: devToolsConfig$jscomp$inline_1157.version,
-  rendererPackageName: devToolsConfig$jscomp$inline_1157.rendererPackageName,
-  rendererConfig: devToolsConfig$jscomp$inline_1157.rendererConfig,
+var internals$jscomp$inline_1343 = {
+  bundleType: devToolsConfig$jscomp$inline_1168.bundleType,
+  version: devToolsConfig$jscomp$inline_1168.version,
+  rendererPackageName: devToolsConfig$jscomp$inline_1168.rendererPackageName,
+  rendererConfig: devToolsConfig$jscomp$inline_1168.rendererConfig,
   overrideHookState: null,
   overrideHookStateDeletePath: null,
   overrideHookStateRenamePath: null,
@@ -9986,26 +9994,26 @@ var internals$jscomp$inline_1332 = {
     return null === fiber ? null : fiber.stateNode;
   },
   findFiberByHostInstance:
-    devToolsConfig$jscomp$inline_1157.findFiberByHostInstance ||
+    devToolsConfig$jscomp$inline_1168.findFiberByHostInstance ||
     emptyFindFiberByHostInstance,
   findHostInstancesForRefresh: null,
   scheduleRefresh: null,
   scheduleRoot: null,
   setRefreshHandler: null,
   getCurrentFiber: null,
-  reconcilerVersion: "18.3.0-www-classic-f03f0ae3"
+  reconcilerVersion: "18.3.0-www-classic-52e77af5"
 };
 if ("undefined" !== typeof __REACT_DEVTOOLS_GLOBAL_HOOK__) {
-  var hook$jscomp$inline_1333 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
+  var hook$jscomp$inline_1344 = __REACT_DEVTOOLS_GLOBAL_HOOK__;
   if (
-    !hook$jscomp$inline_1333.isDisabled &&
-    hook$jscomp$inline_1333.supportsFiber
+    !hook$jscomp$inline_1344.isDisabled &&
+    hook$jscomp$inline_1344.supportsFiber
   )
     try {
-      (rendererID = hook$jscomp$inline_1333.inject(
-        internals$jscomp$inline_1332
+      (rendererID = hook$jscomp$inline_1344.inject(
+        internals$jscomp$inline_1343
       )),
-        (injectedHook = hook$jscomp$inline_1333);
+        (injectedHook = hook$jscomp$inline_1344);
     } catch (err) {}
 }
 var Path = Mode$1.Path;