From ee229183f427356476c4481647816ddc5b4e0475 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 18 Nov 2020 01:28:13 -0600 Subject: [PATCH] Remove passive logic from layout phase Original PR: #19809 --- .../src/ReactFiberCommitWork.new.js | 28 +------ .../src/ReactFiberHooks.new.js | 11 +-- .../src/ReactFiberWorkLoop.new.js | 83 +++++++++---------- 3 files changed, 42 insertions(+), 80 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 6545ad876f879..de5a05d85e17b 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -127,8 +127,6 @@ import { captureCommitPhaseError, resolveRetryWakeable, markCommitTimeOfFallback, - enqueuePendingPassiveHookEffectMount, - enqueuePendingPassiveHookEffectUnmount, enqueuePendingPassiveProfilerEffect, } from './ReactFiberWorkLoop.new'; import { @@ -414,26 +412,6 @@ function commitHookEffectListMount(tag: number, finishedWork: Fiber) { } } -function schedulePassiveEffects(finishedWork: Fiber) { - const updateQueue: FunctionComponentUpdateQueue | null = (finishedWork.updateQueue: any); - const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null; - if (lastEffect !== null) { - const firstEffect = lastEffect.next; - let effect = firstEffect; - do { - const {next, tag} = effect; - if ( - (tag & HookPassive) !== NoHookEffect && - (tag & HookHasEffect) !== NoHookEffect - ) { - enqueuePendingPassiveHookEffectUnmount(finishedWork, effect); - enqueuePendingPassiveHookEffectMount(finishedWork, effect); - } - effect = next; - } while (effect !== firstEffect); - } -} - export function commitPassiveEffectDurations( finishedRoot: FiberRoot, finishedWork: Fiber, @@ -519,8 +497,6 @@ function commitLifeCycles( } else { commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork); } - - schedulePassiveEffects(finishedWork); return; } case ClassComponent: { @@ -957,9 +933,7 @@ function commitUnmount( do { const {destroy, tag} = effect; if (destroy !== undefined) { - if ((tag & HookPassive) !== NoHookEffect) { - enqueuePendingPassiveHookEffectUnmount(current, effect); - } else { + if ((tag & HookLayout) !== NoHookEffect) { if ( enableProfilerTimer && enableProfilerCommitHooks && diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 66c85b274df0e..3c883ff382c3c 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -1305,7 +1305,7 @@ function mountEffect( } } return mountEffectImpl( - UpdateEffect | PassiveEffect | PassiveStaticEffect, + PassiveEffect | PassiveStaticEffect, HookPassive, create, deps, @@ -1322,12 +1322,7 @@ function updateEffect( warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber); } } - return updateEffectImpl( - UpdateEffect | PassiveEffect, - HookPassive, - create, - deps, - ); + return updateEffectImpl(PassiveEffect, HookPassive, create, deps); } function mountLayoutEffect( @@ -1679,7 +1674,7 @@ function mountOpaqueIdentifier(): OpaqueIDType | void { const setId = mountState(id)[1]; if ((currentlyRenderingFiber.mode & BlockingMode) === NoMode) { - currentlyRenderingFiber.flags |= UpdateEffect | PassiveEffect; + currentlyRenderingFiber.flags |= PassiveEffect; pushEffect( HookHasEffect | HookPassive, () => { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index abae51b0b3bb0..99bdf41092d10 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -13,7 +13,6 @@ import type {Lanes, Lane} from './ReactFiberLane.new'; import type {ReactPriorityLevel} from './ReactInternalTypes'; import type {Interaction} from 'scheduler/src/Tracing'; import type {SuspenseState} from './ReactFiberSuspenseComponent.new'; -import type {Effect as HookEffect} from './ReactFiberHooks.new'; import type {StackCursor} from './ReactFiberStack.new'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new'; @@ -121,8 +120,16 @@ import { import {LegacyRoot} from './ReactRootTags'; import { NoFlags, - PerformedWork, Placement, + PassiveStatic, + Incomplete, + HostEffectMask, + Hydrating, + BeforeMutationMask, + MutationMask, + LayoutMask, + PassiveMask, + PerformedWork, Update, PlacementAndUpdate, Deletion, @@ -131,11 +138,6 @@ import { ContentReset, Snapshot, Callback, - Passive, - PassiveStatic, - Incomplete, - HostEffectMask, - Hydrating, HydratingAndUpdate, StaticMask, } from './ReactFiberFlags'; @@ -1956,7 +1958,35 @@ function commitRootImpl(root, renderPriorityLevel) { firstEffect = finishedWork.firstEffect; } - if (firstEffect !== null) { + // 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.) + if ( + (finishedWork.subtreeFlags & PassiveMask) !== NoFlags || + (finishedWork.flags & PassiveMask) !== NoFlags + ) { + rootDoesHavePassiveEffects = true; + 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 + // only other reason this optimization exists is because it affects profiling. + // Reconsider whether this is necessary. + const subtreeHasEffects = + (finishedWork.subtreeFlags & + (BeforeMutationMask | MutationMask | LayoutMask | PassiveMask)) !== + NoFlags; + const rootHasEffect = + (finishedWork.flags & + (BeforeMutationMask | MutationMask | LayoutMask | PassiveMask)) !== + NoFlags; + + if (subtreeHasEffects || rootHasEffect) { let previousLanePriority; if (decoupleUpdatePriorityFromScheduler) { previousLanePriority = getCurrentUpdateLanePriority(); @@ -2275,17 +2305,6 @@ function commitBeforeMutationEffects() { resetCurrentDebugFiberInDEV(); } - if ((flags & Passive) !== NoFlags) { - // If there are passive effects, schedule a callback to flush at - // the earliest opportunity. - if (!rootDoesHavePassiveEffects) { - rootDoesHavePassiveEffects = true; - scheduleCallback(NormalSchedulerPriority, () => { - flushPassiveEffects(); - return null; - }); - } - } nextEffect = nextEffect.nextEffect; } } @@ -2485,32 +2504,6 @@ export function enqueuePendingPassiveProfilerEffect(fiber: Fiber): void { } } -export function enqueuePendingPassiveHookEffectMount( - fiber: Fiber, - effect: HookEffect, -): void { - if (!rootDoesHavePassiveEffects) { - rootDoesHavePassiveEffects = true; - scheduleCallback(NormalSchedulerPriority, () => { - flushPassiveEffects(); - return null; - }); - } -} - -export function enqueuePendingPassiveHookEffectUnmount( - fiber: Fiber, - effect: HookEffect, -): void { - if (!rootDoesHavePassiveEffects) { - rootDoesHavePassiveEffects = true; - scheduleCallback(NormalSchedulerPriority, () => { - flushPassiveEffects(); - return null; - }); - } -} - function flushPassiveEffectsImpl() { if (rootWithPendingPassiveEffects === null) { return false;