From 8c2fa93953101b6b5f7b5b8e3f4a3484cef2ada7 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 5 Jan 2025 21:14:33 -0500 Subject: [PATCH 01/12] Add missing flag check --- packages/react-reconciler/src/ReactFiberReconciler.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 98ef2ef93fc46..70640d68475af 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -41,6 +41,7 @@ import isArray from 'shared/isArray'; import { enableSchedulingProfiler, enableHydrationLaneScheduling, + disableLegacyMode, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import { @@ -364,7 +365,7 @@ export function updateContainerSync( parentComponent: ?React$Component, callback: ?Function, ): Lane { - if (container.tag === LegacyRoot) { + if (!disableLegacyMode && container.tag === LegacyRoot) { flushPassiveEffects(); } const current = container.current; From 8216803dcac27ff6b15f6e4f1e5e105f606b1c9b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 5 Jan 2025 21:21:23 -0500 Subject: [PATCH 02/12] Use module scope to pass arguments to mutation and layout effects In the same way we already do for passive effects. --- .../src/ReactFiberWorkLoop.js | 88 +++++++++++-------- 1 file changed, 49 insertions(+), 39 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index ce253f5436cea..cc423bc9da11b 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -426,7 +426,6 @@ let workInProgressRootDidIncludeRecursiveRenderUpdate: boolean = false; // variable from the one for renders because the commit phase may run // concurrently to a render phase. let didIncludeCommitPhaseUpdate: boolean = false; - // The most recent time we either committed a fallback, or when a fallback was // filled in with the resolved UI. This lets us throttle the appearance of new // content as it streams in, to minimize jank. @@ -617,11 +616,25 @@ export function getRenderTargetTime(): number { let legacyErrorBoundariesThatAlreadyFailed: Set | null = null; +type SuspendedCommitReason = 0 | 1 | 2; +const IMMEDIATE_COMMIT = 0; +const SUSPENDED_COMMIT = 1; +const THROTTLED_COMMIT = 2; + +const NO_PENDING_EFFECTS = 0; +const PENDING_MUTATION_PHASE = 1; +const PENDING_LAYOUT_PHASE = 2; +const PENDING_PASSIVE_PHASE = 3; let rootWithPendingPassiveEffects: FiberRoot | null = null; +let pendingEffectsStatus: 0 | 1 | 2 | 3 = 0; +let pendingFinishedWork: Fiber = (null: any); let pendingPassiveEffectsLanes: Lanes = NoLanes; let pendingPassiveEffectsRemainingLanes: Lanes = NoLanes; let pendingPassiveEffectsRenderEndTime: number = -0; // Profiling-only let pendingPassiveTransitions: Array | null = null; +let pendingRecoverableErrors: null | Array> = null; +let pendingDidIncludeRenderPhaseUpdate: boolean = false; +let pendingSuspendedCommitReason: SuspendedCommitReason = IMMEDIATE_COMMIT; // Profiling-only // Use these to prevent an infinite loop of nested updates const NESTED_UPDATE_LIMIT = 50; @@ -3120,11 +3133,6 @@ function unwindUnitOfWork(unitOfWork: Fiber, skipSiblings: boolean): void { workInProgress = null; } -type SuspendedCommitReason = 0 | 1 | 2; -const IMMEDIATE_COMMIT = 0; -const SUSPENDED_COMMIT = 1; -const THROTTLED_COMMIT = 2; - function commitRoot( root: FiberRoot, finishedWork: null | Fiber, @@ -3243,6 +3251,23 @@ function commitRoot( // times out. } + // workInProgressX might be overwritten, so we want + // to store it in pendingPassiveX until they get processed + // We need to pass this through as an argument to commitRoot + // because workInProgressX might have changed between + // the previous render and commit if we throttle the commit + // with setTimeout + pendingFinishedWork = finishedWork; + pendingPassiveEffectsLanes = lanes; + pendingPassiveEffectsRemainingLanes = remainingLanes; + pendingPassiveTransitions = transitions; + pendingRecoverableErrors = recoverableErrors; + pendingDidIncludeRenderPhaseUpdate = didIncludeRenderPhaseUpdate; + if (enableProfilerTimer) { + pendingPassiveEffectsRenderEndTime = completedRenderEndTime; + pendingSuspendedCommitReason = suspendedCommitReason; + } + // If there are pending passive effects, schedule a callback to process them. // Do this as early as possible, so it is queued before anything else that // might get scheduled in the commit phase. (See #16714.) @@ -3256,15 +3281,6 @@ function commitRoot( (finishedWork.subtreeFlags & PassiveMask) !== NoFlags || (finishedWork.flags & PassiveMask) !== NoFlags ) { - pendingPassiveEffectsRemainingLanes = remainingLanes; - pendingPassiveEffectsRenderEndTime = completedRenderEndTime; - // workInProgressTransitions might be overwritten, so we want - // to store it in pendingPassiveTransitions until they get processed - // We need to pass this through as an argument to commitRoot - // because workInProgressTransitions might have changed between - // the previous render and commit if we throttle the commit - // with setTimeout - pendingPassiveTransitions = transitions; if (enableYieldingBeforePassive) { // We don't schedule a separate task for flushing passive effects. // Instead, we just rely on ensureRootIsScheduled below to schedule @@ -3341,23 +3357,15 @@ function commitRoot( ReactSharedInternals.T = prevTransition; } } - flushMutationEffects(root, finishedWork, lanes); - flushLayoutEffects( - root, - finishedWork, - lanes, - recoverableErrors, - didIncludeRenderPhaseUpdate, - suspendedCommitReason, - completedRenderEndTime, - ); + pendingEffectsStatus = PENDING_MUTATION_PHASE; + flushMutationEffects(root); + pendingEffectsStatus = PENDING_LAYOUT_PHASE; + flushLayoutEffects(root); } -function flushMutationEffects( - root: FiberRoot, - finishedWork: Fiber, - lanes: Lanes, -): void { +function flushMutationEffects(root: FiberRoot): void { + const finishedWork = pendingFinishedWork; + const lanes = pendingPassiveEffectsLanes; const subtreeMutationHasEffects = (finishedWork.subtreeFlags & MutationMask) !== NoFlags; const rootMutationHasEffect = (finishedWork.flags & MutationMask) !== NoFlags; @@ -3394,15 +3402,14 @@ function flushMutationEffects( root.current = finishedWork; } -function flushLayoutEffects( - root: FiberRoot, - finishedWork: Fiber, - lanes: Lanes, - recoverableErrors: null | Array>, - didIncludeRenderPhaseUpdate: boolean, - suspendedCommitReason: SuspendedCommitReason, // Profiling-only - completedRenderEndTime: number, // Profiling-only -): void { +function flushLayoutEffects(root: FiberRoot): void { + const finishedWork = pendingFinishedWork; + const lanes = pendingPassiveEffectsLanes; + const completedRenderEndTime = pendingPassiveEffectsRenderEndTime; + const recoverableErrors = pendingRecoverableErrors; + const didIncludeRenderPhaseUpdate = pendingDidIncludeRenderPhaseUpdate; + const suspendedCommitReason = pendingSuspendedCommitReason; + const subtreeHasLayoutEffects = (finishedWork.subtreeFlags & LayoutMask) !== NoFlags; const rootHasLayoutEffect = (finishedWork.flags & LayoutMask) !== NoFlags; @@ -3458,11 +3465,13 @@ function flushLayoutEffects( if (rootDidHavePassiveEffects) { // This commit has passive effects. Stash a reference to them. But don't // schedule a callback until after flushing layout work. + pendingEffectsStatus = PENDING_PASSIVE_PHASE; rootWithPendingPassiveEffects = root; pendingPassiveEffectsLanes = lanes; } else { // There were no passive effects, so we can immediately release the cache // pool for this render. + pendingEffectsStatus = NO_PENDING_EFFECTS; releaseRootPooledCache(root, root.pendingLanes); if (__DEV__) { nestedPassiveUpdateCount = 0; @@ -3716,6 +3725,7 @@ function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) { const root = rootWithPendingPassiveEffects; const lanes = pendingPassiveEffectsLanes; + pendingEffectsStatus = NO_PENDING_EFFECTS; rootWithPendingPassiveEffects = null; // TODO: This is sometimes out of sync with rootWithPendingPassiveEffects. // Figure out why and fix it. It's not causing any known issues (probably From 4a93883f747c71c1d5ae9f4feb24988e5537ebc6 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 5 Jan 2025 21:38:56 -0500 Subject: [PATCH 03/12] Use status field instead of root to check the status of the commit --- .../react-reconciler/src/ReactFiberWorkLoop.js | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index cc423bc9da11b..e0807abc3c87a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -625,8 +625,8 @@ const NO_PENDING_EFFECTS = 0; const PENDING_MUTATION_PHASE = 1; const PENDING_LAYOUT_PHASE = 2; const PENDING_PASSIVE_PHASE = 3; -let rootWithPendingPassiveEffects: FiberRoot | null = null; let pendingEffectsStatus: 0 | 1 | 2 | 3 = 0; +let rootWithPendingPassiveEffects: FiberRoot = (null: any); let pendingFinishedWork: Fiber = (null: any); let pendingPassiveEffectsLanes: Lanes = NoLanes; let pendingPassiveEffectsRemainingLanes: Lanes = NoLanes; @@ -658,7 +658,9 @@ export function getWorkInProgressRootRenderLanes(): Lanes { } export function getRootWithPendingPassiveEffects(): FiberRoot | null { - return rootWithPendingPassiveEffects; + return pendingEffectsStatus === PENDING_PASSIVE_PHASE + ? rootWithPendingPassiveEffects + : null; } export function getPendingPassiveEffectsLanes(): Lanes { @@ -1635,7 +1637,7 @@ export function flushSyncFromReconciler(fn: (() => R) | void): R | void { // In legacy mode, we flush pending passive effects at the beginning of the // next event, not at the end of the previous one. if ( - rootWithPendingPassiveEffects !== null && + pendingEffectsStatus !== NO_PENDING_EFFECTS && !disableLegacyMode && rootWithPendingPassiveEffects.tag === LegacyRoot && (executionContext & (RenderContext | CommitContext)) === NoContext @@ -3158,7 +3160,7 @@ function commitRoot( // TODO: Might be better if `flushPassiveEffects` did not automatically // flush synchronous work at the end, to avoid factoring hazards like this. flushPassiveEffects(); - } while (rootWithPendingPassiveEffects !== null); + } while (pendingEffectsStatus !== NO_PENDING_EFFECTS); flushRenderPhaseStrictModeWarningsInDEV(); if ((executionContext & (RenderContext | CommitContext)) !== NoContext) { @@ -3682,7 +3684,7 @@ export function flushPassiveEffects(wasDelayedCommit?: boolean): boolean { // in the first place because we used to wrap it with // `Scheduler.runWithPriority`, which accepts a function. But now we track the // priority within React itself, so we can mutate the variable directly. - if (rootWithPendingPassiveEffects !== null) { + if (pendingEffectsStatus !== NO_PENDING_EFFECTS) { // Cache the root since rootWithPendingPassiveEffects is cleared in // flushPassiveEffectsImpl const root = rootWithPendingPassiveEffects; @@ -3715,10 +3717,6 @@ export function flushPassiveEffects(wasDelayedCommit?: boolean): boolean { } function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) { - if (rootWithPendingPassiveEffects === null) { - return false; - } - // Cache and clear the transitions flag const transitions = pendingPassiveTransitions; pendingPassiveTransitions = null; @@ -3726,7 +3724,7 @@ function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) { const root = rootWithPendingPassiveEffects; const lanes = pendingPassiveEffectsLanes; pendingEffectsStatus = NO_PENDING_EFFECTS; - rootWithPendingPassiveEffects = null; + rootWithPendingPassiveEffects = (null: any); // Clear for GC purposes. // TODO: This is sometimes out of sync with rootWithPendingPassiveEffects. // Figure out why and fix it. It's not causing any known issues (probably // because it's only used for profiling), but it's a refactor hazard. From d7fa2c1decdf922ee91d33baf429f688c8398702 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 5 Jan 2025 21:40:49 -0500 Subject: [PATCH 04/12] Drop the Passive since it's now includes other effects too --- .../src/ReactFiberWorkLoop.js | 54 +++++++++---------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index e0807abc3c87a..ad40f0e4cb15e 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -626,11 +626,11 @@ const PENDING_MUTATION_PHASE = 1; const PENDING_LAYOUT_PHASE = 2; const PENDING_PASSIVE_PHASE = 3; let pendingEffectsStatus: 0 | 1 | 2 | 3 = 0; -let rootWithPendingPassiveEffects: FiberRoot = (null: any); +let pendingEffectsRoot: FiberRoot = (null: any); let pendingFinishedWork: Fiber = (null: any); -let pendingPassiveEffectsLanes: Lanes = NoLanes; -let pendingPassiveEffectsRemainingLanes: Lanes = NoLanes; -let pendingPassiveEffectsRenderEndTime: number = -0; // Profiling-only +let pendingEffectsLanes: Lanes = NoLanes; +let pendingEffectsRemainingLanes: Lanes = NoLanes; +let pendingEffectsRenderEndTime: number = -0; // Profiling-only let pendingPassiveTransitions: Array | null = null; let pendingRecoverableErrors: null | Array> = null; let pendingDidIncludeRenderPhaseUpdate: boolean = false; @@ -659,12 +659,12 @@ export function getWorkInProgressRootRenderLanes(): Lanes { export function getRootWithPendingPassiveEffects(): FiberRoot | null { return pendingEffectsStatus === PENDING_PASSIVE_PHASE - ? rootWithPendingPassiveEffects + ? pendingEffectsRoot : null; } export function getPendingPassiveEffectsLanes(): Lanes { - return pendingPassiveEffectsLanes; + return pendingEffectsLanes; } export function isWorkLoopSuspendedOnData(): boolean { @@ -1639,7 +1639,7 @@ export function flushSyncFromReconciler(fn: (() => R) | void): R | void { if ( pendingEffectsStatus !== NO_PENDING_EFFECTS && !disableLegacyMode && - rootWithPendingPassiveEffects.tag === LegacyRoot && + pendingEffectsRoot.tag === LegacyRoot && (executionContext & (RenderContext | CommitContext)) === NoContext ) { flushPassiveEffects(); @@ -3260,13 +3260,13 @@ function commitRoot( // the previous render and commit if we throttle the commit // with setTimeout pendingFinishedWork = finishedWork; - pendingPassiveEffectsLanes = lanes; - pendingPassiveEffectsRemainingLanes = remainingLanes; + pendingEffectsLanes = lanes; + pendingEffectsRemainingLanes = remainingLanes; pendingPassiveTransitions = transitions; pendingRecoverableErrors = recoverableErrors; pendingDidIncludeRenderPhaseUpdate = didIncludeRenderPhaseUpdate; if (enableProfilerTimer) { - pendingPassiveEffectsRenderEndTime = completedRenderEndTime; + pendingEffectsRenderEndTime = completedRenderEndTime; pendingSuspendedCommitReason = suspendedCommitReason; } @@ -3367,7 +3367,7 @@ function commitRoot( function flushMutationEffects(root: FiberRoot): void { const finishedWork = pendingFinishedWork; - const lanes = pendingPassiveEffectsLanes; + const lanes = pendingEffectsLanes; const subtreeMutationHasEffects = (finishedWork.subtreeFlags & MutationMask) !== NoFlags; const rootMutationHasEffect = (finishedWork.flags & MutationMask) !== NoFlags; @@ -3406,8 +3406,8 @@ function flushMutationEffects(root: FiberRoot): void { function flushLayoutEffects(root: FiberRoot): void { const finishedWork = pendingFinishedWork; - const lanes = pendingPassiveEffectsLanes; - const completedRenderEndTime = pendingPassiveEffectsRenderEndTime; + const lanes = pendingEffectsLanes; + const completedRenderEndTime = pendingEffectsRenderEndTime; const recoverableErrors = pendingRecoverableErrors; const didIncludeRenderPhaseUpdate = pendingDidIncludeRenderPhaseUpdate; const suspendedCommitReason = pendingSuspendedCommitReason; @@ -3468,8 +3468,8 @@ function flushLayoutEffects(root: FiberRoot): void { // This commit has passive effects. Stash a reference to them. But don't // schedule a callback until after flushing layout work. pendingEffectsStatus = PENDING_PASSIVE_PHASE; - rootWithPendingPassiveEffects = root; - pendingPassiveEffectsLanes = lanes; + pendingEffectsRoot = root; + pendingEffectsLanes = lanes; } else { // There were no passive effects, so we can immediately release the cache // pool for this render. @@ -3557,7 +3557,7 @@ function flushLayoutEffects(root: FiberRoot): void { // currently schedule the callback in multiple places, will wait until those // are consolidated. if ( - includesSyncLane(pendingPassiveEffectsLanes) && + includesSyncLane(pendingEffectsLanes) && (disableLegacyMode || root.tag !== LegacyRoot) ) { flushPassiveEffects(); @@ -3685,16 +3685,16 @@ export function flushPassiveEffects(wasDelayedCommit?: boolean): boolean { // `Scheduler.runWithPriority`, which accepts a function. But now we track the // priority within React itself, so we can mutate the variable directly. if (pendingEffectsStatus !== NO_PENDING_EFFECTS) { - // Cache the root since rootWithPendingPassiveEffects is cleared in + // Cache the root since pendingEffectsRoot is cleared in // flushPassiveEffectsImpl - const root = rootWithPendingPassiveEffects; + const root = pendingEffectsRoot; // Cache and clear the remaining lanes flag; it must be reset since this // method can be called from various places, not always from commitRoot // where the remaining lanes are known - const remainingLanes = pendingPassiveEffectsRemainingLanes; - pendingPassiveEffectsRemainingLanes = NoLanes; + const remainingLanes = pendingEffectsRemainingLanes; + pendingEffectsRemainingLanes = NoLanes; - const renderPriority = lanesToEventPriority(pendingPassiveEffectsLanes); + const renderPriority = lanesToEventPriority(pendingEffectsLanes); const priority = lowerEventPriority(DefaultEventPriority, renderPriority); const prevTransition = ReactSharedInternals.T; const previousPriority = getCurrentUpdatePriority(); @@ -3721,14 +3721,14 @@ function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) { const transitions = pendingPassiveTransitions; pendingPassiveTransitions = null; - const root = rootWithPendingPassiveEffects; - const lanes = pendingPassiveEffectsLanes; + const root = pendingEffectsRoot; + const lanes = pendingEffectsLanes; pendingEffectsStatus = NO_PENDING_EFFECTS; - rootWithPendingPassiveEffects = (null: any); // Clear for GC purposes. - // TODO: This is sometimes out of sync with rootWithPendingPassiveEffects. + pendingEffectsRoot = (null: any); // Clear for GC purposes. + // TODO: This is sometimes out of sync with pendingEffectsRoot. // Figure out why and fix it. It's not causing any known issues (probably // because it's only used for profiling), but it's a refactor hazard. - pendingPassiveEffectsLanes = NoLanes; + pendingEffectsLanes = NoLanes; if (enableYieldingBeforePassive) { // We've finished our work for this render pass. @@ -3775,7 +3775,7 @@ function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) { root.current, lanes, transitions, - pendingPassiveEffectsRenderEndTime, + pendingEffectsRenderEndTime, ); if (enableSchedulingProfiler) { From 65337a2220137c31b1159c5556978e32b1d5dca2 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 5 Jan 2025 21:44:47 -0500 Subject: [PATCH 05/12] Pass root through the module scope as well --- .../react-reconciler/src/ReactFiberWorkLoop.js | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index ad40f0e4cb15e..5fae296b96b80 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -3260,6 +3260,7 @@ function commitRoot( // the previous render and commit if we throttle the commit // with setTimeout pendingFinishedWork = finishedWork; + pendingEffectsRoot = root; pendingEffectsLanes = lanes; pendingEffectsRemainingLanes = remainingLanes; pendingPassiveTransitions = transitions; @@ -3360,12 +3361,13 @@ function commitRoot( } } pendingEffectsStatus = PENDING_MUTATION_PHASE; - flushMutationEffects(root); + flushMutationEffects(); pendingEffectsStatus = PENDING_LAYOUT_PHASE; - flushLayoutEffects(root); + flushLayoutEffects(); } -function flushMutationEffects(root: FiberRoot): void { +function flushMutationEffects(): void { + const root = pendingEffectsRoot; const finishedWork = pendingFinishedWork; const lanes = pendingEffectsLanes; const subtreeMutationHasEffects = @@ -3404,7 +3406,8 @@ function flushMutationEffects(root: FiberRoot): void { root.current = finishedWork; } -function flushLayoutEffects(root: FiberRoot): void { +function flushLayoutEffects(): void { + const root = pendingEffectsRoot; const finishedWork = pendingFinishedWork; const lanes = pendingEffectsLanes; const completedRenderEndTime = pendingEffectsRenderEndTime; @@ -3468,12 +3471,11 @@ function flushLayoutEffects(root: FiberRoot): void { // This commit has passive effects. Stash a reference to them. But don't // schedule a callback until after flushing layout work. pendingEffectsStatus = PENDING_PASSIVE_PHASE; - pendingEffectsRoot = root; - pendingEffectsLanes = lanes; } else { // There were no passive effects, so we can immediately release the cache // pool for this render. pendingEffectsStatus = NO_PENDING_EFFECTS; + pendingEffectsRoot = (null: any); // Clear for GC purposes. releaseRootPooledCache(root, root.pendingLanes); if (__DEV__) { nestedPassiveUpdateCount = 0; From eb6873443e790627a35e3ff9fef018793dbd6ccf Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 5 Jan 2025 22:01:23 -0500 Subject: [PATCH 06/12] Rename flushPassiveEffects to flushPendingEffects This wrapper flushes any remaining effects. --- .../src/ReactFiberHotReloading.js | 4 +- .../src/ReactFiberReconciler.js | 6 +- .../src/ReactFiberRootScheduler.js | 6 +- .../src/ReactFiberWorkLoop.js | 80 +++++++++++-------- 4 files changed, 53 insertions(+), 43 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHotReloading.js b/packages/react-reconciler/src/ReactFiberHotReloading.js index c42bb0b2c6d12..e1d33f07dca27 100644 --- a/packages/react-reconciler/src/ReactFiberHotReloading.js +++ b/packages/react-reconciler/src/ReactFiberHotReloading.js @@ -16,7 +16,7 @@ import type {ReactNodeList} from 'shared/ReactTypes'; import { flushSyncWork, scheduleUpdateOnFiber, - flushPassiveEffects, + flushPendingEffects, } from './ReactFiberWorkLoop'; import {enqueueConcurrentRenderForLane} from './ReactFiberConcurrentUpdates'; import {updateContainerSync} from './ReactFiberReconciler'; @@ -229,7 +229,7 @@ export const scheduleRefresh: ScheduleRefresh = ( return; } const {staleFamilies, updatedFamilies} = update; - flushPassiveEffects(); + flushPendingEffects(); scheduleFibersWithFamiliesRecursively( root.current, updatedFamilies, diff --git a/packages/react-reconciler/src/ReactFiberReconciler.js b/packages/react-reconciler/src/ReactFiberReconciler.js index 70640d68475af..4c319a14569a1 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.js @@ -76,7 +76,7 @@ import { isAlreadyRendering, deferredUpdates, discreteUpdates, - flushPassiveEffects, + flushPendingEffects, } from './ReactFiberWorkLoop'; import {enqueueConcurrentRenderForLane} from './ReactFiberConcurrentUpdates'; import { @@ -366,7 +366,7 @@ export function updateContainerSync( callback: ?Function, ): Lane { if (!disableLegacyMode && container.tag === LegacyRoot) { - flushPassiveEffects(); + flushPendingEffects(); } const current = container.current; updateContainerImpl( @@ -454,7 +454,7 @@ export { flushSyncFromReconciler, flushSyncWork, isAlreadyRendering, - flushPassiveEffects, + flushPendingEffects as flushPassiveEffects, }; export function getPublicRootInstance( diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index d57458fdbf41c..9e658b34f3697 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -38,7 +38,7 @@ import { CommitContext, NoContext, RenderContext, - flushPassiveEffects, + flushPendingEffects, getExecutionContext, getWorkInProgressRoot, getWorkInProgressRootRenderLanes, @@ -469,7 +469,7 @@ function performWorkOnRootViaSchedulerTask( // Flush any pending passive effects before deciding which lanes to work on, // in case they schedule additional work. const originalCallbackNode = root.callbackNode; - const didFlushPassiveEffects = flushPassiveEffects(); + const didFlushPassiveEffects = flushPendingEffects(); if (didFlushPassiveEffects) { // Something in the passive effect phase may have canceled the current task. // Check if the task node for this root was changed. @@ -534,7 +534,7 @@ function performWorkOnRootViaSchedulerTask( function performSyncWorkOnRoot(root: FiberRoot, lanes: Lanes) { // This is the entry point for synchronous tasks that don't go // through Scheduler. - const didFlushPassiveEffects = flushPassiveEffects(); + const didFlushPassiveEffects = flushPendingEffects(); if (didFlushPassiveEffects) { // If passive effects were flushed, exit to the outer work loop in the root // scheduler, so we can recompute the priority. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 5fae296b96b80..746914fc5f999 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -1642,7 +1642,7 @@ export function flushSyncFromReconciler(fn: (() => R) | void): R | void { pendingEffectsRoot.tag === LegacyRoot && (executionContext & (RenderContext | CommitContext)) === NoContext ) { - flushPassiveEffects(); + flushPendingEffects(); } const prevExecutionContext = executionContext; @@ -3159,7 +3159,7 @@ function commitRoot( // no more pending effects. // TODO: Might be better if `flushPassiveEffects` did not automatically // flush synchronous work at the end, to avoid factoring hazards like this. - flushPassiveEffects(); + flushPendingEffects(); } while (pendingEffectsStatus !== NO_PENDING_EFFECTS); flushRenderPhaseStrictModeWarningsInDEV(); @@ -3298,7 +3298,7 @@ function commitRoot( // event when logging events. trackSchedulerEvent(); } - flushPassiveEffects(true); + flushPendingEffects(true); // This render triggered passive effects: release the root cache pool // *after* passive effects fire to avoid freeing a cache pool that may // be referenced by a node in the tree (HostRoot, Cache boundary etc) @@ -3362,7 +3362,6 @@ function commitRoot( } pendingEffectsStatus = PENDING_MUTATION_PHASE; flushMutationEffects(); - pendingEffectsStatus = PENDING_LAYOUT_PHASE; flushLayoutEffects(); } @@ -3404,6 +3403,7 @@ function flushMutationEffects(): void { // componentWillUnmount, but before the layout phase, so that the finished // work is current during componentDidMount/Update. root.current = finishedWork; + pendingEffectsStatus = PENDING_LAYOUT_PHASE; } function flushLayoutEffects(): void { @@ -3562,7 +3562,7 @@ function flushLayoutEffects(): void { includesSyncLane(pendingEffectsLanes) && (disableLegacyMode || root.tag !== LegacyRoot) ) { - flushPassiveEffects(); + flushPendingEffects(); } // Always call this before exiting `commitRoot`, to ensure that any @@ -3679,43 +3679,53 @@ function releaseRootPooledCache(root: FiberRoot, remainingLanes: Lanes) { } } -export function flushPassiveEffects(wasDelayedCommit?: boolean): boolean { +export function flushPendingEffects(wasDelayedCommit?: boolean): boolean { // Returns whether passive effects were flushed. - // TODO: Combine this check with the one in flushPassiveEFfectsImpl. We should - // probably just combine the two functions. I believe they were only separate + if (pendingEffectsStatus === PENDING_MUTATION_PHASE) { + flushMutationEffects(); + } + if (pendingEffectsStatus === PENDING_LAYOUT_PHASE) { + flushLayoutEffects(); + } + if (pendingEffectsStatus === PENDING_PASSIVE_PHASE) { + return flushPassiveEffects(wasDelayedCommit); + } else { + return false; + } +} + +function flushPassiveEffects(wasDelayedCommit?: boolean): boolean { + // TODO: Merge flushPassiveEffectsImpl into this function. I believe they were only separate // in the first place because we used to wrap it with // `Scheduler.runWithPriority`, which accepts a function. But now we track the // priority within React itself, so we can mutate the variable directly. - if (pendingEffectsStatus !== NO_PENDING_EFFECTS) { - // Cache the root since pendingEffectsRoot is cleared in - // flushPassiveEffectsImpl - const root = pendingEffectsRoot; - // Cache and clear the remaining lanes flag; it must be reset since this - // method can be called from various places, not always from commitRoot - // where the remaining lanes are known - const remainingLanes = pendingEffectsRemainingLanes; - pendingEffectsRemainingLanes = NoLanes; - - const renderPriority = lanesToEventPriority(pendingEffectsLanes); - const priority = lowerEventPriority(DefaultEventPriority, renderPriority); - const prevTransition = ReactSharedInternals.T; - const previousPriority = getCurrentUpdatePriority(); + // Cache the root since pendingEffectsRoot is cleared in + // flushPassiveEffectsImpl + const root = pendingEffectsRoot; + // Cache and clear the remaining lanes flag; it must be reset since this + // method can be called from various places, not always from commitRoot + // where the remaining lanes are known + const remainingLanes = pendingEffectsRemainingLanes; + pendingEffectsRemainingLanes = NoLanes; + + const renderPriority = lanesToEventPriority(pendingEffectsLanes); + const priority = lowerEventPriority(DefaultEventPriority, renderPriority); + const prevTransition = ReactSharedInternals.T; + const previousPriority = getCurrentUpdatePriority(); - try { - setCurrentUpdatePriority(priority); - ReactSharedInternals.T = null; - return flushPassiveEffectsImpl(wasDelayedCommit); - } finally { - setCurrentUpdatePriority(previousPriority); - ReactSharedInternals.T = prevTransition; + try { + setCurrentUpdatePriority(priority); + ReactSharedInternals.T = null; + return flushPassiveEffectsImpl(wasDelayedCommit); + } finally { + setCurrentUpdatePriority(previousPriority); + ReactSharedInternals.T = prevTransition; - // Once passive effects have run for the tree - giving components a - // chance to retain cache instances they use - release the pooled - // cache at the root (if there is one) - releaseRootPooledCache(root, remainingLanes); - } + // Once passive effects have run for the tree - giving components a + // chance to retain cache instances they use - release the pooled + // cache at the root (if there is one) + releaseRootPooledCache(root, remainingLanes); } - return false; } function flushPassiveEffectsImpl(wasDelayedCommit: void | boolean) { From c24399adb05e16554a79cb12b1ed97f3e90f1ecf Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 5 Jan 2025 22:10:26 -0500 Subject: [PATCH 07/12] Fix wasDelayedCommit in enableYieldingBeforePassive flag When we flush from a task, we've yielded to paint. This only matters for performance.measure. --- packages/react-reconciler/src/ReactFiberRootScheduler.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 9e658b34f3697..92daa1110eabc 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -469,7 +469,7 @@ function performWorkOnRootViaSchedulerTask( // Flush any pending passive effects before deciding which lanes to work on, // in case they schedule additional work. const originalCallbackNode = root.callbackNode; - const didFlushPassiveEffects = flushPendingEffects(); + const didFlushPassiveEffects = flushPendingEffects(true); if (didFlushPassiveEffects) { // Something in the passive effect phase may have canceled the current task. // Check if the task node for this root was changed. From e8e72296f982e8287541407fda26cee22da4c3c3 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 5 Jan 2025 23:13:24 -0500 Subject: [PATCH 08/12] Defer work scheduler work if we're inside a pending commit --- .../src/ReactFiberRootScheduler.js | 14 ++++++++++++++ .../react-reconciler/src/ReactFiberWorkLoop.js | 7 +++++++ 2 files changed, 21 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 92daa1110eabc..4c4a35ac92c28 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -44,6 +44,7 @@ import { getWorkInProgressRootRenderLanes, getRootWithPendingPassiveEffects, getPendingPassiveEffectsLanes, + hasPendingCommitEffects, isWorkLoopSuspendedOnData, performWorkOnRoot, } from './ReactFiberWorkLoop'; @@ -466,6 +467,19 @@ function performWorkOnRootViaSchedulerTask( trackSchedulerEvent(); } + if (hasPendingCommitEffects()) { + // We are currently in the middle of an async committing (such as a View Transition). + // We could force these to flush eagerly but it's better to defer any work until + // it finishes. This may not be the same root as we're waiting on. + // TODO: This relies on the commit eventually calling ensureRootIsScheduled which + // always calls processRootScheduleInMicrotask which in turn always loops through + // all the roots to figure out. This is all a bit inefficient and if optimized + // it'll need to consider rescheduling a task for any skipped roots. + root.callbackNode = null; + root.callbackPriority = NoLane; + return null; + } + // Flush any pending passive effects before deciding which lanes to work on, // in case they schedule additional work. const originalCallbackNode = root.callbackNode; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 746914fc5f999..55b8962a46a3e 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -657,6 +657,13 @@ export function getWorkInProgressRootRenderLanes(): Lanes { return workInProgressRootRenderLanes; } +export function hasPendingCommitEffects(): boolean { + return ( + pendingEffectsStatus !== NO_PENDING_EFFECTS && + pendingEffectsStatus !== PENDING_PASSIVE_PHASE + ); +} + export function getRootWithPendingPassiveEffects(): FiberRoot | null { return pendingEffectsStatus === PENDING_PASSIVE_PHASE ? pendingEffectsRoot From d6e7180a3324ceb7791369abfb96939d0a86db4f Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 5 Jan 2025 23:51:20 -0500 Subject: [PATCH 09/12] Check status inside each flush so it's safe to call them individually --- .../src/ReactFiberWorkLoop.js | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 55b8962a46a3e..7b7b11e03700d 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -3373,6 +3373,9 @@ function commitRoot( } function flushMutationEffects(): void { + if (pendingEffectsStatus !== PENDING_MUTATION_PHASE) { + return; + } const root = pendingEffectsRoot; const finishedWork = pendingFinishedWork; const lanes = pendingEffectsLanes; @@ -3414,6 +3417,9 @@ function flushMutationEffects(): void { } function flushLayoutEffects(): void { + if (pendingEffectsStatus !== PENDING_LAYOUT_PHASE) { + return; + } const root = pendingEffectsRoot; const finishedWork = pendingFinishedWork; const lanes = pendingEffectsLanes; @@ -3688,20 +3694,15 @@ function releaseRootPooledCache(root: FiberRoot, remainingLanes: Lanes) { export function flushPendingEffects(wasDelayedCommit?: boolean): boolean { // Returns whether passive effects were flushed. - if (pendingEffectsStatus === PENDING_MUTATION_PHASE) { - flushMutationEffects(); - } - if (pendingEffectsStatus === PENDING_LAYOUT_PHASE) { - flushLayoutEffects(); - } - if (pendingEffectsStatus === PENDING_PASSIVE_PHASE) { - return flushPassiveEffects(wasDelayedCommit); - } else { - return false; - } + flushMutationEffects(); + flushLayoutEffects(); + return flushPassiveEffects(wasDelayedCommit); } function flushPassiveEffects(wasDelayedCommit?: boolean): boolean { + if (pendingEffectsStatus !== PENDING_PASSIVE_PHASE) { + return false; + } // TODO: Merge flushPassiveEffectsImpl into this function. I believe they were only separate // in the first place because we used to wrap it with // `Scheduler.runWithPriority`, which accepts a function. But now we track the From 182d1aa4eed90b8d051e9fcb52402d5c2d26adb0 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 6 Jan 2025 00:14:06 -0500 Subject: [PATCH 10/12] Flush only passive in the callback --- packages/react-reconciler/src/ReactFiberWorkLoop.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 7b7b11e03700d..682df5c193d38 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -3305,7 +3305,7 @@ function commitRoot( // event when logging events. trackSchedulerEvent(); } - flushPendingEffects(true); + flushPassiveEffects(true); // This render triggered passive effects: release the root cache pool // *after* passive effects fire to avoid freeing a cache pool that may // be referenced by a node in the tree (HostRoot, Cache boundary etc) From c89b6b39a66d68cbfe26a0fcd88d31f1fd0230ec Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 6 Jan 2025 11:15:18 -0500 Subject: [PATCH 11/12] Remove stale comment --- packages/react-reconciler/src/ReactFiberWorkLoop.js | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 682df5c193d38..f8654522eb359 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -3481,14 +3481,12 @@ function flushLayoutEffects(): void { (finishedWork.flags & PassiveMask) !== NoFlags; if (rootDidHavePassiveEffects) { - // This commit has passive effects. Stash a reference to them. But don't - // schedule a callback until after flushing layout work. pendingEffectsStatus = PENDING_PASSIVE_PHASE; } else { - // There were no passive effects, so we can immediately release the cache - // pool for this render. pendingEffectsStatus = NO_PENDING_EFFECTS; pendingEffectsRoot = (null: any); // Clear for GC purposes. + // There were no passive effects, so we can immediately release the cache + // pool for this render. releaseRootPooledCache(root, root.pendingLanes); if (__DEV__) { nestedPassiveUpdateCount = 0; From 424f0dd502f2d47225a76d61a43c4b76662786dc Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 6 Jan 2025 11:23:59 -0500 Subject: [PATCH 12/12] Clear effects in case we throw from React That way we don't get stuck. --- packages/react-reconciler/src/ReactFiberWorkLoop.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index f8654522eb359..bc45c06fbd871 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -3376,6 +3376,8 @@ function flushMutationEffects(): void { if (pendingEffectsStatus !== PENDING_MUTATION_PHASE) { return; } + pendingEffectsStatus = NO_PENDING_EFFECTS; + const root = pendingEffectsRoot; const finishedWork = pendingFinishedWork; const lanes = pendingEffectsLanes; @@ -3420,6 +3422,8 @@ function flushLayoutEffects(): void { if (pendingEffectsStatus !== PENDING_LAYOUT_PHASE) { return; } + pendingEffectsStatus = NO_PENDING_EFFECTS; + const root = pendingEffectsRoot; const finishedWork = pendingFinishedWork; const lanes = pendingEffectsLanes;