diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 508dfca5da459..376a7c9670a4e 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -116,6 +116,7 @@ import { Snapshot, Callback, Passive, + PassiveUnmountPendingDev, Incomplete, HostEffectMask, Hydrating, @@ -2310,6 +2311,15 @@ export function enqueuePendingPassiveHookEffectUnmount( ): void { if (runAllPassiveEffectDestroysBeforeCreates) { pendingPassiveHookEffectsUnmount.push(effect, fiber); + if (__DEV__) { + if (deferPassiveEffectCleanupDuringUnmount) { + fiber.effectTag |= PassiveUnmountPendingDev; + const alternate = fiber.alternate; + if (alternate !== null) { + alternate.effectTag |= PassiveUnmountPendingDev; + } + } + } if (!rootDoesHavePassiveEffects) { rootDoesHavePassiveEffects = true; scheduleCallback(NormalPriority, () => { @@ -2372,6 +2382,17 @@ function flushPassiveEffectsImpl() { const fiber = ((unmountEffects[i + 1]: any): Fiber); const destroy = effect.destroy; effect.destroy = undefined; + + if (__DEV__) { + if (deferPassiveEffectCleanupDuringUnmount) { + fiber.effectTag &= ~PassiveUnmountPendingDev; + const alternate = fiber.alternate; + if (alternate !== null) { + alternate.effectTag &= ~PassiveUnmountPendingDev; + } + } + } + if (typeof destroy === 'function') { if (__DEV__) { setCurrentDebugFiberInDEV(fiber); @@ -2851,7 +2872,7 @@ function warnAboutUpdateOnUnmountedFiberInDEV(fiber) { ) { // If there are pending passive effects unmounts for this Fiber, // we can assume that they would have prevented this update. - if (pendingPassiveHookEffectsUnmount.indexOf(fiber) >= 0) { + if ((fiber.effectTag & PassiveUnmountPendingDev) !== NoEffect) { return; } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index ff50b56d3a580..713bcc23f85f2 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -116,6 +116,7 @@ import { Snapshot, Callback, Passive, + PassiveUnmountPendingDev, Incomplete, HostEffectMask, Hydrating, @@ -2329,6 +2330,15 @@ export function enqueuePendingPassiveHookEffectUnmount( ): void { if (runAllPassiveEffectDestroysBeforeCreates) { pendingPassiveHookEffectsUnmount.push(effect, fiber); + if (__DEV__) { + if (deferPassiveEffectCleanupDuringUnmount) { + fiber.effectTag |= PassiveUnmountPendingDev; + const alternate = fiber.alternate; + if (alternate !== null) { + alternate.effectTag |= PassiveUnmountPendingDev; + } + } + } if (!rootDoesHavePassiveEffects) { rootDoesHavePassiveEffects = true; scheduleCallback(NormalPriority, () => { @@ -2391,6 +2401,17 @@ function flushPassiveEffectsImpl() { const fiber = ((unmountEffects[i + 1]: any): Fiber); const destroy = effect.destroy; effect.destroy = undefined; + + if (__DEV__) { + if (deferPassiveEffectCleanupDuringUnmount) { + fiber.effectTag &= ~PassiveUnmountPendingDev; + const alternate = fiber.alternate; + if (alternate !== null) { + alternate.effectTag &= ~PassiveUnmountPendingDev; + } + } + } + if (typeof destroy === 'function') { if (__DEV__) { setCurrentDebugFiberInDEV(fiber); @@ -2873,7 +2894,7 @@ function warnAboutUpdateOnUnmountedFiberInDEV(fiber) { ) { // If there are pending passive effects unmounts for this Fiber, // we can assume that they would have prevented this update. - if (pendingPassiveHookEffectsUnmount.indexOf(fiber) >= 0) { + if ((fiber.effectTag & PassiveUnmountPendingDev) !== NoEffect) { return; } } diff --git a/packages/react-reconciler/src/ReactSideEffectTags.js b/packages/react-reconciler/src/ReactSideEffectTags.js index ad3f01ac4b939..162b860c19eb3 100644 --- a/packages/react-reconciler/src/ReactSideEffectTags.js +++ b/packages/react-reconciler/src/ReactSideEffectTags.js @@ -10,28 +10,29 @@ export type SideEffectTag = number; // Don't change these two values. They're used by React Dev Tools. -export const NoEffect = /* */ 0b0000000000000; -export const PerformedWork = /* */ 0b0000000000001; +export const NoEffect = /* */ 0b00000000000000; +export const PerformedWork = /* */ 0b00000000000001; // You can change the rest (and add more). -export const Placement = /* */ 0b0000000000010; -export const Update = /* */ 0b0000000000100; -export const PlacementAndUpdate = /* */ 0b0000000000110; -export const Deletion = /* */ 0b0000000001000; -export const ContentReset = /* */ 0b0000000010000; -export const Callback = /* */ 0b0000000100000; -export const DidCapture = /* */ 0b0000001000000; -export const Ref = /* */ 0b0000010000000; -export const Snapshot = /* */ 0b0000100000000; -export const Passive = /* */ 0b0001000000000; -export const Hydrating = /* */ 0b0010000000000; -export const HydratingAndUpdate = /* */ 0b0010000000100; +export const Placement = /* */ 0b00000000000010; +export const Update = /* */ 0b00000000000100; +export const PlacementAndUpdate = /* */ 0b00000000000110; +export const Deletion = /* */ 0b00000000001000; +export const ContentReset = /* */ 0b00000000010000; +export const Callback = /* */ 0b00000000100000; +export const DidCapture = /* */ 0b00000001000000; +export const Ref = /* */ 0b00000010000000; +export const Snapshot = /* */ 0b00000100000000; +export const Passive = /* */ 0b00001000000000; +export const PassiveUnmountPendingDev = /* */ 0b10000000000000; +export const Hydrating = /* */ 0b00010000000000; +export const HydratingAndUpdate = /* */ 0b00010000000100; // Passive & Update & Callback & Ref & Snapshot -export const LifecycleEffectMask = /* */ 0b0001110100100; +export const LifecycleEffectMask = /* */ 0b00001110100100; // Union of all host effects -export const HostEffectMask = /* */ 0b0011111111111; +export const HostEffectMask = /* */ 0b00011111111111; -export const Incomplete = /* */ 0b0100000000000; -export const ShouldCapture = /* */ 0b1000000000000; +export const Incomplete = /* */ 0b00100000000000; +export const ShouldCapture = /* */ 0b01000000000000; diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index ff7a695fdca8d..424dc60946ac5 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -1252,6 +1252,95 @@ describe('ReactHooksWithNoopRenderer', () => { }); }); + // @gate deferPassiveEffectCleanupDuringUnmount && runAllPassiveEffectDestroysBeforeCreates + it('does not warn about state updates for unmounted components with pending passive unmounts for alternates', () => { + let setParentState = null; + const setChildStates = []; + + function Parent() { + const [state, setState] = useState(true); + setParentState = setState; + Scheduler.unstable_yieldValue(`Parent ${state} render`); + useLayoutEffect(() => { + Scheduler.unstable_yieldValue(`Parent ${state} commit`); + }); + if (state) { + return ( + <> + + + + ); + } else { + return null; + } + } + + function Child({label}) { + const [state, setState] = useState(0); + useLayoutEffect(() => { + Scheduler.unstable_yieldValue(`Child ${label} commit`); + }); + useEffect(() => { + setChildStates.push(setState); + Scheduler.unstable_yieldValue(`Child ${label} passive create`); + return () => { + Scheduler.unstable_yieldValue(`Child ${label} passive destroy`); + }; + }, []); + Scheduler.unstable_yieldValue(`Child ${label} render`); + return state; + } + + // Schedule debounced state update for child (prob a no-op for this test) + // later tick: schedule unmount for parent + // start process unmount (but don't flush passive effectS) + // State update on child + act(() => { + ReactNoop.render(); + expect(Scheduler).toFlushAndYieldThrough([ + 'Parent true render', + 'Child one render', + 'Child two render', + 'Child one commit', + 'Child two commit', + 'Parent true commit', + 'Child one passive create', + 'Child two passive create', + ]); + + // Update children. + setChildStates.forEach(setChildState => setChildState(1)); + expect(Scheduler).toFlushAndYieldThrough([ + 'Child one render', + 'Child two render', + 'Child one commit', + 'Child two commit', + ]); + + // Schedule another update for children, and partially process it. + setChildStates.forEach(setChildState => setChildState(2)); + expect(Scheduler).toFlushAndYieldThrough(['Child one render']); + + // Schedule unmount for the parent that unmounts children with pending update. + Scheduler.unstable_runWithPriority( + Scheduler.unstable_UserBlockingPriority, + () => setParentState(false), + ); + expect(Scheduler).toFlushAndYieldThrough([ + 'Parent false render', + 'Parent false commit', + ]); + + // Schedule updates for children too (which should be ignored) + setChildStates.forEach(setChildState => setChildState(2)); + expect(Scheduler).toFlushAndYield([ + 'Child one passive destroy', + 'Child two passive destroy', + ]); + }); + }); + it('warns about state updates for unmounted components with no pending passive unmounts', () => { let completePendingRequest = null; function Component() {