From c4cdb73a737f303feae14be5dbd415cd5a45c54f Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 16 Oct 2020 10:35:06 -0500 Subject: [PATCH] Remove last schedulePassiveEffectCallback call Now there's only a single place where the passive effect callback is scheduled. --- .../src/ReactFiberCommitWork.new.js | 6 -- .../src/ReactFiberWorkLoop.new.js | 56 ++++++------------- 2 files changed, 18 insertions(+), 44 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index c43c6f8b1363fb..687e62de976c71 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -70,7 +70,6 @@ import { Update, Callback, LayoutMask, - PassiveMask, Ref, } from './ReactFiberFlags'; import getComponentName from 'shared/getComponentName'; @@ -125,7 +124,6 @@ import { captureCommitPhaseError, resolveRetryWakeable, markCommitTimeOfFallback, - schedulePassiveEffectCallback, } from './ReactFiberWorkLoop.new'; import { NoFlags as NoHookEffect, @@ -599,10 +597,6 @@ function recursivelyCommitLayoutEffects( finishedWork, ); } - - if ((finishedWork.subtreeFlags & PassiveMask) !== NoFlags) { - schedulePassiveEffectCallback(); - } break; } case ClassComponent: { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index ff171af0c53565..53a6b0334d8ebb 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -341,7 +341,6 @@ let hasUncaughtError = false; let firstUncaughtError = null; let legacyErrorBoundariesThatAlreadyFailed: Set | null = null; -let rootDoesHavePassiveEffects: boolean = false; let rootWithPendingPassiveEffects: FiberRoot | null = null; let pendingPassiveEffectsRenderPriority: ReactPriorityLevel = NoSchedulerPriority; let pendingPassiveEffectsLanes: Lanes = NoLanes; @@ -1865,6 +1864,22 @@ function commitRootImpl(root, renderPriorityLevel) { // times out. } + // If there are pending passive effects, schedule a callback to process them. + // Do this as early as possible, before anything else in the commit phase + // is scheduled. + const rootDoesHavePassiveEffects = + (finishedWork.subtreeFlags & PassiveMask) !== NoFlags || + (finishedWork.flags & PassiveMask) !== NoFlags; + if (rootDoesHavePassiveEffects) { + rootWithPendingPassiveEffects = root; + pendingPassiveEffectsLanes = lanes; + pendingPassiveEffectsRenderPriority = renderPriorityLevel; + scheduleCallback(NormalSchedulerPriority, () => { + flushPassiveEffects(); + return null; + }); + } + // Check if there are any effects in the whole tree. // TODO: This is left over from the effect list implementation, where we had // to check for the existence of `firstEffect` to satsify Flow. I think the @@ -1880,20 +1895,6 @@ function commitRootImpl(root, renderPriorityLevel) { NoFlags; if (subtreeHasEffects || rootHasEffect) { - // If there are pending passive effects, schedule a callback to process them. - if ( - (finishedWork.subtreeFlags & PassiveMask) !== NoFlags || - (finishedWork.flags & PassiveMask) !== NoFlags - ) { - if (!rootDoesHavePassiveEffects) { - rootDoesHavePassiveEffects = true; - scheduleCallback(NormalSchedulerPriority, () => { - flushPassiveEffects(); - return null; - }); - } - } - let previousLanePriority; if (decoupleUpdatePriorityFromScheduler) { previousLanePriority = getCurrentUpdateLanePriority(); @@ -2010,17 +2011,6 @@ function commitRootImpl(root, renderPriorityLevel) { } } - const rootDidHavePassiveEffects = rootDoesHavePassiveEffects; - - if (rootDoesHavePassiveEffects) { - // This commit has passive effects. Stash a reference to them. But don't - // schedule a callback until after flushing layout work. - rootDoesHavePassiveEffects = false; - rootWithPendingPassiveEffects = root; - pendingPassiveEffectsLanes = lanes; - pendingPassiveEffectsRenderPriority = renderPriorityLevel; - } - // Read this again, since an effect might have updated it remainingLanes = root.pendingLanes; @@ -2047,13 +2037,13 @@ function commitRootImpl(root, renderPriorityLevel) { } if (__DEV__ && enableDoubleInvokingEffects) { - if (!rootDidHavePassiveEffects) { + if (!rootDoesHavePassiveEffects) { commitDoubleInvokeEffectsInDEV(root.current, false); } } if (enableSchedulerTracing) { - if (!rootDidHavePassiveEffects) { + if (!rootDoesHavePassiveEffects) { // If there are no passive effects, then we can complete the pending interactions. // Otherwise, we'll wait until after the passive effects are flushed. // Wait to do this until after remaining work has been scheduled, @@ -2356,16 +2346,6 @@ function commitMutationEffectsDeletions( } } -export function schedulePassiveEffectCallback() { - if (!rootDoesHavePassiveEffects) { - rootDoesHavePassiveEffects = true; - scheduleCallback(NormalSchedulerPriority, () => { - flushPassiveEffects(); - return null; - }); - } -} - export function flushPassiveEffects(): boolean { // Returns whether passive effects were flushed. if (pendingPassiveEffectsRenderPriority !== NoSchedulerPriority) {