Skip to content

Commit 989af12

Browse files
jackpopeacdliterickhanlonii
authored
Make prerendering always non-blocking with fix (#31452)
We've previously failed to land this change due to some internal apps seeing infinite render loops due to external store state updates during render. It turns out that since the `renderWasConcurrent` var was moved into the do block, the sync render triggered from the external store check was stuck with a `RootSuspended` `exitStatus`. So this is not unique to sibling prerendering but more generally related to how we handle update to a sync external store during render. We've tested this build against local repros which now render without crashes. We will try to add a unit test to cover the scenario as well. --------- Co-authored-by: Andrew Clark <[email protected]> Co-authored-by: Rick Hanlon <[email protected]>
1 parent 5c56b87 commit 989af12

File tree

7 files changed

+428
-129
lines changed

7 files changed

+428
-129
lines changed

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

+1-1
Original file line numberDiff line numberDiff line change
@@ -744,7 +744,7 @@ describe('ReactDOMFiberAsync', () => {
744744
// Because it suspended, it remains on the current path
745745
expect(div.textContent).toBe('/path/a');
746746
});
747-
assertLog([]);
747+
assertLog(gate('enableSiblingPrerendering') ? ['Suspend! [/path/b]'] : []);
748748

749749
await act(async () => {
750750
resolvePromise();

packages/react-reconciler/src/ReactFiberLane.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -765,12 +765,14 @@ export function markRootSuspended(
765765
root: FiberRoot,
766766
suspendedLanes: Lanes,
767767
spawnedLane: Lane,
768-
didSkipSuspendedSiblings: boolean,
768+
didAttemptEntireTree: boolean,
769769
) {
770+
// TODO: Split this into separate functions for marking the root at the end of
771+
// a render attempt versus suspending while the root is still in progress.
770772
root.suspendedLanes |= suspendedLanes;
771773
root.pingedLanes &= ~suspendedLanes;
772774

773-
if (enableSiblingPrerendering && !didSkipSuspendedSiblings) {
775+
if (enableSiblingPrerendering && didAttemptEntireTree) {
774776
// Mark these lanes as warm so we know there's nothing else to work on.
775777
root.warmLanes |= suspendedLanes;
776778
} else {

packages/react-reconciler/src/ReactFiberRootScheduler.js

+16-4
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
disableSchedulerTimeoutInWorkLoop,
1919
enableProfilerTimer,
2020
enableProfilerNestedUpdatePhase,
21+
enableSiblingPrerendering,
2122
} from 'shared/ReactFeatureFlags';
2223
import {
2324
NoLane,
@@ -29,6 +30,7 @@ import {
2930
markStarvedLanesAsExpired,
3031
claimNextTransitionLane,
3132
getNextLanesToFlushSync,
33+
checkIfRootIsPrerendering,
3234
} from './ReactFiberLane';
3335
import {
3436
CommitContext,
@@ -206,7 +208,10 @@ function flushSyncWorkAcrossRoots_impl(
206208
? workInProgressRootRenderLanes
207209
: NoLanes,
208210
);
209-
if (includesSyncLane(nextLanes)) {
211+
if (
212+
includesSyncLane(nextLanes) &&
213+
!checkIfRootIsPrerendering(root, nextLanes)
214+
) {
210215
// This root has pending sync work. Flush it now.
211216
didPerformSomeWork = true;
212217
performSyncWorkOnRoot(root, nextLanes);
@@ -341,7 +346,13 @@ function scheduleTaskForRootDuringMicrotask(
341346
}
342347

343348
// Schedule a new callback in the host environment.
344-
if (includesSyncLane(nextLanes)) {
349+
if (
350+
includesSyncLane(nextLanes) &&
351+
// If we're prerendering, then we should use the concurrent work loop
352+
// even if the lanes are synchronous, so that prerendering never blocks
353+
// the main thread.
354+
!(enableSiblingPrerendering && checkIfRootIsPrerendering(root, nextLanes))
355+
) {
345356
// Synchronous work is always flushed at the end of the microtask, so we
346357
// don't need to schedule an additional task.
347358
if (existingCallbackNode !== null) {
@@ -375,9 +386,10 @@ function scheduleTaskForRootDuringMicrotask(
375386

376387
let schedulerPriorityLevel;
377388
switch (lanesToEventPriority(nextLanes)) {
389+
// Scheduler does have an "ImmediatePriority", but now that we use
390+
// microtasks for sync work we no longer use that. Any sync work that
391+
// reaches this path is meant to be time sliced.
378392
case DiscreteEventPriority:
379-
schedulerPriorityLevel = ImmediateSchedulerPriority;
380-
break;
381393
case ContinuousEventPriority:
382394
schedulerPriorityLevel = UserBlockingSchedulerPriority;
383395
break;

0 commit comments

Comments
 (0)