Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fiber] Refactor Commit Phase into Separate Functions for Before Mutation/Mutation/Layout #31930

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
9 changes: 3 additions & 6 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,23 +232,20 @@ let inProgressLanes: Lanes | null = null;
let inProgressRoot: FiberRoot | null = null;

let focusedInstanceHandle: null | Fiber = null;
let shouldFireAfterActiveInstanceBlur: boolean = false;
export let shouldFireAfterActiveInstanceBlur: boolean = false;

export function commitBeforeMutationEffects(
root: FiberRoot,
firstChild: Fiber,
): boolean {
): void {
focusedInstanceHandle = prepareForCommit(root.containerInfo);
shouldFireAfterActiveInstanceBlur = false;

nextEffect = firstChild;
commitBeforeMutationEffects_begin();

// We no longer need to track the active instance fiber
const shouldFire = shouldFireAfterActiveInstanceBlur;
shouldFireAfterActiveInstanceBlur = false;
focusedInstanceHandle = null;

return shouldFire;
}

function commitBeforeMutationEffects_begin() {
Expand Down
16 changes: 5 additions & 11 deletions packages/react-reconciler/src/ReactFiberLane.js
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,11 @@ function getHighestPriorityLanes(lanes: Lanes | Lane): Lanes {
}
}

export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
export function getNextLanes(
root: FiberRoot,
wipLanes: Lanes,
rootHasPendingCommit: boolean,
): Lanes {
// Early bailout if there's no pending work left.
const pendingLanes = root.pendingLanes;
if (pendingLanes === NoLanes) {
Expand All @@ -246,16 +250,6 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
// a brief amount of time (i.e. below the "Just Noticeable Difference"
// threshold).
//
// TODO: finishedLanes is also set when a Suspensey resource, like CSS or
// images, suspends during the commit phase. (We could detect that here by
// checking for root.cancelPendingCommit.) These are also expected to resolve
// quickly, because of preloading, but theoretically they could block forever
// like in a normal "suspend indefinitely" scenario. In the future, we should
// consider only blocking for up to some time limit before discarding the
// commit in favor of prerendering. If we do discard a pending commit, then
// the commit phase callback should act as a ping to try the original
// render again.
const rootHasPendingCommit = root.finishedLanes !== NoLanes;

// Do not work on any idle work until all the non-idle work has finished,
// even if the work is suspended.
Expand Down
2 changes: 0 additions & 2 deletions packages/react-reconciler/src/ReactFiberRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@ function FiberRootNode(
this.pendingChildren = null;
this.current = null;
this.pingCache = null;
this.finishedWork = null;
this.timeoutHandle = noTimeout;
this.cancelPendingCommit = null;
this.context = null;
Expand All @@ -76,7 +75,6 @@ function FiberRootNode(
this.pingedLanes = NoLanes;
this.warmLanes = NoLanes;
this.expiredLanes = NoLanes;
this.finishedLanes = NoLanes;
this.errorRecoveryDisabledLanes = NoLanes;
this.shellSuspendCounter = 0;

Expand Down
11 changes: 11 additions & 0 deletions packages/react-reconciler/src/ReactFiberRootScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ import {
scheduleMicrotask,
shouldAttemptEagerTransition,
trackSchedulerEvent,
noTimeout,
} from './ReactFiberConfig';

import ReactSharedInternals from 'shared/ReactSharedInternals';
Expand Down Expand Up @@ -207,11 +208,15 @@ function flushSyncWorkAcrossRoots_impl(
const workInProgressRoot = getWorkInProgressRoot();
const workInProgressRootRenderLanes =
getWorkInProgressRootRenderLanes();
const rootHasPendingCommit =
root.cancelPendingCommit !== null ||
root.timeoutHandle !== noTimeout;
const nextLanes = getNextLanes(
root,
root === workInProgressRoot
? workInProgressRootRenderLanes
: NoLanes,
rootHasPendingCommit,
);
if (
includesSyncLane(nextLanes) &&
Expand Down Expand Up @@ -331,6 +336,8 @@ function scheduleTaskForRootDuringMicrotask(
const pendingPassiveEffectsLanes = getPendingPassiveEffectsLanes();
const workInProgressRoot = getWorkInProgressRoot();
const workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes();
const rootHasPendingCommit =
root.cancelPendingCommit !== null || root.timeoutHandle !== noTimeout;
const nextLanes =
enableYieldingBeforePassive && root === rootWithPendingPassiveEffects
? // This will schedule the callback at the priority of the lane but we used to
Expand All @@ -341,6 +348,7 @@ function scheduleTaskForRootDuringMicrotask(
: getNextLanes(
root,
root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes,
rootHasPendingCommit,
);

const existingCallbackNode = root.callbackNode;
Expand Down Expand Up @@ -484,9 +492,12 @@ function performWorkOnRootViaSchedulerTask(
// it's available).
const workInProgressRoot = getWorkInProgressRoot();
const workInProgressRootRenderLanes = getWorkInProgressRootRenderLanes();
const rootHasPendingCommit =
root.cancelPendingCommit !== null || root.timeoutHandle !== noTimeout;
const lanes = getNextLanes(
root,
root === workInProgressRoot ? workInProgressRootRenderLanes : NoLanes,
rootHasPendingCommit,
);
if (lanes === NoLanes) {
// No more work on this root.
Expand Down
Loading
Loading