From a7cfb9bb29b0a7a237f47a61870ea0b0386d3a39 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 7 Dec 2020 18:01:47 -0600 Subject: [PATCH] Convert passive unmount phase to tree traversal --- .../react-reconciler/src/ReactFiber.new.js | 2 +- .../src/ReactFiberCommitWork.new.js | 158 +++++++++++++++++- .../src/ReactFiberWorkLoop.new.js | 111 +++--------- 3 files changed, 182 insertions(+), 89 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiber.new.js b/packages/react-reconciler/src/ReactFiber.new.js index 2581831470001..b617bb3eeeb13 100644 --- a/packages/react-reconciler/src/ReactFiber.new.js +++ b/packages/react-reconciler/src/ReactFiber.new.js @@ -363,7 +363,7 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) { // We assume pendingProps, index, key, ref, return are still untouched to // avoid doing another reconciliation. - // Reset the effect tag but keep any Placement tags, since that's something + // Reset the effect flags but keep any Placement tags, since that's something // that child fiber is setting, not the reconciliation. workInProgress.flags &= Placement; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index a3147ab213c45..6545ad876f879 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -24,6 +24,7 @@ import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new'; import type {Wakeable} from 'shared/ReactTypes'; import type {ReactPriorityLevel} from './ReactInternalTypes'; import type {OffscreenState} from './ReactFiberOffscreenComponent'; +import type {HookFlags} from './ReactHookEffectTags'; import {unstable_wrap as Schedule_tracing_wrap} from 'scheduler/tracing'; import { @@ -65,10 +66,13 @@ import { NoFlags, ContentReset, Placement, + ChildDeletion, Snapshot, Update, Passive, + PassiveStatic, PassiveMask, + PassiveUnmountPendingDev, } from './ReactFiberFlags'; import getComponentName from 'shared/getComponentName'; import invariant from 'shared/invariant'; @@ -340,19 +344,19 @@ function commitBeforeMutationLifeCycles( ); } -function commitHookEffectListUnmount(tag: number, finishedWork: Fiber) { +function commitHookEffectListUnmount(flags: HookFlags, 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 { - if ((effect.tag & tag) === tag) { + if ((effect.tag & flags) === flags) { // Unmount const destroy = effect.destroy; effect.destroy = undefined; if (destroy !== undefined) { - destroy(); + safelyCallDestroy(finishedWork, destroy); } } effect = effect.next; @@ -1914,6 +1918,154 @@ function commitPassiveMountOnFiber( } } +export function commitPassiveUnmountEffects(firstChild: Fiber): void { + nextEffect = firstChild; + commitPassiveUnmountEffects_begin(); +} + +function commitPassiveUnmountEffects_begin() { + while (nextEffect !== null) { + const fiber = nextEffect; + const child = fiber.child; + + if ((nextEffect.flags & ChildDeletion) !== NoFlags) { + const deletions = fiber.deletions; + if (deletions !== null) { + for (let i = 0; i < deletions.length; i++) { + const fiberToDelete = deletions[i]; + nextEffect = fiberToDelete; + commitPassiveUnmountEffectsInsideOfDeletedTree_begin(fiberToDelete); + + // Now that passive effects have been processed, it's safe to detach lingering pointers. + detachFiberAfterEffects(fiberToDelete); + } + nextEffect = fiber; + } + } + + if ((fiber.subtreeFlags & PassiveMask) !== NoFlags && child !== null) { + ensureCorrectReturnPointer(child, fiber); + nextEffect = child; + } else { + commitPassiveUnmountEffects_complete(); + } + } +} + +function commitPassiveUnmountEffects_complete() { + while (nextEffect !== null) { + const fiber = nextEffect; + if ((fiber.flags & Passive) !== NoFlags) { + setCurrentDebugFiberInDEV(fiber); + commitPassiveUnmountOnFiber(fiber); + resetCurrentDebugFiberInDEV(); + } + + const sibling = fiber.sibling; + if (sibling !== null) { + ensureCorrectReturnPointer(sibling, fiber.return); + nextEffect = sibling; + return; + } + + nextEffect = fiber.return; + } +} + +function commitPassiveUnmountOnFiber(finishedWork: Fiber): void { + if (__DEV__) { + finishedWork.flags &= ~PassiveUnmountPendingDev; + const alternate = finishedWork.alternate; + if (alternate !== null) { + alternate.flags &= ~PassiveUnmountPendingDev; + } + } + + switch (finishedWork.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + if ( + enableProfilerTimer && + enableProfilerCommitHooks && + finishedWork.mode & ProfileMode + ) { + startPassiveEffectTimer(); + commitHookEffectListUnmount(HookPassive | HookHasEffect, finishedWork); + recordPassiveEffectDuration(finishedWork); + } else { + commitHookEffectListUnmount(HookPassive | HookHasEffect, finishedWork); + } + break; + } + } +} + +function commitPassiveUnmountEffectsInsideOfDeletedTree_begin( + deletedSubtreeRoot: Fiber, +) { + while (nextEffect !== null) { + const fiber = nextEffect; + const child = fiber.child; + if ((fiber.subtreeFlags & PassiveStatic) !== NoFlags && child !== null) { + ensureCorrectReturnPointer(child, fiber); + nextEffect = child; + } else { + commitPassiveUnmountEffectsInsideOfDeletedTree_complete( + deletedSubtreeRoot, + ); + } + } +} + +function commitPassiveUnmountEffectsInsideOfDeletedTree_complete( + deletedSubtreeRoot: Fiber, +) { + while (nextEffect !== null) { + const fiber = nextEffect; + if ((fiber.flags & PassiveStatic) !== NoFlags) { + setCurrentDebugFiberInDEV(fiber); + commitPassiveUnmountInsideDeletedTreeOnFiber(fiber); + resetCurrentDebugFiberInDEV(); + } + + if (fiber === deletedSubtreeRoot) { + nextEffect = null; + return; + } + + const sibling = fiber.sibling; + if (sibling !== null) { + ensureCorrectReturnPointer(sibling, fiber.return); + nextEffect = sibling; + return; + } + + nextEffect = fiber.return; + } +} + +function commitPassiveUnmountInsideDeletedTreeOnFiber(current: Fiber): void { + switch (current.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + if ( + enableProfilerTimer && + enableProfilerCommitHooks && + current.mode & ProfileMode + ) { + startPassiveEffectTimer(); + commitHookEffectListUnmount(HookPassive, current); + recordPassiveEffectDuration(current); + } else { + commitHookEffectListUnmount(HookPassive, current); + } + break; + } + } +} + let didWarnWrongReturnPointer = false; function ensureCorrectReturnPointer(fiber, expectedReturnFiber) { if (__DEV__) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 68da637f24299..0b3dcbd5a7b30 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -15,6 +15,7 @@ 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'; import { warnAboutDeprecatedLifecycles, @@ -50,6 +51,10 @@ import { flushSyncCallbackQueue, scheduleSyncCallback, } from './SchedulerWithReactIntegration.new'; +import { + NoFlags as NoHookEffect, + Passive as HookPassive, +} from './ReactHookEffectTags'; import { logCommitStarted, logCommitStopped, @@ -127,7 +132,7 @@ import { Snapshot, Callback, Passive, - PassiveUnmountPendingDev, + PassiveStatic, Incomplete, HostEffectMask, Hydrating, @@ -194,6 +199,7 @@ import { commitResetTextContent, isSuspenseBoundaryBeingHidden, commitPassiveMountEffects, + commitPassiveUnmountEffects, detachFiberAfterEffects, } from './ReactFiberCommitWork.new'; import {enqueueUpdate} from './ReactUpdateQueue.new'; @@ -213,9 +219,7 @@ import { import { markNestedUpdateScheduled, recordCommitTime, - recordPassiveEffectDuration, resetNestedUpdateFlag, - startPassiveEffectTimer, startProfilerTimer, stopProfilerTimerIfRunningAndRecordDelta, syncNestedUpdateFlag, @@ -341,7 +345,6 @@ let rootDoesHavePassiveEffects: boolean = false; let rootWithPendingPassiveEffects: FiberRoot | null = null; let pendingPassiveEffectsRenderPriority: ReactPriorityLevel = NoSchedulerPriority; let pendingPassiveEffectsLanes: Lanes = NoLanes; -let pendingPassiveHookEffectsUnmount: Array = []; let pendingPassiveProfilerEffects: Array = []; let rootsWithPendingDiscreteUpdates: Set | null = null; @@ -2499,14 +2502,6 @@ export function enqueuePendingPassiveHookEffectUnmount( fiber: Fiber, effect: HookEffect, ): void { - pendingPassiveHookEffectsUnmount.push(effect, fiber); - if (__DEV__) { - fiber.flags |= PassiveUnmountPendingDev; - const alternate = fiber.alternate; - if (alternate !== null) { - alternate.flags |= PassiveUnmountPendingDev; - } - } if (!rootDoesHavePassiveEffects) { rootDoesHavePassiveEffects = true; scheduleCallback(NormalSchedulerPriority, () => { @@ -2549,74 +2544,7 @@ function flushPassiveEffectsImpl() { executionContext |= CommitContext; const prevInteractions = pushInteractions(root); - // It's important that ALL pending passive effect destroy functions are called - // before ANY passive effect create functions are called. - // Otherwise effects in sibling components might interfere with each other. - // e.g. a destroy function in one component may unintentionally override a ref - // value set by a create function in another component. - // Layout effects have the same constraint. - - // First pass: Destroy stale passive effects. - const unmountEffects = pendingPassiveHookEffectsUnmount; - pendingPassiveHookEffectsUnmount = []; - for (let i = 0; i < unmountEffects.length; i += 2) { - const effect = ((unmountEffects[i]: any): HookEffect); - const fiber = ((unmountEffects[i + 1]: any): Fiber); - const destroy = effect.destroy; - effect.destroy = undefined; - - if (__DEV__) { - fiber.flags &= ~PassiveUnmountPendingDev; - const alternate = fiber.alternate; - if (alternate !== null) { - alternate.flags &= ~PassiveUnmountPendingDev; - } - } - - if (typeof destroy === 'function') { - if (__DEV__) { - setCurrentDebugFiberInDEV(fiber); - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - fiber.mode & ProfileMode - ) { - startPassiveEffectTimer(); - invokeGuardedCallback(null, destroy, null); - recordPassiveEffectDuration(fiber); - } else { - invokeGuardedCallback(null, destroy, null); - } - if (hasCaughtError()) { - invariant(fiber !== null, 'Should be working on an effect.'); - const error = clearCaughtError(); - captureCommitPhaseError(fiber, error); - } - resetCurrentDebugFiberInDEV(); - } else { - try { - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - fiber.mode & ProfileMode - ) { - try { - startPassiveEffectTimer(); - destroy(); - } finally { - recordPassiveEffectDuration(fiber); - } - } else { - destroy(); - } - } catch (error) { - invariant(fiber !== null, 'Should be working on an effect.'); - captureCommitPhaseError(fiber, error); - } - } - } - } - // Second pass: Create new passive effects. + commitPassiveUnmountEffects(root.current); commitPassiveMountEffects(root, root.current); // TODO: Move to commitPassiveMountEffects @@ -3004,12 +2932,25 @@ function warnAboutUpdateOnUnmountedFiberInDEV(fiber) { return; } - // If there are pending passive effects unmounts for this Fiber, - // we can assume that they would have prevented this update. - if ((fiber.flags & PassiveUnmountPendingDev) !== NoFlags) { - return; - } + if ((fiber.flags & PassiveStatic) !== NoFlags) { + const updateQueue: FunctionComponentUpdateQueue | null = (fiber.updateQueue: any); + if (updateQueue !== null) { + const lastEffect = updateQueue.lastEffect; + if (lastEffect !== null) { + const firstEffect = lastEffect.next; + let effect = firstEffect; + do { + if (effect.destroy !== undefined) { + if ((effect.tag & HookPassive) !== NoHookEffect) { + return; + } + } + effect = effect.next; + } while (effect !== firstEffect); + } + } + } // We show the whole stack but dedupe on the top component's name because // the problematic code almost always lies inside that component. const componentName = getComponentName(fiber.type) || 'ReactComponent';