diff --git a/packages/react-reconciler/src/ReactChildFiber.new.js b/packages/react-reconciler/src/ReactChildFiber.new.js index c0c131bb928b9..b43ed8bb8183b 100644 --- a/packages/react-reconciler/src/ReactChildFiber.new.js +++ b/packages/react-reconciler/src/ReactChildFiber.new.js @@ -15,16 +15,8 @@ import type {Fiber} from './ReactInternalTypes'; import type {Lanes} from './ReactFiberLane'; import getComponentName from 'shared/getComponentName'; -import { - Deletion, - NoEffect, - PassiveMask, - Placement, -} from './ReactSideEffectTags'; -import { - NoEffect as NoSubtreeTag, - Passive as PassiveSubtreeTag, -} from './ReactSubtreeTags'; +import {Deletion, Placement} from './ReactSideEffectTags'; +import {Passive as PassiveSubtreeTag} from './ReactSubtreeTags'; import { getIteratorFn, REACT_ELEMENT_TYPE, @@ -303,13 +295,11 @@ function ChildReconciler(shouldTrackSideEffects) { // TODO (effects) Rename this to better reflect its new usage (e.g. ChildDeletions) returnFiber.effectTag |= Deletion; - // If we are deleting a subtree that contains a passive effect, - // mark the parent so that we're sure to traverse after commit and run any unmount functions. - const primaryEffectTag = childToDelete.effectTag & PassiveMask; - const primarySubtreeTag = childToDelete.subtreeTag & PassiveSubtreeTag; - if (primaryEffectTag !== NoEffect || primarySubtreeTag !== NoSubtreeTag) { - returnFiber.subtreeTag |= PassiveSubtreeTag; - } + // We are deleting a subtree that may contain a passive effect. + // Mark the parent so we traverse this path after commit and run any unmount functions. + // This may cause us to traverse unnecessarily in some cases, but effects are common, + // and the cost of over traversing is small (just the path to the deleted node). + returnFiber.subtreeTag |= PassiveSubtreeTag; } else { deletions.push(childToDelete); } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 244472069a808..fa2445664b6fe 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -64,13 +64,9 @@ import { Ref, Deletion, ForceUpdateForLegacySuspense, - PassiveMask, StaticMask, } from './ReactSideEffectTags'; -import { - NoEffect as NoSubtreeTag, - Passive as PassiveSubtreeTag, -} from './ReactSubtreeTags'; +import {Passive as PassiveSubtreeTag} from './ReactSubtreeTags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import { debugRenderPhaseSideEffectsForStrictMode, @@ -2079,15 +2075,11 @@ function updateSuspensePrimaryChildren( // TODO (effects) Rename this to better reflect its new usage (e.g. ChildDeletions) workInProgress.effectTag |= Deletion; - // If we are deleting a subtree that contains a passive effect, - // mark the parent so that we're sure to traverse after commit and run any unmount functions. - const primaryEffectTag = - currentFallbackChildFragment.effectTag & PassiveMask; - const primarySubtreeTag = - currentFallbackChildFragment.subtreeTag & PassiveSubtreeTag; - if (primaryEffectTag !== NoEffect || primarySubtreeTag !== NoSubtreeTag) { - workInProgress.subtreeTag |= PassiveSubtreeTag; - } + // We are deleting a subtree that may contain a passive effect. + // Mark the parent so we traverse this path after commit and run any unmount functions. + // This may cause us to traverse unnecessarily in some cases, but effects are common, + // and the cost of over traversing is small (just the path to the deleted node). + workInProgress.subtreeTag |= PassiveSubtreeTag; } else { deletions.push(currentFallbackChildFragment); } @@ -3073,13 +3065,11 @@ function remountFiber( // TODO (effects) Rename this to better reflect its new usage (e.g. ChildDeletions) returnFiber.effectTag |= Deletion; - // If we are deleting a subtree that contains a passive effect, - // mark the parent so that we're sure to traverse after commit and run any unmount functions. - const primaryEffectTag = current.effectTag & PassiveMask; - const primarySubtreeTag = current.subtreeTag & PassiveSubtreeTag; - if (primaryEffectTag !== NoEffect || primarySubtreeTag !== NoSubtreeTag) { - returnFiber.subtreeTag |= PassiveSubtreeTag; - } + // We are deleting a subtree that may contain a passive effect. + // Mark the parent so we traverse this path after commit and run any unmount functions. + // This may cause us to traverse unnecessarily in some cases, but effects are common, + // and the cost of over traversing is small (just the path to the deleted node). + returnFiber.subtreeTag |= PassiveSubtreeTag; } else { deletions.push(current); } diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index 81201f82257e9..914370df882ec 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -24,17 +24,8 @@ import { HostRoot, SuspenseComponent, } from './ReactWorkTags'; -import { - Deletion, - Hydrating, - NoEffect, - PassiveMask, - Placement, -} from './ReactSideEffectTags'; -import { - NoEffect as NoSubtreeTag, - Passive as PassiveSubtreeTag, -} from './ReactSubtreeTags'; +import {Deletion, Hydrating, Placement} from './ReactSideEffectTags'; +import {Passive as PassiveSubtreeTag} from './ReactSubtreeTags'; import invariant from 'shared/invariant'; import { @@ -141,13 +132,11 @@ function deleteHydratableInstance( // TODO (effects) Rename this to better reflect its new usage (e.g. ChildDeletions) returnFiber.effectTag |= Deletion; - // If we are deleting a subtree that contains a passive effect, - // mark the parent so that we're sure to traverse after commit and run any unmount functions. - const primaryEffectTag = childToDelete.effectTag & PassiveMask; - const primarySubtreeTag = childToDelete.subtreeTag & PassiveSubtreeTag; - if (primaryEffectTag !== NoEffect || primarySubtreeTag !== NoSubtreeTag) { - returnFiber.subtreeTag |= PassiveSubtreeTag; - } + // We are deleting a subtree that may contain a passive effect. + // Mark the parent so we traverse this path after commit and run any unmount functions. + // This may cause us to traverse unnecessarily in some cases, but effects are common, + // and the cost of over traversing is small (just the path to the deleted node). + returnFiber.subtreeTag |= PassiveSubtreeTag; } else { deletions.push(childToDelete); } diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index 3c7440502957c..8c709cb593971 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -1454,7 +1454,7 @@ describe('ReactSuspense', () => { ]); }); - it('should call onInteractionScheduledWorkCompleted after suspending', done => { + it('should call onInteractionScheduledWorkCompleted after suspending', () => { const subscriber = { onInteractionScheduledWorkCompleted: jest.fn(), onInteractionTraced: jest.fn(), @@ -1512,13 +1512,11 @@ describe('ReactSuspense', () => { jest.advanceTimersByTime(1000); expect(Scheduler).toHaveYielded(['Promise resolved [C]']); - expect(Scheduler).toFlushExpired([ + expect(Scheduler).toFlushAndYield([ // Even though the promise for C was thrown three times, we should only // re-render once. 'C', ]); - - done(); }); expect(