[Fizz] Fix for failing id overwrites for postpone (#27684)
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. Co-authored-by: Josh Story <story@hey.com>
Sebastian Markbåge committed
Nov 9, 2023 at 22:52 UTC
0e352ea01c0209b6c5e23f0e857af4e01c783024
2 files changed
+60
-5
packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js
+47
@@ -1495,4 +1495,51 @@ describe('ReactDOMFizzStaticBrowser', () => {
1495
'hello',
1496
]);
1497
});
1498
+
1499
+ // @gate enablePostpone
1500
+ it('can render a deep list of single components where one postpones', async () => {
1501
+ let isPrerendering = true;
1502
+ function Outer({children}) {
1503
+ return children;
1504
+ }
1505
+
1506
+ function Middle({children}) {
1507
+ return children;
1508
+ }
1509
+
1510
+ function Inner() {
1511
+ if (isPrerendering) {
1512
+ React.unstable_postpone();
1513
+ }
1514
+ return 'hello';
1515
+ }
1516
+
1517
+ function App() {
1518
+ return (
1519
+ <Suspense fallback="loading...">
1520
+ <Outer>
1521
+ <Middle>
1522
+ <Inner />
1523
+ </Middle>
1524
+ </Outer>
1525
+ </Suspense>
1526
+ );
1527
+ }
1528
+
1529
+ const prerendered = await ReactDOMFizzStatic.prerender(<App />);
1530
+ const postponedState = JSON.stringify(prerendered.postponed);
1531
+
1532
+ await readIntoContainer(prerendered.prelude);
1533
+ expect(getVisibleChildren(container)).toEqual('loading...');
1534
+
1535
+ isPrerendering = false;
1536
+
1537
+ const dynamic = await ReactDOMFizzServer.resume(
1538
+ <App />,
1539
+ JSON.parse(postponedState),
1540
+ );
1541
+
1542
+ await readIntoContainer(dynamic);
1543
+ expect(getVisibleChildren(container)).toEqual('hello');
1544
+ });
1545
});
packages/react-server/src/ReactFizzServer.js
+13
-5
@@ -2542,15 +2542,22 @@ function trackPostpone(
2542
2543
const children: Array<ReplayNode> = [];
2544
if (boundaryKeyPath === keyPath && task.childIndex === -1) {
2545
- // Since we postponed directly in the Suspense boundary we can't have written anything
2546
- // to its segment. Therefore this will end up becoming the root segment.
2547
- segment.id = boundary.rootSegmentID;
2545
+ // Assign ID
2546
+ if (segment.id === -1) {
2547
+ if (segment.parentFlushed) {
2548
+ // If this segment's parent was already flushed, it means we really just
2549
+ // skipped the parent and this segment is now the root.
2550
+ segment.id = boundary.rootSegmentID;
2551
+ } else {
2552
+ segment.id = request.nextSegmentId++;
2553
+ }
2554
+ }
2555
// We postponed directly inside the Suspense boundary so we mark this for resuming.
2556
const boundaryNode: ReplaySuspenseBoundary = [
2557
boundaryKeyPath[1],
2558
boundaryKeyPath[2],
2559
children,
2553
- boundary.rootSegmentID,
2560
+ segment.id,
2561
fallbackReplayNode,
2562
boundary.rootSegmentID,
2563
];
@@ -3264,7 +3271,8 @@ function queueCompletedSegment(
3271
if (
3272
segment.chunks.length === 0 &&
3273
segment.children.length === 1 &&
3267
- segment.children[0].boundary === null
3274
+ segment.children[0].boundary === null &&
3275
+ segment.children[0].id === -1
3276
) {
3277
// This is an empty segment. There's nothing to write, so we can instead transfer the ID
3278
// to the child. That way any existing references point to the child.