Repository: facebook/react
Pull request: [Fizz] Fix for failing id overwrites for postpone

When we postpone during a render we inject a new segment synchronously which we postpone. That gets assigned an ID so we can refer to it immediately in the postponed state.

When we do that, the parent segment may complete later even though it's also synchronous. If that ends up not having any content in it, it'll inline into the child and that will override the child's segment id which is not correct since it was already assigned one.

To fix this, we simply opt-out of the optimization in that case which is unfortunate because we'll generate many more unnecessary empty segments. So we should come up with a new strategy for segment id assignment but this fixes the bug.

Changed files:
filename: packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js
patch: @@ -1495,4 +1495,51 @@ describe('ReactDOMFizzStaticBrowser', () => {
       'hello',
     ]);
   });
+
+  // @gate enablePostpone
+  it('can render a deep list of single components where one postpones', async () => {
+    let isPrerendering = true;
+    function Outer({children}) {
+      return children;
+    }
+
+    function Middle({children}) {
+      return children;
+    }
+
+    function Inner() {
+      if (isPrerendering) {
+        React.unstable_postpone();
+      }
+      return 'hello';
+    }
+
+    function App() {
+      return (
+        <Suspense fallback="loading...">
+          <Outer>
+            <Middle>
+              <Inner />
+            </Middle>
+          </Outer>
+        </Suspense>
+      );
+    }
+
+    const prerendered = await ReactDOMFizzStatic.prerender(<App />);
+    const postponedState = JSON.stringify(prerendered.postponed);
+
+    await readIntoContainer(prerendered.prelude);
+    expect(getVisibleChildren(container)).toEqual('loading...');
+
+    isPrerendering = false;
+
+    const dynamic = await ReactDOMFizzServer.resume(
+      <App />,
+      JSON.parse(postponedState),
+    );
+
+    await readIntoContainer(dynamic);
+    expect(getVisibleChildren(container)).toEqual('hello');
+  });
 });

filename: packages/react-server/src/ReactFizzServer.js
patch: @@ -2542,15 +2542,22 @@ function trackPostpone(
 
     const children: Array<ReplayNode> = [];
     if (boundaryKeyPath === keyPath && task.childIndex === -1) {
-      // Since we postponed directly in the Suspense boundary we can't have written anything
-      // to its segment. Therefore this will end up becoming the root segment.
-      segment.id = boundary.rootSegmentID;
+      // Assign ID
+      if (segment.id === -1) {
+        if (segment.parentFlushed) {
+          // If this segment's parent was already flushed, it means we really just
+          // skipped the parent and this segment is now the root.
+          segment.id = boundary.rootSegmentID;
+        } else {
+          segment.id = request.nextSegmentId++;
+        }
+      }
       // We postponed directly inside the Suspense boundary so we mark this for resuming.
       const boundaryNode: ReplaySuspenseBoundary = [
         boundaryKeyPath[1],
         boundaryKeyPath[2],
         children,
-        boundary.rootSegmentID,
+        segment.id,
         fallbackReplayNode,
         boundary.rootSegmentID,
       ];
@@ -3264,7 +3271,8 @@ function queueCompletedSegment(
   if (
     segment.chunks.length === 0 &&
     segment.children.length === 1 &&
-    segment.children[0].boundary === null
+    segment.children[0].boundary === null &&
+    segment.children[0].id === -1
   ) {
     // This is an empty segment. There's nothing to write, so we can instead transfer the ID
     // to the child. That way any existing references point to the child.