[Fizz] Support deeply nested Suspense inside fallback (#33467)

gnoff · web-flow · commit 142aa0744d0e · 2025-06-06T11:59:15.000-07:00
When deeply nested Suspense boundaries inside a fallback of another
boundary resolve it is possible to encounter situations where you either
attempt to flush an aborted Segment or you have a boundary without any
root segment. We intended for both of these conditions to be impossible
to arrive at legitimately however it turns out in this situation you
can. The fix is two-fold

1. allow flushing aborted segments by simply skipping them. This does
remove some protection against future misconfiguraiton of React because
it is no longer an invariant that you hsould never attempt to flush an
aborted segment but there are legitimate cases where this can come up
and simply omitting the segment is fine b/c we know that the user will
never observe this. A semantically better solution would be to avoid
flushing boudaries inside an unneeded fallback but to do this we would
need to track all boundaries inside a fallback or create back pointers
which add to memory overhead and possibly make GC harder to do
efficiently. By flushing extra we're maintaining status quo and only
suffer in performance not with broken semantics.

2. when queuing completed segments allow for queueing aborted segments
and if we are eliding the enqueued segment allow for child segments that
are errored to be enqueued too. This will mean that we can maintain the
invariant that a boundary must have a root segment the first time we
flush it, it just might be aborted (see point 1 above).

This change has two seemingly similar test cases to exercise this fix.
The reason we need both is that when you have empty segments you hit
different code paths within Fizz and so each one (without this fix)
triggers a different error pathway.

This change also includes a fix to our tests where we were not
appropriately setting CSPnonce back to null at the start of each test so
in some contexts scripts would not run for some tests
diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
@@ -88,6 +88,7 @@ describe('ReactDOMFizzServer', () => {
       setTimeout(cb);
     container = document.getElementById('container');
 
+    CSPnonce = null;
     Scheduler = require('scheduler');
     React = require('react');
     ReactDOM = require('react-dom');
@@ -10447,4 +10448,110 @@ describe('ReactDOMFizzServer', () => {
       </html>,
     );
   });
+
+  it('should not error when discarding deeply nested Suspense boundaries in a parent fallback partially complete before the parent boundary resolves', async () => {
+    let resolve1;
+    const promise1 = new Promise(r => (resolve1 = r));
+    let resolve2;
+    const promise2 = new Promise(r => (resolve2 = r));
+    const promise3 = new Promise(r => {});
+
+    function Use({children, promise}) {
+      React.use(promise);
+      return children;
+    }
+    function App() {
+      return (
+        <div>
+          <Suspense
+            fallback={
+              <div>
+                <Suspense fallback="Loading...">
+                  <div>
+                    <Use promise={promise1}>
+                      <div>
+                        <Suspense fallback="Loading more...">
+                          <div>
+                            <Use promise={promise3}>
+                              <div>deep fallback</div>
+                            </Use>
+                          </div>
+                        </Suspense>
+                      </div>
+                    </Use>
+                  </div>
+                </Suspense>
+              </div>
+            }>
+            <Use promise={promise2}>Success!</Use>
+          </Suspense>
+        </div>
+      );
+    }
+
+    await act(() => {
+      const {pipe} = renderToPipeableStream(<App />);
+      pipe(writable);
+    });
+
+    expect(getVisibleChildren(container)).toEqual(
+      <div>
+        <div>Loading...</div>
+      </div>,
+    );
+
+    await act(() => {
+      resolve1('resolved');
+      resolve2('resolved');
+    });
+
+    expect(getVisibleChildren(container)).toEqual(<div>Success!</div>);
+  });
+
+  it('should not error when discarding deeply nested Suspense boundaries in a parent fallback partially complete before the parent boundary resolves with empty segments', async () => {
+    let resolve1;
+    const promise1 = new Promise(r => (resolve1 = r));
+    let resolve2;
+    const promise2 = new Promise(r => (resolve2 = r));
+    const promise3 = new Promise(r => {});
+
+    function Use({children, promise}) {
+      React.use(promise);
+      return children;
+    }
+    function App() {
+      return (
+        <div>
+          <Suspense
+            fallback={
+              <Suspense fallback="Loading...">
+                <Use promise={promise1}>
+                  <Suspense fallback="Loading more...">
+                    <Use promise={promise3}>
+                      <div>deep fallback</div>
+                    </Use>
+                  </Suspense>
+                </Use>
+              </Suspense>
+            }>
+            <Use promise={promise2}>Success!</Use>
+          </Suspense>
+        </div>
+      );
+    }
+
+    await act(() => {
+      const {pipe} = renderToPipeableStream(<App />);
+      pipe(writable);
+    });
+
+    expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);
+
+    await act(() => {
+      resolve1('resolved');
+      resolve2('resolved');
+    });
+
+    expect(getVisibleChildren(container)).toEqual(<div>Success!</div>);
+  });
 });
diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js
@@ -25,7 +25,7 @@ let SuspenseList;
 let textCache;
 let loadCache;
 let writable;
-const CSPnonce = null;
+let CSPnonce = null;
 let container;
 let buffer = '';
 let hasErrored = false;
@@ -69,6 +69,7 @@ describe('ReactDOMFloat', () => {
       setTimeout(cb);
     container = document.getElementById('container');
 
+    CSPnonce = null;
     React = require('react');
     ReactDOM = require('react-dom');
     ReactDOMClient = require('react-dom/client');
diff --git a/packages/react-server/src/ReactFizzServer.js b/packages/react-server/src/ReactFizzServer.js
@@ -4918,7 +4918,11 @@ function queueCompletedSegment(
     const childSegment = segment.children[0];
     childSegment.id = segment.id;
     childSegment.parentFlushed = true;
-    if (childSegment.status === COMPLETED) {
+    if (
+      childSegment.status === COMPLETED ||
+      childSegment.status === ABORTED ||
+      childSegment.status === ERRORED
+    ) {
       queueCompletedSegment(boundary, childSegment);
     }
   } else {
@@ -4989,7 +4993,7 @@ function finishedTask(
         // Our parent segment already flushed, so we need to schedule this segment to be emitted.
         // If it is a segment that was aborted, we'll write other content instead so we don't need
         // to emit it.
-        if (segment.status === COMPLETED) {
+        if (segment.status === COMPLETED || segment.status === ABORTED) {
           queueCompletedSegment(boundary, segment);
         }
       }
@@ -5058,7 +5062,7 @@ function finishedTask(
         // Our parent already flushed, so we need to schedule this segment to be emitted.
         // If it is a segment that was aborted, we'll write other content instead so we don't need
         // to emit it.
-        if (segment.status === COMPLETED) {
+        if (segment.status === COMPLETED || segment.status === ABORTED) {
           queueCompletedSegment(boundary, segment);
           const completedSegments = boundary.completedSegments;
           if (completedSegments.length === 1) {
@@ -5575,6 +5579,9 @@ function flushSubtree(
       }
       return r;
     }
+    case ABORTED: {
+      return true;
+    }
     default: {
       throw new Error(
         'Aborted, errored or already flushed boundaries should not be flushed again. This is a bug in React.',
