[Fiber] SuspenseList with "hidden" tail row should "catch" suspense

packages/react-reconciler/src/ReactFiberBeginWork.js

@@ -3397,6 +3397,13 @@ function updateSuspenseListComponent(
 
   let suspenseContext: SuspenseContext = suspenseStackCursor.current;
 
+  if (workInProgress.flags & DidCapture) {
+    // This is the second pass after having suspended in a row. Proceed directly
+    // to the complete phase.
+    pushSuspenseListContext(workInProgress, suspenseContext);
+    return null;
+  }
+
   const shouldForceFallback = hasSuspenseListContext(
     suspenseContext,
     (ForceSuspenseFallback: SuspenseContext),
@@ -4011,6 +4018,14 @@ function attemptEarlyBailoutIfNoScheduledUpdate(
       break;
     }
     case SuspenseListComponent: {
+      if (workInProgress.flags & DidCapture) {
+        // Second pass caught.
+        return updateSuspenseListComponent(
+          current,
+          workInProgress,
+          renderLanes,
+        );
+      }
       const didSuspendBefore = (current.flags & DidCapture) !== NoFlags;
 
       let hasChildWork = includesSomeLane(

packages/react-reconciler/src/ReactFiberCompleteWork.js

@@ -138,6 +138,7 @@ import {
   popSuspenseListContext,
   popSuspenseHandler,
   pushSuspenseListContext,
+  pushSuspenseListCatch,
   setShallowSuspenseListContext,
   ForceSuspenseFallback,
   setDefaultShallowSuspenseListContext,
@@ -765,6 +766,17 @@ function cutOffTailIfNeeded(
   }
 }
 
+function isOnlyNewMounts(tail: Fiber): boolean {
+  let fiber: null | Fiber = tail;
+  while (fiber !== null) {
+    if (fiber.alternate !== null) {
+      return false;
+    }
+    fiber = fiber.sibling;
+  }
+  return true;
+}
+
 function bubbleProperties(completedWork: Fiber) {
   const didBailout =
     completedWork.alternate !== null &&
@@ -1855,7 +1867,10 @@ function completeWork(
       if (renderState.tail !== null) {
         // We still have tail rows to render.
         // Pop a row.
+        // TODO: Consider storing the first of the new mount tail in the state so
+        // that we don't have to recompute this for every row in the list.
         const next = renderState.tail;
+        const onlyNewMounts = isOnlyNewMounts(next);
         renderState.rendering = next;
         renderState.tail = next.sibling;
         renderState.renderingStartTime = now();
@@ -1874,7 +1889,26 @@ function completeWork(
           suspenseContext =
             setDefaultShallowSuspenseListContext(suspenseContext);
         }
-        pushSuspenseListContext(workInProgress, suspenseContext);
+        if (
+          renderState.tailMode === 'visible' ||
+          renderState.tailMode === 'collapsed' ||
+          !onlyNewMounts ||
+          // TODO: While hydrating, we still let it suspend the parent. Tail mode hidden has broken
+          // hydration anyway right now but this preserves the previous semantics out of caution.
+          // Once proper hydration is implemented, this special case should be removed as it should
+          // never be needed.
+          getIsHydrating()
+        ) {
+          pushSuspenseListContext(workInProgress, suspenseContext);
+        } else {
+          // If we are rendering in 'hidden' (default) tail mode, then we if we suspend in the
+          // tail itself, we can delete it rather than suspend the parent. So we act as a catch in that
+          // case. For 'collapsed' we need to render at least one in suspended state, after which we'll
+          // have cut off the rest to never attempt it so it never hits this case.
+          // If this is an updated node, we cannot delete it from the tail so it's effectively visible.
+          // As a consequence, if it resuspends it actually suspends the parent by taking the other path.
+          pushSuspenseListCatch(workInProgress, suspenseContext);
+        }
         // Do a pass over the next row.
         if (getIsHydrating()) {
           // Re-apply tree fork since we popped the tree fork context in the beginning of this function.

packages/react-reconciler/src/ReactFiberSuspenseContext.js

@@ -48,9 +48,10 @@ export function pushPrimaryTreeSuspenseHandler(handler: Fiber): void {
   // 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,
+  push(
+    suspenseStackCursor,
     setDefaultShallowSuspenseListContext(suspenseStackCursor.current),
+    handler,
   );
 
   // Experimental feature: Some Suspense boundaries are marked as having an
@@ -113,7 +114,7 @@ export function pushDehydratedActivitySuspenseHandler(fiber: Fiber): void {
   // Reuse the current value on the stack.
   // TODO: We can avoid needing to push here by by forking popSuspenseHandler
   // into separate functions for Activity, Suspense and Offscreen.
-  pushSuspenseListContext(fiber, suspenseStackCursor.current);
+  push(suspenseStackCursor, suspenseStackCursor.current, fiber);
   push(suspenseHandlerStackCursor, fiber, fiber);
   if (shellBoundary === null) {
     // We can contain any suspense inside the Activity boundary.
@@ -127,7 +128,7 @@ export function pushOffscreenSuspenseHandler(fiber: Fiber): void {
     // Reuse the current value on the stack.
     // TODO: We can avoid needing to push here by by forking popSuspenseHandler
     // into separate functions for Activity, Suspense and Offscreen.
-    pushSuspenseListContext(fiber, suspenseStackCursor.current);
+    push(suspenseStackCursor, suspenseStackCursor.current, fiber);
     push(suspenseHandlerStackCursor, fiber, fiber);
     if (shellBoundary === null) {
       // We're rendering hidden content. If it suspends, we can handle it by
@@ -141,7 +142,7 @@ export function pushOffscreenSuspenseHandler(fiber: Fiber): void {
 }
 
 export function reuseSuspenseHandlerOnStack(fiber: Fiber) {
-  pushSuspenseListContext(fiber, suspenseStackCursor.current);
+  push(suspenseStackCursor, suspenseStackCursor.current, fiber);
   push(suspenseHandlerStackCursor, getSuspenseHandler(), fiber);
 }
 
@@ -155,7 +156,7 @@ export function popSuspenseHandler(fiber: Fiber): void {
     // Popping back into the shell.
     shellBoundary = null;
   }
-  popSuspenseListContext(fiber);
+  pop(suspenseStackCursor, fiber);
 }
 
 // SuspenseList context
@@ -201,9 +202,32 @@ export function pushSuspenseListContext(
   fiber: Fiber,
   newContext: SuspenseContext,
 ): void {
+  // Push the current handler in this case since we're not catching at the SuspenseList
+  // for typical rows.
+  const handlerOnStack = suspenseHandlerStackCursor.current;
+  push(suspenseHandlerStackCursor, handlerOnStack, fiber);
+  push(suspenseStackCursor, newContext, fiber);
+}
+
+export function pushSuspenseListCatch(
+  fiber: Fiber,
+  newContext: SuspenseContext,
+): void {
+  // In this case we do want to handle catching suspending on the actual boundary itself.
+  // This is used for rows that are allowed to be hidden anyway.
+  push(suspenseHandlerStackCursor, fiber, fiber);
   push(suspenseStackCursor, newContext, fiber);
+  if (shellBoundary === null) {
+    // We can contain the effects to hiding the current row.
+    shellBoundary = fiber;
+  }
 }
 
 export function popSuspenseListContext(fiber: Fiber): void {
   pop(suspenseStackCursor, fiber);
+  pop(suspenseHandlerStackCursor, fiber);
+  if (shellBoundary === fiber) {
+    // Popping back into the shell.
+    shellBoundary = null;
+  }
 }

packages/react-reconciler/src/ReactFiberThrow.js

@@ -27,6 +27,7 @@ import {
   ActivityComponent,
   SuspenseComponent,
   OffscreenComponent,
+  SuspenseListComponent,
 } from './ReactWorkTags';
 import {
   DidCapture,
@@ -400,7 +401,8 @@ function throwException(
       if (suspenseBoundary !== null) {
         switch (suspenseBoundary.tag) {
           case ActivityComponent:
-          case SuspenseComponent: {
+          case SuspenseComponent:
+          case SuspenseListComponent: {
             // If this suspense/activity boundary is not already showing a fallback, mark
             // the in-progress render as suspended. We try to perform this logic
             // as soon as soon as possible during the render phase, so the work
@@ -561,6 +563,13 @@ function throwException(
     // Instead of surfacing the error, find the nearest Suspense boundary
     // and render it again without hydration.
     if (hydrationBoundary !== null) {
+      if (__DEV__) {
+        if (hydrationBoundary.tag === SuspenseListComponent) {
+          console.error(
+            'SuspenseList should never catch while hydrating. This is a bug in React.',
+          );
+        }
+      }
       if ((hydrationBoundary.flags & ShouldCapture) === NoFlags) {
         // Set a flag to indicate that we should try rendering the normal
         // children again, not the fallback.

packages/react-reconciler/src/ReactFiberUnwindWork.js

@@ -11,7 +11,10 @@ import type {ReactContext} from 'shared/ReactTypes';
 import type {Fiber, FiberRoot} from './ReactInternalTypes';
 import type {Lanes} from './ReactFiberLane';
 import type {ActivityState} from './ReactFiberActivityComponent';
-import type {SuspenseState} from './ReactFiberSuspenseComponent';
+import type {
+  SuspenseState,
+  SuspenseListRenderState,
+} from './ReactFiberSuspenseComponent';
 import type {Cache} from './ReactFiberCacheComponent';
 import type {TracingMarkerInstance} from './ReactFiberTracingMarkerComponent';
 
@@ -31,7 +34,7 @@ import {
   CacheComponent,
   TracingMarkerComponent,
 } from './ReactWorkTags';
-import {DidCapture, NoFlags, ShouldCapture} from './ReactFiberFlags';
+import {DidCapture, NoFlags, ShouldCapture, Update} from './ReactFiberFlags';
 import {NoMode, ProfileMode} from './ReactTypeOfMode';
 import {
   enableProfilerTimer,
@@ -180,8 +183,27 @@ function unwindWork(
     }
     case SuspenseListComponent: {
       popSuspenseListContext(workInProgress);
-      // SuspenseList doesn't actually catch anything. It should've been
+      // SuspenseList doesn't normally catch anything. It should've been
       // caught by a nested boundary. If not, it should bubble through.
+      const flags = workInProgress.flags;
+      if (flags & ShouldCapture) {
+        workInProgress.flags = (flags & ~ShouldCapture) | DidCapture;
+        // If we caught something on the SuspenseList itself it's because
+        // we want to ignore something. Re-enter the cycle and handle it
+        // in the complete phase.
+        const renderState: null | SuspenseListRenderState =
+          workInProgress.memoizedState;
+        if (renderState !== null) {
+          // Cut off any remaining tail work and don't commit the rendering one.
+          // This assumes that we have already confirmed that none of these are
+          // already mounted.
+          renderState.rendering = null;
+          renderState.tail = null;
+        }
+        // Schedule the commit phase to attach retry listeners.
+        workInProgress.flags |= Update;
+        return workInProgress;
+      }
       return null;
     }
     case HostPortal:

packages/react-reconciler/src/ReactFiberWorkLoop.js

@@ -1355,7 +1355,7 @@ function finishConcurrentRender(
       throw new Error('Root did not complete. This is a bug in React.');
     }
     case RootSuspendedWithDelay: {
-      if (!includesOnlyTransitions(lanes)) {
+      if (!includesOnlyTransitions(lanes) && !includesOnlyRetries(lanes)) {
         // Commit the placeholder.
         break;
       }