Skip to content

Commit 967ca13

Browse files
sebmarkbageAndyPengc12
authored andcommitted
[Fizz] Don't pop the replay stack if we've already rendered past an element (facebook#27513)
This is the same problem as we had with keyPath before where if the element itself suspends, we have to restore the replay node to what it was before, however, if something below the element suspends we shouldn't pop it because that will pop it back up the stack. Instead of passing replay as an argument to every renderElement function, I use a hack to compare if the node is still the same as the one we tried to render, then that means we haven't stepped down into the child yet. Maybe this is not quite correct because in theory you could have a recursive node that just renders itself over and over until some context bails out. This solves an issue where if you suspended in an element it would retry trying to replay from that element but using the postponed state from the root.
1 parent ce1e825 commit 967ca13

File tree

2 files changed

+73
-6
lines changed

2 files changed

+73
-6
lines changed

packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1126,4 +1126,65 @@ describe('ReactDOMFizzStaticBrowser', () => {
11261126
// Client rendered
11271127
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);
11281128
});
1129+
1130+
// @gate enablePostpone
1131+
it('can suspend in a replayed component several layers deep', async () => {
1132+
let prerendering = true;
1133+
function Postpone() {
1134+
if (prerendering) {
1135+
React.unstable_postpone();
1136+
}
1137+
return 'Hello';
1138+
}
1139+
1140+
let resolve;
1141+
const promise = new Promise(r => (resolve = r));
1142+
function Delay({children}) {
1143+
if (!prerendering) {
1144+
React.use(promise);
1145+
}
1146+
return children;
1147+
}
1148+
1149+
// This wrapper will cause us to do one destructive render past this.
1150+
function Outer({children}) {
1151+
return children;
1152+
}
1153+
1154+
function App() {
1155+
return (
1156+
<div>
1157+
<Outer>
1158+
<Delay>
1159+
<Suspense fallback="Loading...">
1160+
<Postpone />
1161+
</Suspense>
1162+
</Delay>
1163+
</Outer>
1164+
</div>
1165+
);
1166+
}
1167+
1168+
const prerendered = await ReactDOMFizzStatic.prerender(<App />);
1169+
expect(prerendered.postponed).not.toBe(null);
1170+
1171+
await readIntoContainer(prerendered.prelude);
1172+
1173+
prerendering = false;
1174+
1175+
const resumedPromise = ReactDOMFizzServer.resume(
1176+
<App />,
1177+
JSON.parse(JSON.stringify(prerendered.postponed)),
1178+
);
1179+
1180+
await jest.runAllTimers();
1181+
1182+
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);
1183+
1184+
await resolve();
1185+
1186+
await readIntoContainer(await resumedPromise);
1187+
1188+
expect(getVisibleChildren(container)).toEqual(<div>Hello</div>);
1189+
});
11291190
});

packages/react-server/src/ReactFizzServer.js

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1967,6 +1967,7 @@ function replayElement(
19671967
}
19681968
const childNodes = node[2];
19691969
const childSlots = node[3];
1970+
const currentNode = task.node;
19701971
task.replay = {nodes: childNodes, slots: childSlots, pendingTasks: 1};
19711972
try {
19721973
renderElement(
@@ -1988,25 +1989,29 @@ function replayElement(
19881989
"The tree doesn't match so React will fallback to client rendering.",
19891990
);
19901991
}
1992+
task.replay.pendingTasks--;
19911993
} catch (x) {
19921994
if (
19931995
typeof x === 'object' &&
19941996
x !== null &&
19951997
(x === SuspenseException || typeof x.then === 'function')
19961998
) {
19971999
// Suspend
2000+
if (task.node === currentNode) {
2001+
// This same element suspended so we need to pop the replay we just added.
2002+
task.replay = replay;
2003+
}
19982004
throw x;
19992005
}
2006+
task.replay.pendingTasks--;
20002007
// Unlike regular render, we don't terminate the siblings if we error
20012008
// during a replay. That's because this component didn't actually error
20022009
// in the original prerender. What's unable to complete is the child
20032010
// replay nodes which might be Suspense boundaries which are able to
20042011
// absorb the error and we can still continue with siblings.
20052012
erroredReplay(request, task.blockedBoundary, x, childNodes, childSlots);
2006-
} finally {
2007-
task.replay.pendingTasks--;
2008-
task.replay = replay;
20092013
}
2014+
task.replay = replay;
20102015
} else {
20112016
// Let's double check that the component type matches.
20122017
if (type !== REACT_SUSPENSE_TYPE) {
@@ -2370,6 +2375,7 @@ function replayFragment(
23702375
"The tree doesn't match so React will fallback to client rendering.",
23712376
);
23722377
}
2378+
task.replay.pendingTasks--;
23732379
} catch (x) {
23742380
if (
23752381
typeof x === 'object' &&
@@ -2379,17 +2385,16 @@ function replayFragment(
23792385
// Suspend
23802386
throw x;
23812387
}
2388+
task.replay.pendingTasks--;
23822389
// Unlike regular render, we don't terminate the siblings if we error
23832390
// during a replay. That's because this component didn't actually error
23842391
// in the original prerender. What's unable to complete is the child
23852392
// replay nodes which might be Suspense boundaries which are able to
23862393
// absorb the error and we can still continue with siblings.
23872394
// This is an error, stash the component stack if it is null.
23882395
erroredReplay(request, task.blockedBoundary, x, childNodes, childSlots);
2389-
} finally {
2390-
task.replay.pendingTasks--;
2391-
task.replay = replay;
23922396
}
2397+
task.replay = replay;
23932398
// We finished rendering this node, so now we can consume this
23942399
// slot. This must happen after in case we rerender this task.
23952400
replayNodes.splice(j, 1);
@@ -2432,6 +2437,7 @@ function renderChildrenArray(
24322437
// We need to use the non-destructive form so that we can safely pop back
24332438
// up and render the sibling if something suspends.
24342439
const resumeSegmentID = resumeSlots[i];
2440+
// TODO: If this errors we should still continue with the next sibling.
24352441
if (typeof resumeSegmentID === 'number') {
24362442
resumeNode(request, task, resumeSegmentID, node, i);
24372443
// We finished rendering this node, so now we can consume this

0 commit comments

Comments
 (0)