From 95feb0e701a5ae20996e8cc6c4acd0f504d5985a Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 15 Jan 2021 14:24:20 -0600 Subject: [PATCH] Convert mutation phase to depth-first traversal (#20596) --- .../src/ReactChildFiber.new.js | 13 +- .../src/ReactFiberBeginWork.new.js | 10 +- .../src/ReactFiberCommitWork.new.js | 170 ++++++++++++++++++ .../src/ReactFiberHydrationContext.new.js | 5 +- .../src/ReactFiberWorkLoop.new.js | 152 +--------------- 5 files changed, 180 insertions(+), 170 deletions(-) diff --git a/packages/react-reconciler/src/ReactChildFiber.new.js b/packages/react-reconciler/src/ReactChildFiber.new.js index 7163df34d8568..83362b5e15411 100644 --- a/packages/react-reconciler/src/ReactChildFiber.new.js +++ b/packages/react-reconciler/src/ReactChildFiber.new.js @@ -282,22 +282,13 @@ function ChildReconciler(shouldTrackSideEffects) { childToDelete.nextEffect = null; childToDelete.flags = (childToDelete.flags & StaticMask) | Deletion; - let deletions = returnFiber.deletions; + const deletions = returnFiber.deletions; if (deletions === null) { - deletions = returnFiber.deletions = [childToDelete]; + returnFiber.deletions = [childToDelete]; returnFiber.flags |= ChildDeletion; } else { deletions.push(childToDelete); } - // Stash a reference to the return fiber's deletion array on each of the - // deleted children. This is really weird, but it's a temporary workaround - // while we're still using the effect list to traverse effect fibers. A - // better workaround would be to follow the `.return` pointer in the commit - // phase, but unfortunately we can't assume that `.return` points to the - // correct fiber, even in the commit phase, because `findDOMNode` might - // mutate it. - // TODO: Remove this line. - childToDelete.deletions = deletions; } function deleteRemainingChildren( diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 52e6216d442eb..9684efafc409c 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -2203,14 +2203,13 @@ function updateSuspensePrimaryChildren( currentFallbackChildFragment.flags = (currentFallbackChildFragment.flags & StaticMask) | Deletion; workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChildFragment; - let deletions = workInProgress.deletions; + const deletions = workInProgress.deletions; if (deletions === null) { - deletions = workInProgress.deletions = [currentFallbackChildFragment]; + workInProgress.deletions = [currentFallbackChildFragment]; workInProgress.flags |= ChildDeletion; } else { deletions.push(currentFallbackChildFragment); } - currentFallbackChildFragment.deletions = deletions; } workInProgress.child = primaryChildFragment; @@ -3194,14 +3193,13 @@ function remountFiber( current.nextEffect = null; current.flags = (current.flags & StaticMask) | Deletion; - let deletions = returnFiber.deletions; + const deletions = returnFiber.deletions; if (deletions === null) { - deletions = returnFiber.deletions = [current]; + returnFiber.deletions = [current]; returnFiber.flags |= ChildDeletion; } else { deletions.push(current); } - current.deletions = deletions; newWorkInProgress.flags |= Placement; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 4f77868463fc5..35115140041c7 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -66,12 +66,16 @@ import { NoFlags, ContentReset, Placement, + PlacementAndUpdate, ChildDeletion, Snapshot, Update, Callback, Ref, + Hydrating, + HydratingAndUpdate, Passive, + MutationMask, PassiveMask, LayoutMask, PassiveUnmountPendingDev, @@ -1841,6 +1845,172 @@ function commitResetTextContent(current: Fiber) { resetTextContent(current.stateNode); } +export function commitMutationEffects( + root: FiberRoot, + renderPriorityLevel: ReactPriorityLevel, + firstChild: Fiber, +) { + nextEffect = firstChild; + commitMutationEffects_begin(root, renderPriorityLevel); +} + +function commitMutationEffects_begin( + root: FiberRoot, + renderPriorityLevel: ReactPriorityLevel, +) { + while (nextEffect !== null) { + const fiber = nextEffect; + + // TODO: Should wrap this in flags check, too, as optimization + const deletions = fiber.deletions; + if (deletions !== null) { + for (let i = 0; i < deletions.length; i++) { + const childToDelete = deletions[i]; + if (__DEV__) { + invokeGuardedCallback( + null, + commitDeletion, + null, + root, + childToDelete, + renderPriorityLevel, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(childToDelete, error); + } + } else { + try { + commitDeletion(root, childToDelete, renderPriorityLevel); + } catch (error) { + captureCommitPhaseError(childToDelete, error); + } + } + } + } + + const child = fiber.child; + if ((fiber.subtreeFlags & MutationMask) !== NoFlags && child !== null) { + ensureCorrectReturnPointer(child, fiber); + nextEffect = child; + } else { + commitMutationEffects_complete(root, renderPriorityLevel); + } + } +} + +function commitMutationEffects_complete( + root: FiberRoot, + renderPriorityLevel: ReactPriorityLevel, +) { + while (nextEffect !== null) { + const fiber = nextEffect; + if (__DEV__) { + setCurrentDebugFiberInDEV(fiber); + invokeGuardedCallback( + null, + commitMutationEffectsOnFiber, + null, + fiber, + root, + renderPriorityLevel, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(fiber, error); + } + resetCurrentDebugFiberInDEV(); + } else { + try { + commitMutationEffectsOnFiber(fiber, root, renderPriorityLevel); + } catch (error) { + captureCommitPhaseError(fiber, error); + } + } + + const sibling = fiber.sibling; + if (sibling !== null) { + ensureCorrectReturnPointer(sibling, fiber.return); + nextEffect = sibling; + return; + } + + nextEffect = fiber.return; + } +} + +function commitMutationEffectsOnFiber( + finishedWork: Fiber, + root: FiberRoot, + renderPriorityLevel: ReactPriorityLevel, +) { + const flags = finishedWork.flags; + + if (flags & ContentReset) { + commitResetTextContent(finishedWork); + } + + if (flags & Ref) { + const current = finishedWork.alternate; + if (current !== null) { + commitDetachRef(current); + } + if (enableScopeAPI) { + // TODO: This is a temporary solution that allowed us to transition away + // from React Flare on www. + if (finishedWork.tag === ScopeComponent) { + commitAttachRef(finishedWork); + } + } + } + + // The following switch statement is only concerned about placement, + // updates, and deletions. To avoid needing to add a case for every possible + // bitmap value, we remove the secondary effects from the effect tag and + // switch on that value. + const primaryFlags = flags & (Placement | Update | Hydrating); + outer: switch (primaryFlags) { + case Placement: { + commitPlacement(finishedWork); + // Clear the "placement" from effect tag so that we know that this is + // inserted, before any life-cycles like componentDidMount gets called. + // TODO: findDOMNode doesn't rely on this any more but isMounted does + // and isMounted is deprecated anyway so we should be able to kill this. + finishedWork.flags &= ~Placement; + break; + } + case PlacementAndUpdate: { + // Placement + commitPlacement(finishedWork); + // Clear the "placement" from effect tag so that we know that this is + // inserted, before any life-cycles like componentDidMount gets called. + finishedWork.flags &= ~Placement; + + // Update + const current = finishedWork.alternate; + commitWork(current, finishedWork); + break; + } + case Hydrating: { + finishedWork.flags &= ~Hydrating; + break; + } + case HydratingAndUpdate: { + finishedWork.flags &= ~Hydrating; + + // Update + const current = finishedWork.alternate; + commitWork(current, finishedWork); + break; + } + case Update: { + const current = finishedWork.alternate; + commitWork(current, finishedWork); + break; + } + } +} + export function commitLayoutEffects( finishedWork: Fiber, root: FiberRoot, diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index 37be28be6628c..f416c33b21f1b 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -144,14 +144,13 @@ function deleteHydratableInstance( returnFiber.firstEffect = returnFiber.lastEffect = childToDelete; } - let deletions = returnFiber.deletions; + const deletions = returnFiber.deletions; if (deletions === null) { - deletions = returnFiber.deletions = [childToDelete]; + returnFiber.deletions = [childToDelete]; returnFiber.flags |= ChildDeletion; } else { deletions.push(childToDelete); } - childToDelete.deletions = deletions; } function insertNonHydratedInstance(returnFiber: Fiber, fiber: Fiber) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 37f513f29d51f..4eafe42585043 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -31,7 +31,6 @@ import { decoupleUpdatePriorityFromScheduler, enableDebugTracing, enableSchedulingProfiler, - enableScopeAPI, disableSchedulerTimeoutInWorkLoop, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -116,7 +115,6 @@ import { ForwardRef, MemoComponent, SimpleMemoComponent, - ScopeComponent, Profiler, } from './ReactWorkTags'; import {LegacyRoot} from './ReactRootTags'; @@ -124,19 +122,14 @@ import { NoFlags, PerformedWork, Placement, - Update, - PlacementAndUpdate, Deletion, ChildDeletion, - Ref, - ContentReset, Snapshot, Passive, PassiveStatic, Incomplete, HostEffectMask, Hydrating, - HydratingAndUpdate, StaticMask, } from './ReactFiberFlags'; import { @@ -190,13 +183,8 @@ import { import { commitBeforeMutationLifeCycles as commitBeforeMutationEffectOnFiber, commitLayoutEffects, - commitPlacement, - commitWork, - commitDeletion, - commitDetachRef, - commitAttachRef, + commitMutationEffects, commitPassiveEffectDurations, - commitResetTextContent, isSuspenseBoundaryBeingHidden, commitPassiveMountEffects, commitPassiveUnmountEffects, @@ -2031,32 +2019,7 @@ function commitRootImpl(root, renderPriorityLevel) { } // The next phase is the mutation phase, where we mutate the host tree. - nextEffect = firstEffect; - do { - if (__DEV__) { - invokeGuardedCallback( - null, - commitMutationEffects, - null, - root, - renderPriorityLevel, - ); - if (hasCaughtError()) { - invariant(nextEffect !== null, 'Should be working on an effect.'); - const error = clearCaughtError(); - captureCommitPhaseError(nextEffect, error); - nextEffect = nextEffect.nextEffect; - } - } else { - try { - commitMutationEffects(root, renderPriorityLevel); - } catch (error) { - invariant(nextEffect !== null, 'Should be working on an effect.'); - captureCommitPhaseError(nextEffect, error); - nextEffect = nextEffect.nextEffect; - } - } - } while (nextEffect !== null); + commitMutationEffects(root, renderPriorityLevel, finishedWork); if (shouldFireAfterActiveInstanceBlur) { afterActiveInstanceBlur(); @@ -2304,117 +2267,6 @@ function commitBeforeMutationEffects() { } } -function commitMutationEffects( - root: FiberRoot, - renderPriorityLevel: ReactPriorityLevel, -) { - // TODO: Should probably move the bulk of this function to commitWork. - while (nextEffect !== null) { - setCurrentDebugFiberInDEV(nextEffect); - - const flags = nextEffect.flags; - - if (flags & ContentReset) { - commitResetTextContent(nextEffect); - } - - if (flags & Ref) { - const current = nextEffect.alternate; - if (current !== null) { - commitDetachRef(current); - } - if (enableScopeAPI) { - // TODO: This is a temporary solution that allowed us to transition away - // from React Flare on www. - if (nextEffect.tag === ScopeComponent) { - commitAttachRef(nextEffect); - } - } - } - - // The following switch statement is only concerned about placement, - // updates, and deletions. To avoid needing to add a case for every possible - // bitmap value, we remove the secondary effects from the effect tag and - // switch on that value. - const primaryFlags = flags & (Placement | Update | Deletion | Hydrating); - outer: switch (primaryFlags) { - case Placement: { - commitPlacement(nextEffect); - // Clear the "placement" from effect tag so that we know that this is - // inserted, before any life-cycles like componentDidMount gets called. - // TODO: findDOMNode doesn't rely on this any more but isMounted does - // and isMounted is deprecated anyway so we should be able to kill this. - nextEffect.flags &= ~Placement; - break; - } - case PlacementAndUpdate: { - // Placement - commitPlacement(nextEffect); - // Clear the "placement" from effect tag so that we know that this is - // inserted, before any life-cycles like componentDidMount gets called. - nextEffect.flags &= ~Placement; - - // Update - const current = nextEffect.alternate; - commitWork(current, nextEffect); - break; - } - case Hydrating: { - nextEffect.flags &= ~Hydrating; - break; - } - case HydratingAndUpdate: { - nextEffect.flags &= ~Hydrating; - - // Update - const current = nextEffect.alternate; - commitWork(current, nextEffect); - break; - } - case Update: { - const current = nextEffect.alternate; - commitWork(current, nextEffect); - break; - } - case Deletion: { - // Reached a deletion effect. Instead of commit this effect like we - // normally do, we're going to use the `deletions` array of the parent. - // However, because the effect list is sorted in depth-first order, we - // can't wait until we reach the parent node, because the child effects - // will have run in the meantime. - // - // So instead, we use a trick where the first time we hit a deletion - // effect, we commit all the deletion effects that belong to that parent. - // - // This is an incremental step away from using the effect list and - // toward a DFS + subtreeFlags traversal. - // - // A reference to the deletion array of the parent is also stored on - // each of the deletions. This is really weird. It would be better to - // follow the `.return` pointer, but unfortunately we can't assume that - // `.return` points to the correct fiber, even in the commit phase, - // because `findDOMNode` might mutate it. - const deletedChild = nextEffect; - const deletions = deletedChild.deletions; - if (deletions !== null) { - for (let i = 0; i < deletions.length; i++) { - const deletion = deletions[i]; - // Clear the deletion effect so that we don't delete this node more - // than once. - deletion.flags &= ~Deletion; - deletion.deletions = null; - commitDeletion(root, deletion, renderPriorityLevel); - } - } - break; - } - } - - resetCurrentDebugFiberInDEV(); - nextEffect = nextEffect.nextEffect; - } -} - export function flushPassiveEffects(): boolean { // Returns whether passive effects were flushed. if (pendingPassiveEffectsRenderPriority !== NoSchedulerPriority) {