Skip to content

Commit

Permalink
[Re-land] Make prerendering always non-blocking (#31268)
Browse files Browse the repository at this point in the history
Follows #31238

___

This is a partial re-land of
#31056. We saw breakages surface
after the original land and had to revert. Now that they've been fixed,
let's try this again. This time we'll split up the commits to give us
more control of testing and rollout internally.

Original PR: #31056
Original Commit:
4c71025
Revert PR: #31080

Commit description:

> When a synchronous update suspends, and we prerender the siblings, the
prerendering should be non-blocking so that we can immediately restart
once the data arrives.
>
> This happens automatically when there's a Suspense boundary, because
we immediately commit the boundary and then proceed to a Retry render,
which are always concurrent. When there's not a Suspense boundary, there
is no Retry, so we need to take care to switch from the synchronous work
loop to the concurrent one, to enable time slicing.

Co-authored-by: Andrew Clark <git@andrewclark.io>
  • Loading branch information
jackpope and acdlite authored Oct 15, 2024
1 parent 8382581 commit 6c4bbc7
Show file tree
Hide file tree
Showing 6 changed files with 278 additions and 126 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ describe('ReactDOMFiberAsync', () => {
// Because it suspended, it remains on the current path
expect(div.textContent).toBe('/path/a');
});
assertLog([]);
assertLog(gate('enableSiblingPrerendering') ? ['Suspend! [/path/b]'] : []);

await act(async () => {
resolvePromise();
Expand Down
6 changes: 4 additions & 2 deletions packages/react-reconciler/src/ReactFiberLane.js
Original file line number Diff line number Diff line change
Expand Up @@ -765,12 +765,14 @@ export function markRootSuspended(
root: FiberRoot,
suspendedLanes: Lanes,
spawnedLane: Lane,
didSkipSuspendedSiblings: boolean,
didAttemptEntireTree: boolean,
) {
// TODO: Split this into separate functions for marking the root at the end of
// a render attempt versus suspending while the root is still in progress.
root.suspendedLanes |= suspendedLanes;
root.pingedLanes &= ~suspendedLanes;

if (enableSiblingPrerendering && !didSkipSuspendedSiblings) {
if (enableSiblingPrerendering && didAttemptEntireTree) {
// Mark these lanes as warm so we know there's nothing else to work on.
root.warmLanes |= suspendedLanes;
} else {
Expand Down
20 changes: 16 additions & 4 deletions packages/react-reconciler/src/ReactFiberRootScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
disableSchedulerTimeoutInWorkLoop,
enableProfilerTimer,
enableProfilerNestedUpdatePhase,
enableSiblingPrerendering,
} from 'shared/ReactFeatureFlags';
import {
NoLane,
Expand All @@ -29,6 +30,7 @@ import {
markStarvedLanesAsExpired,
claimNextTransitionLane,
getNextLanesToFlushSync,
checkIfRootIsPrerendering,
} from './ReactFiberLane';
import {
CommitContext,
Expand Down Expand Up @@ -206,7 +208,10 @@ function flushSyncWorkAcrossRoots_impl(
? workInProgressRootRenderLanes
: NoLanes,
);
if (includesSyncLane(nextLanes)) {
if (
includesSyncLane(nextLanes) &&
!checkIfRootIsPrerendering(root, nextLanes)
) {
// This root has pending sync work. Flush it now.
didPerformSomeWork = true;
performSyncWorkOnRoot(root, nextLanes);
Expand Down Expand Up @@ -341,7 +346,13 @@ function scheduleTaskForRootDuringMicrotask(
}

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

let schedulerPriorityLevel;
switch (lanesToEventPriority(nextLanes)) {
// Scheduler does have an "ImmediatePriority", but now that we use
// microtasks for sync work we no longer use that. Any sync work that
// reaches this path is meant to be time sliced.
case DiscreteEventPriority:
schedulerPriorityLevel = ImmediateSchedulerPriority;
break;
case ContinuousEventPriority:
schedulerPriorityLevel = UserBlockingSchedulerPriority;
break;
Expand Down
Loading

0 comments on commit 6c4bbc7

Please sign in to comment.