From 0fd6805c6d0477be917d31949a73b3bbdccfe71c Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 22 Jan 2021 13:49:33 -0600 Subject: [PATCH] Land rest of effects refactor in main fork (#20644) --- .eslintrc.js | 2 +- .../src/ReactChildFiber.old.js | 22 +- .../react-reconciler/src/ReactFiber.old.js | 20 +- .../src/ReactFiberBeginWork.old.js | 40 +-- .../src/ReactFiberCommitWork.old.js | 201 ++++++++++----- .../src/ReactFiberCompleteWork.old.js | 57 +++-- .../src/ReactFiberHooks.old.js | 21 +- .../src/ReactFiberHydrationContext.old.js | 21 +- .../src/ReactFiberSuspenseComponent.old.js | 3 - .../src/ReactFiberThrow.old.js | 2 - .../src/ReactFiberWorkLoop.old.js | 237 +++--------------- 11 files changed, 234 insertions(+), 392 deletions(-) diff --git a/.eslintrc.js b/.eslintrc.js index bed7ff123afd1..a456964bdbdb0 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -116,7 +116,7 @@ module.exports = { 'react-internal/no-cross-fork-types': [ ERROR, { - old: ['firstEffect', 'nextEffect'], + old: [], new: [], }, ], diff --git a/packages/react-reconciler/src/ReactChildFiber.old.js b/packages/react-reconciler/src/ReactChildFiber.old.js index 036b84a5493a0..7a4ab0b172085 100644 --- a/packages/react-reconciler/src/ReactChildFiber.old.js +++ b/packages/react-reconciler/src/ReactChildFiber.old.js @@ -13,12 +13,7 @@ import type {Fiber} from './ReactInternalTypes'; import type {Lanes} from './ReactFiberLane.old'; import getComponentName from 'shared/getComponentName'; -import { - Deletion, - ChildDeletion, - Placement, - StaticMask, -} from './ReactFiberFlags'; +import {Placement, ChildDeletion} from './ReactFiberFlags'; import { getIteratorFn, REACT_ELEMENT_TYPE, @@ -268,21 +263,6 @@ function ChildReconciler(shouldTrackSideEffects) { // Noop. return; } - // Deletions are added in reversed order so we add it to the front. - // At this point, the return fiber's effect list is empty except for - // deletions, so we can just append the deletion to the list. The remaining - // effects aren't added until the complete phase. Once we implement - // resuming, this may not be true. - const last = returnFiber.lastEffect; - if (last !== null) { - last.nextEffect = childToDelete; - returnFiber.lastEffect = childToDelete; - } else { - returnFiber.firstEffect = returnFiber.lastEffect = childToDelete; - } - childToDelete.nextEffect = null; - childToDelete.flags = (childToDelete.flags & StaticMask) | Deletion; - const deletions = returnFiber.deletions; if (deletions === null) { returnFiber.deletions = [childToDelete]; diff --git a/packages/react-reconciler/src/ReactFiber.old.js b/packages/react-reconciler/src/ReactFiber.old.js index 1e20db2851007..c9783c3b7137f 100644 --- a/packages/react-reconciler/src/ReactFiber.old.js +++ b/packages/react-reconciler/src/ReactFiber.old.js @@ -144,10 +144,6 @@ function FiberNode( // Effects this.flags = NoFlags; - this.nextEffect = null; - - this.firstEffect = null; - this.lastEffect = null; this.subtreeFlags = NoFlags; this.deletions = null; @@ -285,10 +281,7 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber { // Reset the effect tag. workInProgress.flags = NoFlags; - // The effect list is no longer valid. - workInProgress.nextEffect = null; - workInProgress.firstEffect = null; - workInProgress.lastEffect = null; + // The effects are no longer valid. workInProgress.subtreeFlags = NoFlags; workInProgress.deletions = null; @@ -370,10 +363,7 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) { // that child fiber is setting, not the reconciliation. workInProgress.flags &= StaticMask | Placement; - // The effect list is no longer valid. - workInProgress.nextEffect = null; - workInProgress.firstEffect = null; - workInProgress.lastEffect = null; + // The effects are no longer valid. const current = workInProgress.alternate; if (current === null) { @@ -403,6 +393,9 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) { workInProgress.lanes = current.lanes; workInProgress.child = current.child; + // TODO: `subtreeFlags` should be reset to NoFlags, like we do in + // `createWorkInProgress`. Nothing reads this until the complete phase, + // currently, but it might in the future, and we should be consistent. workInProgress.subtreeFlags = current.subtreeFlags; workInProgress.deletions = null; workInProgress.memoizedProps = current.memoizedProps; @@ -847,9 +840,6 @@ export function assignFiberPropertiesInDEV( target.dependencies = source.dependencies; target.mode = source.mode; target.flags = source.flags; - target.nextEffect = source.nextEffect; - target.firstEffect = source.firstEffect; - target.lastEffect = source.lastEffect; target.subtreeFlags = source.subtreeFlags; target.deletions = source.deletions; target.lanes = source.lanes; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index bf03aeb24012f..5f9ba706b4229 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -67,7 +67,6 @@ import { DidCapture, Update, Ref, - Deletion, ChildDeletion, ForceUpdateForLegacySuspense, StaticMask, @@ -2199,10 +2198,6 @@ function updateSuspensePrimaryChildren( primaryChildFragment.sibling = null; if (currentFallbackChildFragment !== null) { // Delete the fallback child fragment - currentFallbackChildFragment.nextEffect = null; - currentFallbackChildFragment.flags = - (currentFallbackChildFragment.flags & StaticMask) | Deletion; - workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChildFragment; const deletions = workInProgress.deletions; if (deletions === null) { workInProgress.deletions = [currentFallbackChildFragment]; @@ -2264,22 +2259,9 @@ function updateSuspenseFallbackChildren( currentPrimaryChildFragment.treeBaseDuration; } - if (currentFallbackChildFragment !== null) { - // The fallback fiber was added as a deletion effect during the first - // pass. However, since we're going to remain on the fallback, we no - // longer want to delete it. So we need to remove it from the list. - // Deletions are stored on the same list as effects, and are always added - // to the front. So we know that the first effect must be the fallback - // deletion effect, and everything after that is from the primary free. - const firstPrimaryTreeEffect = currentFallbackChildFragment.nextEffect; - if (firstPrimaryTreeEffect !== null) { - workInProgress.firstEffect = firstPrimaryTreeEffect; - } else { - // TODO: Reset this somewhere else? Lol legacy mode is so weird. - workInProgress.firstEffect = workInProgress.lastEffect = null; - } - } - + // The fallback fiber was added as a deletion during the first pass. + // However, since we're going to remain on the fallback, we no longer want + // to delete it. workInProgress.deletions = null; } else { primaryChildFragment = createWorkInProgressOffscreenFiber( @@ -2776,7 +2758,6 @@ function initSuspenseListRenderState( tail: null | Fiber, lastContentRow: null | Fiber, tailMode: SuspenseListTailMode, - lastEffectBeforeRendering: null | Fiber, ): void { const renderState: null | SuspenseListRenderState = workInProgress.memoizedState; @@ -2788,7 +2769,6 @@ function initSuspenseListRenderState( last: lastContentRow, tail: tail, tailMode: tailMode, - lastEffect: lastEffectBeforeRendering, }: SuspenseListRenderState); } else { // We can reuse the existing object from previous renders. @@ -2798,7 +2778,6 @@ function initSuspenseListRenderState( renderState.last = lastContentRow; renderState.tail = tail; renderState.tailMode = tailMode; - renderState.lastEffect = lastEffectBeforeRendering; } } @@ -2880,7 +2859,6 @@ function updateSuspenseListComponent( tail, lastContentRow, tailMode, - workInProgress.lastEffect, ); break; } @@ -2912,7 +2890,6 @@ function updateSuspenseListComponent( tail, null, // last tailMode, - workInProgress.lastEffect, ); break; } @@ -2923,7 +2900,6 @@ function updateSuspenseListComponent( null, // tail null, // last undefined, - workInProgress.lastEffect, ); break; } @@ -3183,16 +3159,6 @@ function remountFiber( // Delete the old fiber and place the new one. // Since the old fiber is disconnected, we have to schedule it manually. - const last = returnFiber.lastEffect; - if (last !== null) { - last.nextEffect = current; - returnFiber.lastEffect = current; - } else { - returnFiber.firstEffect = returnFiber.lastEffect = current; - } - current.nextEffect = null; - current.flags = (current.flags & StaticMask) | Deletion; - const deletions = returnFiber.deletions; if (deletions === null) { returnFiber.deletions = [current]; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index bc443576e05dc..642f6dce02aa8 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -76,9 +76,10 @@ import { Hydrating, HydratingAndUpdate, Passive, + BeforeMutationMask, MutationMask, - PassiveMask, LayoutMask, + PassiveMask, PassiveUnmountPendingDev, } from './ReactFiberFlags'; import getComponentName from 'shared/getComponentName'; @@ -129,13 +130,13 @@ import { commitHydratedSuspenseInstance, clearContainer, prepareScopeUpdate, + prepareForCommit, + beforeActiveInstanceBlur, } from './ReactFiberHostConfig'; import { captureCommitPhaseError, resolveRetryWakeable, markCommitTimeOfFallback, - enqueuePendingPassiveHookEffectMount, - enqueuePendingPassiveHookEffectUnmount, enqueuePendingPassiveProfilerEffect, } from './ReactFiberWorkLoop.old'; import { @@ -145,6 +146,7 @@ import { Passive as HookPassive, } from './ReactHookEffectTags'; import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.old'; +import {doesFiberContain} from './ReactFiberTreeReflection'; let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; if (__DEV__) { @@ -260,18 +262,114 @@ function safelyCallDestroy(current: Fiber, destroy: () => void) { } } -function commitBeforeMutationLifeCycles( - current: Fiber | null, - finishedWork: Fiber, -): void { - switch (finishedWork.tag) { - case FunctionComponent: - case ForwardRef: - case SimpleMemoComponent: { +let focusedInstanceHandle: null | Fiber = null; +let shouldFireAfterActiveInstanceBlur: boolean = false; + +export function commitBeforeMutationEffects( + root: FiberRoot, + firstChild: Fiber, +) { + focusedInstanceHandle = prepareForCommit(root.containerInfo); + + nextEffect = firstChild; + commitBeforeMutationEffects_begin(); + + // We no longer need to track the active instance fiber + const shouldFire = shouldFireAfterActiveInstanceBlur; + shouldFireAfterActiveInstanceBlur = false; + focusedInstanceHandle = null; + + return shouldFire; +} + +function commitBeforeMutationEffects_begin() { + 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 deletion = deletions[i]; + commitBeforeMutationEffectsDeletion(deletion); + } + } + + const child = fiber.child; + if ( + (fiber.subtreeFlags & BeforeMutationMask) !== NoFlags && + child !== null + ) { + ensureCorrectReturnPointer(child, fiber); + nextEffect = child; + } else { + commitBeforeMutationEffects_complete(); + } + } +} + +function commitBeforeMutationEffects_complete() { + while (nextEffect !== null) { + const fiber = nextEffect; + if (__DEV__) { + setCurrentDebugFiberInDEV(fiber); + invokeGuardedCallback( + null, + commitBeforeMutationEffectsOnFiber, + null, + fiber, + ); + if (hasCaughtError()) { + const error = clearCaughtError(); + captureCommitPhaseError(fiber, error); + } + resetCurrentDebugFiberInDEV(); + } else { + try { + commitBeforeMutationEffectsOnFiber(fiber); + } catch (error) { + captureCommitPhaseError(fiber, error); + } + } + + const sibling = fiber.sibling; + if (sibling !== null) { + ensureCorrectReturnPointer(sibling, fiber.return); + nextEffect = sibling; return; } - case ClassComponent: { - if (finishedWork.flags & Snapshot) { + + nextEffect = fiber.return; + } +} + +function commitBeforeMutationEffectsOnFiber(finishedWork: Fiber) { + const current = finishedWork.alternate; + const flags = finishedWork.flags; + + if (!shouldFireAfterActiveInstanceBlur && focusedInstanceHandle !== null) { + // Check to see if the focused element was inside of a hidden (Suspense) subtree. + // TODO: Move this out of the hot path using a dedicated effect tag. + if ( + finishedWork.tag === SuspenseComponent && + isSuspenseBoundaryBeingHidden(current, finishedWork) && + doesFiberContain(finishedWork, focusedInstanceHandle) + ) { + shouldFireAfterActiveInstanceBlur = true; + beforeActiveInstanceBlur(finishedWork); + } + } + + if ((flags & Snapshot) !== NoFlags) { + setCurrentDebugFiberInDEV(finishedWork); + + switch (finishedWork.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + break; + } + case ClassComponent: { if (current !== null) { const prevProps = current.memoizedProps; const prevState = current.memoizedState; @@ -325,30 +423,43 @@ function commitBeforeMutationLifeCycles( } instance.__reactInternalSnapshotBeforeUpdate = snapshot; } + break; } - return; - } - case HostRoot: { - if (supportsMutation) { - if (finishedWork.flags & Snapshot) { + case HostRoot: { + if (supportsMutation) { const root = finishedWork.stateNode; clearContainer(root.containerInfo); } + break; + } + case HostComponent: + case HostText: + case HostPortal: + case IncompleteClassComponent: + // Nothing to do for these component types + break; + default: { + invariant( + false, + 'This unit of work tag should not have side-effects. This error is ' + + 'likely caused by a bug in React. Please file an issue.', + ); } - return; } - case HostComponent: - case HostText: - case HostPortal: - case IncompleteClassComponent: - // Nothing to do for these component types - return; + + resetCurrentDebugFiberInDEV(); + } +} + +function commitBeforeMutationEffectsDeletion(deletion: Fiber) { + // TODO (effects) It would be nice to avoid calling doesFiberContain() + // Maybe we can repurpose one of the subtreeFlags positions for this instead? + // Use it to store which part of the tree the focused instance is in? + // This assumes we can safely determine that instance during the "render" phase. + if (doesFiberContain(deletion, ((focusedInstanceHandle: any): Fiber))) { + shouldFireAfterActiveInstanceBlur = true; + beforeActiveInstanceBlur(deletion); } - invariant( - false, - 'This unit of work tag should not have side-effects. This error is ' + - 'likely caused by a bug in React. Please file an issue.', - ); } function commitHookEffectListUnmount(flags: HookFlags, finishedWork: Fiber) { @@ -421,26 +532,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, @@ -527,8 +618,6 @@ function commitLayoutEffectOnFiber( } else { commitHookEffectListMount(HookLayout | HookHasEffect, finishedWork); } - - schedulePassiveEffects(finishedWork); break; } case ClassComponent: { @@ -979,9 +1068,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 && @@ -1127,9 +1214,6 @@ export function detachFiberAfterEffects(fiber: Fiber): void { fiber.sibling = null; fiber.stateNode = null; fiber.updateQueue = null; - fiber.nextEffect = null; - fiber.firstEffect = null; - fiber.lastEffect = null; if (__DEV__) { fiber._debugOwner = null; @@ -2483,7 +2567,6 @@ function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { } export { - commitBeforeMutationLifeCycles, commitResetTextContent, commitPlacement, commitDeletion, diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js index a44029c8347c4..61c8a01bf27e3 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js @@ -72,7 +72,9 @@ import { NoFlags, DidCapture, Snapshot, + ChildDeletion, StaticMask, + MutationMask, } from './ReactFiberFlags'; import invariant from 'shared/invariant'; @@ -173,6 +175,31 @@ function markRef(workInProgress: Fiber) { workInProgress.flags |= Ref; } +function hadNoMutationsEffects(current: null | Fiber, completedWork: Fiber) { + const didBailout = current !== null && current.child === completedWork.child; + if (didBailout) { + return true; + } + + if ((completedWork.flags & ChildDeletion) !== NoFlags) { + return false; + } + + // TODO: If we move the `hadNoMutationsEffects` call after `bubbleProperties` + // then we only have to check the `completedWork.subtreeFlags`. + let child = completedWork.child; + while (child !== null) { + if ( + (child.flags & MutationMask) !== NoFlags || + (child.subtreeFlags & MutationMask) !== NoFlags + ) { + return false; + } + child = child.sibling; + } + return true; +} + let appendAllChildren; let updateHostContainer; let updateHostComponent; @@ -217,7 +244,7 @@ if (supportsMutation) { } }; - updateHostContainer = function(workInProgress: Fiber) { + updateHostContainer = function(current: null | Fiber, workInProgress: Fiber) { // Noop }; updateHostComponent = function( @@ -461,13 +488,13 @@ if (supportsMutation) { node = node.sibling; } }; - updateHostContainer = function(workInProgress: Fiber) { + updateHostContainer = function(current: null | Fiber, workInProgress: Fiber) { const portalOrRoot: { containerInfo: Container, pendingChildren: ChildSet, ... } = workInProgress.stateNode; - const childrenUnchanged = workInProgress.firstEffect === null; + const childrenUnchanged = hadNoMutationsEffects(current, workInProgress); if (childrenUnchanged) { // No changes, just reuse the existing instance. } else { @@ -492,7 +519,7 @@ if (supportsMutation) { const oldProps = current.memoizedProps; // If there are no effects associated with this node, then none of our children had any updates. // This guarantees that we can reuse all of them. - const childrenUnchanged = workInProgress.firstEffect === null; + const childrenUnchanged = hadNoMutationsEffects(current, workInProgress); if (childrenUnchanged && oldProps === newProps) { // No changes, just reuse the existing instance. // Note that this might release a previous clone. @@ -575,7 +602,7 @@ if (supportsMutation) { }; } else { // No host operations - updateHostContainer = function(workInProgress: Fiber) { + updateHostContainer = function(current: null | Fiber, workInProgress: Fiber) { // Noop }; updateHostComponent = function( @@ -847,7 +874,7 @@ function completeWork( workInProgress.flags |= Snapshot; } } - updateHostContainer(workInProgress); + updateHostContainer(current, workInProgress); bubbleProperties(workInProgress); return null; } @@ -1142,7 +1169,7 @@ function completeWork( } case HostPortal: popHostContainer(workInProgress); - updateHostContainer(workInProgress); + updateHostContainer(current, workInProgress); if (current === null) { preparePortalMount(workInProgress.stateNode.containerInfo); } @@ -1226,11 +1253,7 @@ function completeWork( // Rerender the whole list, but this time, we'll force fallbacks // to stay in place. - // Reset the effect list before doing the second pass since that's now invalid. - if (renderState.lastEffect === null) { - workInProgress.firstEffect = null; - } - workInProgress.lastEffect = renderState.lastEffect; + // Reset the effect flags before doing the second pass since that's now invalid. // Reset the child fibers to their original state. workInProgress.subtreeFlags = NoFlags; resetChildFibers(workInProgress, renderLanes); @@ -1301,15 +1324,6 @@ function completeWork( !renderedTail.alternate && !getIsHydrating() // We don't cut it if we're hydrating. ) { - // We need to delete the row we just rendered. - // Reset the effect list to what it was before we rendered this - // child. The nested children have already appended themselves. - const lastEffect = (workInProgress.lastEffect = - renderState.lastEffect); - // Remove any effects that were appended after this point. - if (lastEffect !== null) { - lastEffect.nextEffect = null; - } // We're done. bubbleProperties(workInProgress); return null; @@ -1369,7 +1383,6 @@ function completeWork( const next = renderState.tail; renderState.rendering = next; renderState.tail = next.sibling; - renderState.lastEffect = workInProgress.lastEffect; renderState.renderingStartTime = now(); next.sibling = null; diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index f8cd86cc7d730..26721dcd11264 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -488,8 +488,8 @@ export function bailoutHooks( (workInProgress.mode & (BlockingMode | ConcurrentMode)) !== NoMode ) { workInProgress.flags &= ~( - MountLayoutDevEffect | MountPassiveDevEffect | + MountLayoutDevEffect | PassiveEffect | UpdateEffect ); @@ -1337,17 +1337,14 @@ function mountEffect( (currentlyRenderingFiber.mode & (BlockingMode | ConcurrentMode)) !== NoMode ) { return mountEffectImpl( - MountPassiveDevEffect | - UpdateEffect | - PassiveEffect | - PassiveStaticEffect, + MountPassiveDevEffect | PassiveEffect | PassiveStaticEffect, HookPassive, create, deps, ); } else { return mountEffectImpl( - UpdateEffect | PassiveEffect | PassiveStaticEffect, + PassiveEffect | PassiveStaticEffect, HookPassive, create, deps, @@ -1365,12 +1362,7 @@ function updateEffect( warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber); } } - return updateEffectImpl( - UpdateEffect | PassiveEffect, - HookPassive, - create, - deps, - ); + return updateEffectImpl(PassiveEffect, HookPassive, create, deps); } function mountLayoutEffect( @@ -1749,10 +1741,9 @@ function mountOpaqueIdentifier(): OpaqueIDType | void { if ((currentlyRenderingFiber.mode & BlockingMode) === NoMode) { if (__DEV__ && enableDoubleInvokingEffects) { - currentlyRenderingFiber.flags |= - MountPassiveDevEffect | UpdateEffect | PassiveEffect; + currentlyRenderingFiber.flags |= MountPassiveDevEffect | PassiveEffect; } else { - currentlyRenderingFiber.flags |= UpdateEffect | PassiveEffect; + currentlyRenderingFiber.flags |= PassiveEffect; } pushEffect( HookHasEffect | HookPassive, diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js index e0ed2901c52d9..6f7c51e0a569f 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js @@ -24,13 +24,7 @@ import { HostRoot, SuspenseComponent, } from './ReactWorkTags'; -import { - Deletion, - ChildDeletion, - Placement, - Hydrating, - StaticMask, -} from './ReactFiberFlags'; +import {ChildDeletion, Placement, Hydrating} from './ReactFiberFlags'; import invariant from 'shared/invariant'; import { @@ -130,19 +124,6 @@ function deleteHydratableInstance( const childToDelete = createFiberFromHostInstanceForDeletion(); childToDelete.stateNode = instance; childToDelete.return = returnFiber; - childToDelete.flags = (childToDelete.flags & StaticMask) | Deletion; - - // This might seem like it belongs on progressedFirstDeletion. However, - // these children are not part of the reconciliation list of children. - // Even if we abort and rereconcile the children, that will try to hydrate - // again and the nodes are still in the host tree so these will be - // recreated. - if (returnFiber.lastEffect !== null) { - returnFiber.lastEffect.nextEffect = childToDelete; - returnFiber.lastEffect = childToDelete; - } else { - returnFiber.firstEffect = returnFiber.lastEffect = childToDelete; - } const deletions = returnFiber.deletions; if (deletions === null) { diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js index e98826932f921..b970646967608 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js @@ -60,9 +60,6 @@ export type SuspenseListRenderState = {| tail: null | Fiber, // Tail insertions setting. tailMode: SuspenseListTailMode, - // Last Effect before we rendered the "rendering" item. - // Used to remove new effects added by the rendered item. - lastEffect: null | Fiber, |}; export function shouldCaptureSuspense( diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index 781d523b48815..e7534497735c5 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -188,8 +188,6 @@ function throwException( ) { // The source fiber did not complete. sourceFiber.flags |= Incomplete; - // Its effect list is no longer valid. - sourceFiber.firstEffect = sourceFiber.lastEffect = null; if ( value !== null && diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 79386f5fff5c1..699b47596d0c8 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -13,7 +13,6 @@ import type {Lanes, Lane} from './ReactFiberLane.old'; import type {ReactPriorityLevel} from './ReactInternalTypes'; import type {Interaction} from 'scheduler/src/Tracing'; import type {SuspenseState} from './ReactFiberSuspenseComponent.old'; -import type {Effect as HookEffect} from './ReactFiberHooks.old'; import type {StackCursor} from './ReactFiberStack.old'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.old'; import type {Flags} from './ReactFiberFlags'; @@ -85,13 +84,11 @@ import * as Scheduler from 'scheduler'; import {__interactionsRef, __subscriberRef} from 'scheduler/tracing'; import { - prepareForCommit, resetAfterCommit, scheduleTimeout, cancelTimeout, noTimeout, warnsIfNotActing, - beforeActiveInstanceBlur, afterActiveInstanceBlur, clearContainer, } from './ReactFiberHostConfig'; @@ -122,17 +119,15 @@ import { import {LegacyRoot} from './ReactRootTags'; import { NoFlags, - PerformedWork, Placement, - Deletion, - ChildDeletion, - Snapshot, - Passive, PassiveStatic, Incomplete, HostEffectMask, Hydrating, - StaticMask, + BeforeMutationMask, + MutationMask, + LayoutMask, + PassiveMask, MountPassiveDev, MountLayoutDev, } from './ReactFiberFlags'; @@ -185,14 +180,12 @@ import { createClassErrorUpdate, } from './ReactFiberThrow.old'; import { - commitBeforeMutationLifeCycles as commitBeforeMutationEffectOnFiber, + commitBeforeMutationEffects, commitLayoutEffects, commitMutationEffects, commitPassiveEffectDurations, - isSuspenseBoundaryBeingHidden, commitPassiveMountEffects, commitPassiveUnmountEffects, - detachFiberAfterEffects, invokeLayoutEffectMountInDEV, invokePassiveEffectMountInDEV, invokeLayoutEffectUnmountInDEV, @@ -240,7 +233,6 @@ import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors'; // Used by `act` import enqueueTask from 'shared/enqueueTask'; -import {doesFiberContain} from './ReactFiberTreeReflection'; const ceil = Math.ceil; @@ -328,7 +320,6 @@ export function getRenderTargetTime(): number { return workInProgressRootRenderTargetTime; } -let nextEffect: Fiber | null = null; let hasUncaughtError = false; let firstUncaughtError = null; let legacyErrorBoundariesThatAlreadyFailed: Set | null = null; @@ -371,9 +362,6 @@ let currentEventPendingLanes: Lanes = NoLanes; // We warn about state updates for unmounted components differently in this case. let isFlushingPassiveEffects = false; -let focusedInstanceHandle: null | Fiber = null; -let shouldFireAfterActiveInstanceBlur: boolean = false; - export function getWorkInProgressRoot(): FiberRoot | null { return workInProgressRoot; } @@ -1737,45 +1725,6 @@ function completeUnitOfWork(unitOfWork: Fiber): void { workInProgress = next; return; } - - if ( - returnFiber !== null && - // Do not append effects to parents if a sibling failed to complete - (returnFiber.flags & Incomplete) === NoFlags - ) { - // Append all the effects of the subtree and this fiber onto the effect - // list of the parent. The completion order of the children affects the - // side-effect order. - if (returnFiber.firstEffect === null) { - returnFiber.firstEffect = completedWork.firstEffect; - } - if (completedWork.lastEffect !== null) { - if (returnFiber.lastEffect !== null) { - returnFiber.lastEffect.nextEffect = completedWork.firstEffect; - } - returnFiber.lastEffect = completedWork.lastEffect; - } - - // If this fiber had side-effects, we append it AFTER the children's - // side-effects. We can perform certain side-effects earlier if needed, - // by doing multiple passes over the effect list. We don't want to - // schedule our own side-effect on our own list because if end up - // reusing children we'll schedule this effect onto itself since we're - // at the end. - const flags = completedWork.flags; - - // Skip both NoWork and PerformedWork tags when creating the effect - // list. PerformedWork effect is read by React DevTools but shouldn't be - // committed. - if ((flags & ~StaticMask) > PerformedWork) { - if (returnFiber.lastEffect !== null) { - returnFiber.lastEffect.nextEffect = completedWork; - } else { - returnFiber.firstEffect = completedWork; - } - returnFiber.lastEffect = completedWork; - } - } } else { // This fiber did not complete because something threw. Pop values off // the stack without entering the complete phase. If this is a boundary, @@ -1812,8 +1761,7 @@ function completeUnitOfWork(unitOfWork: Fiber): void { } if (returnFiber !== null) { - // Mark the parent fiber as incomplete and clear its effect list. - returnFiber.firstEffect = returnFiber.lastEffect = null; + // Mark the parent fiber as incomplete and clear its subtree flags. returnFiber.flags |= Incomplete; returnFiber.subtreeFlags = NoFlags; returnFiber.deletions = null; @@ -1931,25 +1879,39 @@ function commitRootImpl(root, renderPriorityLevel) { // times out. } - // Get the list of effects. - let firstEffect; - if (finishedWork.flags > PerformedWork) { - // A fiber's effect list consists only of its children, not itself. So if - // the root has an effect, we need to add it to the end of the list. The - // resulting list is the set that would belong to the root's parent, if it - // had one; that is, all the effects in the tree including the root. - if (finishedWork.lastEffect !== null) { - finishedWork.lastEffect.nextEffect = finishedWork; - firstEffect = finishedWork.firstEffect; - } else { - firstEffect = finishedWork; + // 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.) + // TODO: Delete all other places that schedule the passive effect callback + // They're redundant. + if ( + (finishedWork.subtreeFlags & PassiveMask) !== NoFlags || + (finishedWork.flags & PassiveMask) !== NoFlags + ) { + if (!rootDoesHavePassiveEffects) { + rootDoesHavePassiveEffects = true; + scheduleCallback(NormalSchedulerPriority, () => { + flushPassiveEffects(); + return null; + }); } - } else { - // There is no effect on the root. - firstEffect = finishedWork.firstEffect; } - if (firstEffect !== 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(); @@ -1970,32 +1932,10 @@ function commitRootImpl(root, renderPriorityLevel) { // The first phase a "before mutation" phase. We use this phase to read the // state of the host tree right before we mutate it. This is where // getSnapshotBeforeUpdate is called. - focusedInstanceHandle = prepareForCommit(root.containerInfo); - shouldFireAfterActiveInstanceBlur = false; - - nextEffect = firstEffect; - do { - if (__DEV__) { - invokeGuardedCallback(null, commitBeforeMutationEffects, null); - if (hasCaughtError()) { - invariant(nextEffect !== null, 'Should be working on an effect.'); - const error = clearCaughtError(); - captureCommitPhaseError(nextEffect, error); - nextEffect = nextEffect.nextEffect; - } - } else { - try { - commitBeforeMutationEffects(); - } catch (error) { - invariant(nextEffect !== null, 'Should be working on an effect.'); - captureCommitPhaseError(nextEffect, error); - nextEffect = nextEffect.nextEffect; - } - } - } while (nextEffect !== null); - - // We no longer need to track the active instance fiber - focusedInstanceHandle = null; + const shouldFireAfterActiveInstanceBlur = commitBeforeMutationEffects( + root, + finishedWork, + ); if (enableProfilerTimer) { // Mark the current commit time to be shared by all Profilers in this @@ -2082,31 +2022,6 @@ function commitRootImpl(root, renderPriorityLevel) { rootWithPendingPassiveEffects = root; pendingPassiveEffectsLanes = lanes; pendingPassiveEffectsRenderPriority = renderPriorityLevel; - } else { - // We are done with the effect chain at this point so let's clear the - // nextEffect pointers to assist with GC. If we have passive effects, we'll - // clear this in flushPassiveEffects - // TODO: We should always do this in the passive phase, by scheduling - // a passive callback for every deletion. - nextEffect = firstEffect; - while (nextEffect !== null) { - const nextNextEffect = nextEffect.nextEffect; - nextEffect.nextEffect = null; - if (nextEffect.flags & ChildDeletion) { - const deletions = nextEffect.deletions; - if (deletions !== null) { - for (let i = 0; i < deletions.length; i++) { - const deletion = deletions[i]; - const alternate = deletion.alternate; - detachFiberAfterEffects(deletion); - if (alternate !== null) { - detachFiberAfterEffects(alternate); - } - } - } - } - nextEffect = nextNextEffect; - } } // Read this again, since an effect might have updated it @@ -2218,52 +2133,6 @@ function commitRootImpl(root, renderPriorityLevel) { return null; } -function commitBeforeMutationEffects() { - while (nextEffect !== null) { - const current = nextEffect.alternate; - - if (!shouldFireAfterActiveInstanceBlur && focusedInstanceHandle !== null) { - if ((nextEffect.flags & Deletion) !== NoFlags) { - if (doesFiberContain(nextEffect, focusedInstanceHandle)) { - shouldFireAfterActiveInstanceBlur = true; - beforeActiveInstanceBlur(nextEffect); - } - } else { - // TODO: Move this out of the hot path using a dedicated effect tag. - if ( - nextEffect.tag === SuspenseComponent && - isSuspenseBoundaryBeingHidden(current, nextEffect) && - doesFiberContain(nextEffect, focusedInstanceHandle) - ) { - shouldFireAfterActiveInstanceBlur = true; - beforeActiveInstanceBlur(nextEffect); - } - } - } - - const flags = nextEffect.flags; - if ((flags & Snapshot) !== NoFlags) { - setCurrentDebugFiberInDEV(nextEffect); - - commitBeforeMutationEffectOnFiber(current, nextEffect); - - 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; - } -} - export function flushPassiveEffects(): boolean { // Returns whether passive effects were flushed. if (pendingPassiveEffectsRenderPriority !== NoSchedulerPriority) { @@ -2302,32 +2171,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;