From c63741fb3daef6c1e8746cbe7d7b07ecb281a9fd Mon Sep 17 00:00:00 2001 From: Luna Ruan Date: Thu, 24 Sep 2020 13:42:17 -0700 Subject: [PATCH] offscreen double invoke effects (#19523) This PR double invokes effects in __DEV__ mode. We are thinking about unmounting layout and/or passive effects for a hidden tree. To understand potential issues with this, we want to double invoke effects. This PR changes the behavior in DEV when an effect runs from create() to create() -> destroy() -> create(). The effect cleanup function will still be called before the effect runs in both dev and prod. (Note: This change is purely for research for now as it is likely to break real code.) **Note: The change is fully behind a flag and does not affect any of the code on npm.** --- .../src/ReactFiberClassComponent.new.js | 27 +- .../src/ReactFiberCommitWork.new.js | 140 ++++- .../react-reconciler/src/ReactFiberFlags.js | 58 +- .../src/ReactFiberHooks.new.js | 75 ++- .../src/ReactFiberWorkLoop.new.js | 82 ++- .../ReactDoubleInvokeEvents-test.internal.js | 504 ++++++++++++++++++ .../__tests__/ReactProfiler-test.internal.js | 43 ++ packages/shared/ReactFeatureFlags.js | 2 + .../forks/ReactFeatureFlags.native-fb.js | 2 + .../forks/ReactFeatureFlags.native-oss.js | 2 + .../forks/ReactFeatureFlags.test-renderer.js | 2 + .../ReactFeatureFlags.test-renderer.native.js | 2 + .../ReactFeatureFlags.test-renderer.www.js | 2 + .../shared/forks/ReactFeatureFlags.testing.js | 2 + .../forks/ReactFeatureFlags.testing.www.js | 2 + .../forks/ReactFeatureFlags.www-dynamic.js | 2 + .../shared/forks/ReactFeatureFlags.www.js | 1 + 17 files changed, 888 insertions(+), 60 deletions(-) create mode 100644 packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.internal.js diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.new.js b/packages/react-reconciler/src/ReactFiberClassComponent.new.js index 4e58dd71f3dc6..4688c10998d17 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.new.js @@ -12,13 +12,14 @@ import type {Lanes} from './ReactFiberLane'; import type {UpdateQueue} from './ReactUpdateQueue.new'; import * as React from 'react'; -import {Update, Snapshot} from './ReactFiberFlags'; +import {Update, Snapshot, MountLayoutDev} from './ReactFiberFlags'; import { debugRenderPhaseSideEffectsForStrictMode, disableLegacyContext, enableDebugTracing, enableSchedulingProfiler, warnAboutDeprecatedLifecycles, + enableDoubleInvokingEffects, } from 'shared/ReactFeatureFlags'; import ReactStrictModeWarnings from './ReactStrictModeWarnings.new'; import {isMounted} from './ReactFiberTreeReflection'; @@ -890,7 +891,11 @@ function mountClassInstance( } if (typeof instance.componentDidMount === 'function') { - workInProgress.flags |= Update; + if (__DEV__ && enableDoubleInvokingEffects) { + workInProgress.flags |= MountLayoutDev | Update; + } else { + workInProgress.flags |= Update; + } } } @@ -960,7 +965,11 @@ function resumeMountClassInstance( // If an update was already in progress, we should schedule an Update // effect even though we're bailing out, so that cWU/cDU are called. if (typeof instance.componentDidMount === 'function') { - workInProgress.flags |= Update; + if (__DEV__ && enableDoubleInvokingEffects) { + workInProgress.flags |= MountLayoutDev | Update; + } else { + workInProgress.flags |= Update; + } } return false; } @@ -1003,13 +1012,21 @@ function resumeMountClassInstance( } } if (typeof instance.componentDidMount === 'function') { - workInProgress.flags |= Update; + if (__DEV__ && enableDoubleInvokingEffects) { + workInProgress.flags |= MountLayoutDev | Update; + } else { + workInProgress.flags |= Update; + } } } else { // If an update was already in progress, we should schedule an Update // effect even though we're bailing out, so that cWU/cDU are called. if (typeof instance.componentDidMount === 'function') { - workInProgress.flags |= Update; + if (__DEV__ && enableDoubleInvokingEffects) { + workInProgress.flags |= MountLayoutDev | Update; + } else { + workInProgress.flags |= Update; + } } // If shouldComponentUpdate returned false, we should still update the diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 68c592dd93f00..6d47fac3f4661 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -35,6 +35,7 @@ import { enableFundamentalAPI, enableSuspenseCallback, enableScopeAPI, + enableDoubleInvokingEffects, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -159,7 +160,7 @@ const callComponentWillUnmountWithTimer = function(current, instance) { function safelyCallComponentWillUnmount( current: Fiber, instance: any, - nearestMountedAncestor: Fiber, + nearestMountedAncestor: Fiber | null, ) { if (__DEV__) { invokeGuardedCallback( @@ -318,7 +319,7 @@ function commitBeforeMutationLifeCycles( } function commitHookEffectListUnmount( - tag: HookFlags, + flags: HookFlags, finishedWork: Fiber, nearestMountedAncestor: Fiber | null, ) { @@ -328,7 +329,7 @@ function commitHookEffectListUnmount( 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; @@ -341,14 +342,14 @@ function commitHookEffectListUnmount( } } -function commitHookEffectListMount(tag: HookFlags, finishedWork: Fiber) { +function commitHookEffectListMount(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) { // Mount const create = effect.create; effect.destroy = create(); @@ -1884,6 +1885,131 @@ function commitPassiveMount( } } +function invokeLayoutEffectMountInDEV(fiber: Fiber): void { + if (__DEV__ && enableDoubleInvokingEffects) { + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: + case Block: { + invokeGuardedCallback( + null, + commitHookEffectListMount, + null, + HookLayout | HookHasEffect, + fiber, + ); + if (hasCaughtError()) { + const mountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, mountError); + } + break; + } + case ClassComponent: { + const instance = fiber.stateNode; + invokeGuardedCallback(null, instance.componentDidMount, null); + if (hasCaughtError()) { + const mountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, mountError); + } + break; + } + } + } +} + +function invokePassiveEffectMountInDEV(fiber: Fiber): void { + if (__DEV__ && enableDoubleInvokingEffects) { + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: + case Block: { + invokeGuardedCallback( + null, + commitHookEffectListMount, + null, + HookPassive | HookHasEffect, + fiber, + ); + if (hasCaughtError()) { + const mountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, mountError); + } + break; + } + } + } +} + +function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { + if (__DEV__ && enableDoubleInvokingEffects) { + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: + case Block: { + invokeGuardedCallback( + null, + commitHookEffectListUnmount, + null, + HookLayout | HookHasEffect, + fiber, + fiber.return, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, unmountError); + } + break; + } + case ClassComponent: { + const instance = fiber.stateNode; + if (typeof instance.componentWillUnmount === 'function') { + invokeGuardedCallback( + null, + safelyCallComponentWillUnmount, + null, + fiber, + instance, + fiber.return, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, unmountError); + } + } + break; + } + } + } +} + +function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { + if (__DEV__ && enableDoubleInvokingEffects) { + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: + case Block: { + invokeGuardedCallback( + null, + commitHookEffectListUnmount, + null, + HookPassive | HookHasEffect, + fiber, + fiber.return, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(fiber, fiber.return, unmountError); + } + break; + } + } + } +} + export { commitBeforeMutationLifeCycles, commitResetTextContent, @@ -1896,4 +2022,8 @@ export { commitPassiveUnmount, commitPassiveUnmountInsideDeletedTree, commitPassiveMount, + invokeLayoutEffectMountInDEV, + invokeLayoutEffectUnmountInDEV, + invokePassiveEffectMountInDEV, + invokePassiveEffectUnmountInDEV, }; diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index 919957750c2f0..9fc29c29adb68 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -10,50 +10,56 @@ export type Flags = number; // Don't change these two values. They're used by React Dev Tools. -export const NoFlags = /* */ 0b0000000000000000; -export const PerformedWork = /* */ 0b0000000000000001; +export const NoFlags = /* */ 0b000000000000000000; +export const PerformedWork = /* */ 0b000000000000000001; // You can change the rest (and add more). -export const Placement = /* */ 0b0000000000000010; -export const Update = /* */ 0b0000000000000100; -export const PlacementAndUpdate = /* */ 0b0000000000000110; -export const Deletion = /* */ 0b0000000000001000; -export const ContentReset = /* */ 0b0000000000010000; -export const Callback = /* */ 0b0000000000100000; -export const DidCapture = /* */ 0b0000000001000000; -export const Ref = /* */ 0b0000000010000000; -export const Snapshot = /* */ 0b0000000100000000; -export const Passive = /* */ 0b0000001000000000; +export const Placement = /* */ 0b000000000000000010; +export const Update = /* */ 0b000000000000000100; +export const PlacementAndUpdate = /* */ 0b000000000000000110; +export const Deletion = /* */ 0b000000000000001000; +export const ContentReset = /* */ 0b000000000000010000; +export const Callback = /* */ 0b000000000000100000; +export const DidCapture = /* */ 0b000000000001000000; +export const Ref = /* */ 0b000000000010000000; +export const Snapshot = /* */ 0b000000000100000000; +export const Passive = /* */ 0b000000001000000000; // TODO (effects) Remove this bit once the new reconciler is synced to the old. -export const PassiveUnmountPendingDev = /* */ 0b0010000000000000; -export const Hydrating = /* */ 0b0000010000000000; -export const HydratingAndUpdate = /* */ 0b0000010000000100; +export const PassiveUnmountPendingDev = /* */ 0b000010000000000000; +export const Hydrating = /* */ 0b000000010000000000; +export const HydratingAndUpdate = /* */ 0b000000010000000100; // Passive & Update & Callback & Ref & Snapshot -export const LifecycleEffectMask = /* */ 0b0000001110100100; +export const LifecycleEffectMask = /* */ 0b000000001110100100; // Union of all host effects -export const HostEffectMask = /* */ 0b0000011111111111; +export const HostEffectMask = /* */ 0b000000011111111111; // These are not really side effects, but we still reuse this field. -export const Incomplete = /* */ 0b0000100000000000; -export const ShouldCapture = /* */ 0b0001000000000000; -export const ForceUpdateForLegacySuspense = /* */ 0b0100000000000000; +export const Incomplete = /* */ 0b000000100000000000; +export const ShouldCapture = /* */ 0b000001000000000000; +export const ForceUpdateForLegacySuspense = /* */ 0b000100000000000000; // Static tags describe aspects of a fiber that are not specific to a render, // e.g. a fiber uses a passive effect (even if there are no updates on this particular render). // This enables us to defer more work in the unmount case, // since we can defer traversing the tree during layout to look for Passive effects, // and instead rely on the static flag as a signal that there may be cleanup work. -export const PassiveStatic = /* */ 0b1000000000000000; +export const PassiveStatic = /* */ 0b001000000000000000; // Union of side effect groupings as pertains to subtreeFlags -export const BeforeMutationMask = /* */ 0b0000001100001010; -export const MutationMask = /* */ 0b0000010010011110; -export const LayoutMask = /* */ 0b0000000010100100; -export const PassiveMask = /* */ 0b0000001000001000; +export const BeforeMutationMask = /* */ 0b000000001100001010; +export const MutationMask = /* */ 0b000000010010011110; +export const LayoutMask = /* */ 0b000000000010100100; +export const PassiveMask = /* */ 0b000000001000001000; // Union of tags that don't get reset on clones. // This allows certain concepts to persist without recalculting them, // e.g. whether a subtree contains passive effects or portals. -export const StaticMask = /* */ 0b1000000000000000; +export const StaticMask = /* */ 0b001000000000000000; + +// These flags allow us to traverse to fibers that have effects on mount +// without traversing the entire tree after every commit for +// double invoking +export const MountLayoutDev = /* */ 0b010000000000000000; +export const MountPassiveDev = /* */ 0b100000000000000000; diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index ea7d5a983d242..92a71d2f82d5a 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -26,6 +26,7 @@ import { enableSchedulingProfiler, enableNewReconciler, decoupleUpdatePriorityFromScheduler, + enableDoubleInvokingEffects, } from 'shared/ReactFeatureFlags'; import {NoMode, BlockingMode, DebugTracingMode} from './ReactTypeOfMode'; @@ -48,6 +49,8 @@ import { Update as UpdateEffect, Passive as PassiveEffect, PassiveStatic as PassiveStaticEffect, + MountLayoutDev as MountLayoutDevEffect, + MountPassiveDev as MountPassiveDevEffect, } from './ReactFiberFlags'; import { HasEffect as HookHasEffect, @@ -482,7 +485,16 @@ export function bailoutHooks( lanes: Lanes, ) { workInProgress.updateQueue = current.updateQueue; - workInProgress.flags &= ~(PassiveEffect | UpdateEffect); + if (__DEV__ && enableDoubleInvokingEffects) { + workInProgress.flags &= ~( + MountPassiveDevEffect | + PassiveEffect | + MountLayoutDevEffect | + UpdateEffect + ); + } else { + workInProgress.flags &= ~(PassiveEffect | UpdateEffect); + } current.lanes = removeLanes(current.lanes, lanes); } @@ -1240,12 +1252,22 @@ function mountEffect( warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber); } } - return mountEffectImpl( - PassiveEffect | PassiveStaticEffect, - HookPassive, - create, - deps, - ); + + if (__DEV__ && enableDoubleInvokingEffects) { + return mountEffectImpl( + MountPassiveDevEffect | PassiveEffect | PassiveStaticEffect, + HookPassive, + create, + deps, + ); + } else { + return mountEffectImpl( + PassiveEffect | PassiveStaticEffect, + HookPassive, + create, + deps, + ); + } } function updateEffect( @@ -1265,7 +1287,16 @@ function mountLayoutEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { - return mountEffectImpl(UpdateEffect, HookLayout, create, deps); + if (__DEV__ && enableDoubleInvokingEffects) { + return mountEffectImpl( + MountLayoutDevEffect | UpdateEffect, + HookLayout, + create, + deps, + ); + } else { + return mountEffectImpl(UpdateEffect, HookLayout, create, deps); + } } function updateLayoutEffect( @@ -1324,12 +1355,21 @@ function mountImperativeHandle( const effectDeps = deps !== null && deps !== undefined ? deps.concat([ref]) : null; - return mountEffectImpl( - UpdateEffect, - HookLayout, - imperativeHandleEffect.bind(null, create, ref), - effectDeps, - ); + if (__DEV__ && enableDoubleInvokingEffects) { + return mountEffectImpl( + MountLayoutDevEffect | UpdateEffect, + HookLayout, + imperativeHandleEffect.bind(null, create, ref), + effectDeps, + ); + } else { + return mountEffectImpl( + UpdateEffect, + HookLayout, + imperativeHandleEffect.bind(null, create, ref), + effectDeps, + ); + } } function updateImperativeHandle( @@ -1610,7 +1650,12 @@ function mountOpaqueIdentifier(): OpaqueIDType | void { const setId = mountState(id)[1]; if ((currentlyRenderingFiber.mode & BlockingMode) === NoMode) { - currentlyRenderingFiber.flags |= PassiveEffect | PassiveStaticEffect; + if (__DEV__ && enableDoubleInvokingEffects) { + currentlyRenderingFiber.flags |= + MountPassiveDevEffect | PassiveEffect | PassiveStaticEffect; + } else { + currentlyRenderingFiber.flags |= PassiveEffect | PassiveStaticEffect; + } pushEffect( HookHasEffect | HookPassive, () => { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 1f9cf3235cf81..f9439601f5c50 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 {StackCursor} from './ReactFiberStack.new'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new'; +import type {Flags} from './ReactFiberFlags'; import { warnAboutDeprecatedLifecycles, @@ -30,6 +31,7 @@ import { enableScopeAPI, skipUnmountedBoundaries, disableSchedulerTimeoutInWorkLoop, + enableDoubleInvokingEffects, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; @@ -136,6 +138,8 @@ import { MutationMask, LayoutMask, PassiveMask, + MountPassiveDev, + MountLayoutDev, } from './ReactFiberFlags'; import { NoLanePriority, @@ -198,6 +202,10 @@ import { commitAttachRef, commitResetTextContent, isSuspenseBoundaryBeingHidden, + invokeLayoutEffectMountInDEV, + invokePassiveEffectMountInDEV, + invokeLayoutEffectUnmountInDEV, + invokePassiveEffectUnmountInDEV, } from './ReactFiberCommitWork.new'; import {enqueueUpdate} from './ReactUpdateQueue.new'; import {resetContextDependencies} from './ReactFiberNewContext.new'; @@ -2027,6 +2035,12 @@ function commitRootImpl(root, renderPriorityLevel) { legacyErrorBoundariesThatAlreadyFailed = null; } + if (__DEV__ && enableDoubleInvokingEffects) { + if (!rootDidHavePassiveEffects) { + commitDoubleInvokeEffectsInDEV(root.current, false); + } + } + if (enableSchedulerTracing) { if (!rootDidHavePassiveEffects) { // If there are no passive effects, then we can complete the pending interactions. @@ -2590,15 +2604,6 @@ function flushPassiveEffectsImpl() { flushPassiveUnmountEffects(root.current); flushPassiveMountEffects(root, root.current); - if (enableSchedulerTracing) { - popInteractions(((prevInteractions: any): Set)); - finishPendingInteractions(root, lanes); - } - - if (__DEV__) { - isFlushingPassiveEffects = false; - } - if (__DEV__) { if (enableDebugTracing) { logPassiveEffectsStopped(); @@ -2609,6 +2614,19 @@ function flushPassiveEffectsImpl() { markPassiveEffectsStopped(); } + if (__DEV__ && enableDoubleInvokingEffects) { + commitDoubleInvokeEffectsInDEV(root.current, true); + } + + if (__DEV__) { + isFlushingPassiveEffects = false; + } + + if (enableSchedulerTracing) { + popInteractions(((prevInteractions: any): Set)); + finishPendingInteractions(root, lanes); + } + executionContext = prevExecutionContext; flushSyncCallbackQueue(); @@ -2886,6 +2904,52 @@ function flushRenderPhaseStrictModeWarningsInDEV() { } } +function commitDoubleInvokeEffectsInDEV( + fiber: Fiber, + hasPassiveEffects: boolean, +) { + if (__DEV__ && enableDoubleInvokingEffects) { + setCurrentDebugFiberInDEV(fiber); + invokeEffectsInDev(fiber, MountLayoutDev, invokeLayoutEffectUnmountInDEV); + if (hasPassiveEffects) { + invokeEffectsInDev( + fiber, + MountPassiveDev, + invokePassiveEffectUnmountInDEV, + ); + } + + invokeEffectsInDev(fiber, MountLayoutDev, invokeLayoutEffectMountInDEV); + if (hasPassiveEffects) { + invokeEffectsInDev(fiber, MountPassiveDev, invokePassiveEffectMountInDEV); + } + resetCurrentDebugFiberInDEV(); + } +} + +function invokeEffectsInDev( + firstChild: Fiber, + fiberFlags: Flags, + invokeEffectFn: (fiber: Fiber) => void, +): void { + if (__DEV__ && enableDoubleInvokingEffects) { + let fiber = firstChild; + while (fiber !== null) { + if (fiber.child !== null) { + const primarySubtreeFlag = fiber.subtreeFlags & fiberFlags; + if (primarySubtreeFlag !== NoFlags) { + invokeEffectsInDev(fiber.child, fiberFlags, invokeEffectFn); + } + } + + if ((fiber.flags & fiberFlags) !== NoFlags) { + invokeEffectFn(fiber); + } + fiber = fiber.sibling; + } + } +} + let didWarnStateUpdateForNotYetMountedComponent: Set | null = null; function warnAboutUpdateOnNotYetMountedFiberInDEV(fiber) { if (__DEV__) { diff --git a/packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.internal.js b/packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.internal.js new file mode 100644 index 0000000000000..e6eaa8a9d615a --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.internal.js @@ -0,0 +1,504 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +let React; +let ReactFeatureFlags; +let ReactNoop; +let Scheduler; + +describe('ReactDoubleInvokeEvents', () => { + beforeEach(() => { + jest.resetModules(); + React = require('react'); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + ReactFeatureFlags.enableDoubleInvokingEffects = __VARIANT__; + }); + + it('double invoking for effects works properly', () => { + function App({text}) { + React.useEffect(() => { + Scheduler.unstable_yieldValue('useEffect mount'); + return () => Scheduler.unstable_yieldValue('useEffect unmount'); + }); + + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue('useLayoutEffect mount'); + return () => Scheduler.unstable_yieldValue('useLayoutEffect unmount'); + }); + + return text; + } + ReactNoop.act(() => { + ReactNoop.render(); + }); + + if (__DEV__ && __VARIANT__) { + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect mount', + 'useEffect mount', + 'useLayoutEffect unmount', + 'useEffect unmount', + 'useLayoutEffect mount', + 'useEffect mount', + ]); + } else { + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect mount', + 'useEffect mount', + ]); + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect unmount', + 'useLayoutEffect mount', + 'useEffect unmount', + 'useEffect mount', + ]); + + ReactNoop.act(() => { + ReactNoop.render(null); + }); + + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect unmount', + 'useEffect unmount', + ]); + }); + + it('multiple effects are double invoked in the right order (all mounted, all unmounted, all remounted)', () => { + function App({text}) { + React.useEffect(() => { + Scheduler.unstable_yieldValue('useEffect One mount'); + return () => Scheduler.unstable_yieldValue('useEffect One unmount'); + }); + + React.useEffect(() => { + Scheduler.unstable_yieldValue('useEffect Two mount'); + return () => Scheduler.unstable_yieldValue('useEffect Two unmount'); + }); + + return text; + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + if (__DEV__ && __VARIANT__) { + expect(Scheduler).toHaveYielded([ + 'useEffect One mount', + 'useEffect Two mount', + 'useEffect One unmount', + 'useEffect Two unmount', + 'useEffect One mount', + 'useEffect Two mount', + ]); + } else { + expect(Scheduler).toHaveYielded([ + 'useEffect One mount', + 'useEffect Two mount', + ]); + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + expect(Scheduler).toHaveYielded([ + 'useEffect One unmount', + 'useEffect Two unmount', + 'useEffect One mount', + 'useEffect Two mount', + ]); + + ReactNoop.act(() => { + ReactNoop.render(null); + }); + + expect(Scheduler).toHaveYielded([ + 'useEffect One unmount', + 'useEffect Two unmount', + ]); + }); + + it('multiple layout effects are double invoked in the right order (all mounted, all unmounted, all remounted)', () => { + function App({text}) { + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue('useLayoutEffect One mount'); + return () => + Scheduler.unstable_yieldValue('useLayoutEffect One unmount'); + }); + + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue('useLayoutEffect Two mount'); + return () => + Scheduler.unstable_yieldValue('useLayoutEffect Two unmount'); + }); + + return text; + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + if (__DEV__ && __VARIANT__) { + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect One mount', + 'useLayoutEffect Two mount', + 'useLayoutEffect One unmount', + 'useLayoutEffect Two unmount', + 'useLayoutEffect One mount', + 'useLayoutEffect Two mount', + ]); + } else { + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect One mount', + 'useLayoutEffect Two mount', + ]); + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect One unmount', + 'useLayoutEffect Two unmount', + 'useLayoutEffect One mount', + 'useLayoutEffect Two mount', + ]); + + ReactNoop.act(() => { + ReactNoop.render(null); + }); + + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect One unmount', + 'useLayoutEffect Two unmount', + ]); + }); + + it('useEffect and useLayoutEffect is called twice when there is no unmount', () => { + function App({text}) { + React.useEffect(() => { + Scheduler.unstable_yieldValue('useEffect mount'); + }); + + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue('useLayoutEffect mount'); + }); + + return text; + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + if (__DEV__ && __VARIANT__) { + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect mount', + 'useEffect mount', + 'useLayoutEffect mount', + 'useEffect mount', + ]); + } else { + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect mount', + 'useEffect mount', + ]); + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect mount', + 'useEffect mount', + ]); + + ReactNoop.act(() => { + ReactNoop.render(null); + }); + + expect(Scheduler).toHaveYielded([]); + }); + + it('double invoking works for class components', () => { + class App extends React.PureComponent { + componentDidMount() { + Scheduler.unstable_yieldValue('componentDidMount'); + } + + componentDidUpdate() { + Scheduler.unstable_yieldValue('componentDidUpdate'); + } + + componentWillUnmount() { + Scheduler.unstable_yieldValue('componentWillUnmount'); + } + + render() { + return this.props.text; + } + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + if (__DEV__ && __VARIANT__) { + expect(Scheduler).toHaveYielded([ + 'componentDidMount', + 'componentWillUnmount', + 'componentDidMount', + ]); + } else { + expect(Scheduler).toHaveYielded(['componentDidMount']); + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + expect(Scheduler).toHaveYielded(['componentDidUpdate']); + + ReactNoop.act(() => { + ReactNoop.render(null); + }); + + expect(Scheduler).toHaveYielded(['componentWillUnmount']); + }); + + it('double flushing passive effects only results in one double invoke', () => { + function App({text}) { + const [state, setState] = React.useState(0); + React.useEffect(() => { + if (state !== 1) { + setState(1); + } + Scheduler.unstable_yieldValue('useEffect mount'); + return () => Scheduler.unstable_yieldValue('useEffect unmount'); + }); + + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue('useLayoutEffect mount'); + return () => Scheduler.unstable_yieldValue('useLayoutEffect unmount'); + }); + + Scheduler.unstable_yieldValue(text); + return text; + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + if (__DEV__ && __VARIANT__) { + expect(Scheduler).toHaveYielded([ + 'mount', + 'useLayoutEffect mount', + 'useEffect mount', + 'useLayoutEffect unmount', + 'useEffect unmount', + 'useLayoutEffect mount', + 'useEffect mount', + 'mount', + 'useLayoutEffect unmount', + 'useLayoutEffect mount', + 'useEffect unmount', + 'useEffect mount', + ]); + } else { + expect(Scheduler).toHaveYielded([ + 'mount', + 'useLayoutEffect mount', + 'useEffect mount', + 'mount', + 'useLayoutEffect unmount', + 'useLayoutEffect mount', + 'useEffect unmount', + 'useEffect mount', + ]); + } + }); + + it('newly mounted components after initial mount get double invoked', () => { + let _setShowChild; + function Child() { + React.useEffect(() => { + Scheduler.unstable_yieldValue('Child useEffect mount'); + return () => Scheduler.unstable_yieldValue('Child useEffect unmount'); + }); + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue('Child useLayoutEffect mount'); + return () => + Scheduler.unstable_yieldValue('Child useLayoutEffect unmount'); + }); + + return null; + } + + function App() { + const [showChild, setShowChild] = React.useState(false); + _setShowChild = setShowChild; + React.useEffect(() => { + Scheduler.unstable_yieldValue('App useEffect mount'); + return () => Scheduler.unstable_yieldValue('App useEffect unmount'); + }); + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue('App useLayoutEffect mount'); + return () => + Scheduler.unstable_yieldValue('App useLayoutEffect unmount'); + }); + + return showChild && ; + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + if (__DEV__ && __VARIANT__) { + expect(Scheduler).toHaveYielded([ + 'App useLayoutEffect mount', + 'App useEffect mount', + 'App useLayoutEffect unmount', + 'App useEffect unmount', + 'App useLayoutEffect mount', + 'App useEffect mount', + ]); + } else { + expect(Scheduler).toHaveYielded([ + 'App useLayoutEffect mount', + 'App useEffect mount', + ]); + } + + ReactNoop.act(() => { + _setShowChild(true); + }); + + if (__DEV__ && __VARIANT__) { + expect(Scheduler).toHaveYielded([ + 'App useLayoutEffect unmount', + 'Child useLayoutEffect mount', + 'App useLayoutEffect mount', + 'App useEffect unmount', + 'Child useEffect mount', + 'App useEffect mount', + 'Child useLayoutEffect unmount', + 'Child useEffect unmount', + 'Child useLayoutEffect mount', + 'Child useEffect mount', + ]); + } else { + expect(Scheduler).toHaveYielded([ + 'App useLayoutEffect unmount', + 'Child useLayoutEffect mount', + 'App useLayoutEffect mount', + 'App useEffect unmount', + 'Child useEffect mount', + 'App useEffect mount', + ]); + } + }); + + it('classes and functions are double invoked together correctly', () => { + class ClassChild extends React.PureComponent { + componentDidMount() { + Scheduler.unstable_yieldValue('componentDidMount'); + } + + componentWillUnmount() { + Scheduler.unstable_yieldValue('componentWillUnmount'); + } + + render() { + return this.props.text; + } + } + + function FunctionChild({text}) { + React.useEffect(() => { + Scheduler.unstable_yieldValue('useEffect mount'); + return () => Scheduler.unstable_yieldValue('useEffect unmount'); + }); + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue('useLayoutEffect mount'); + return () => Scheduler.unstable_yieldValue('useLayoutEffect unmount'); + }); + return text; + } + + function App({text}) { + return ( + <> + + + + ); + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + if (__DEV__ && __VARIANT__) { + expect(Scheduler).toHaveYielded([ + 'componentDidMount', + 'useLayoutEffect mount', + 'useEffect mount', + 'componentWillUnmount', + 'useLayoutEffect unmount', + 'useEffect unmount', + 'componentDidMount', + 'useLayoutEffect mount', + 'useEffect mount', + ]); + } else { + expect(Scheduler).toHaveYielded([ + 'componentDidMount', + 'useLayoutEffect mount', + 'useEffect mount', + ]); + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect unmount', + 'useLayoutEffect mount', + 'useEffect unmount', + 'useEffect mount', + ]); + + ReactNoop.act(() => { + ReactNoop.render(null); + }); + + expect(Scheduler).toHaveYielded([ + 'componentWillUnmount', + 'useLayoutEffect unmount', + 'useEffect unmount', + ]); + }); +}); diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index 18c0266008b19..191cec9492cec 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -4149,6 +4149,49 @@ describe('Profiler', () => { expect(onRender.mock.calls[2][2]).toBe(15); // actual expect(onRender.mock.calls[2][3]).toBe(1 + 15); // base }); + + if (__DEV__) { + // @gate new + it('double invoking does not disconnect wrapped async work', () => { + ReactFeatureFlags.enableDoubleInvokingEffects = true; + + const callback = jest.fn(() => { + const wrappedInteractions = SchedulerTracing.unstable_getCurrent(); + // Expect wrappedInteractions and interactions to be the same set. + expect(wrappedInteractions).toMatchInteractions([interaction]); + }); + + const Component = jest.fn(() => { + React.useEffect(() => { + setTimeout(SchedulerTracing.unstable_wrap(callback), 0); + }); + React.useLayoutEffect(() => { + setTimeout(SchedulerTracing.unstable_wrap(callback), 0); + }); + + return null; + }); + + let interaction; + SchedulerTracing.unstable_trace( + 'event', + Scheduler.unstable_now(), + () => { + const interactions = SchedulerTracing.unstable_getCurrent(); + expect(interactions.size).toBe(1); + interaction = Array.from(interactions)[0]; + ReactTestRenderer.create(); + }, + ); + Scheduler.unstable_flushAll(); + + jest.runAllTimers(); + + expect(callback).toHaveBeenCalledTimes(4); // 2x per effect + + expect(onInteractionScheduledWorkCompleted).toHaveBeenCalledTimes(1); + }); + } }); }); }); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index cfc2cd06fca70..1d2516ab84385 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -136,3 +136,5 @@ export const enableDiscreteEventFlushingChange = false; export const enableEagerRootListeners = true; export const disableSchedulerTimeoutInWorkLoop = false; + +export const enableDoubleInvokingEffects = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index daf21367e8cee..62669046467f4 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -52,6 +52,8 @@ export const enableDiscreteEventFlushingChange = false; export const enableEagerRootListeners = true; export const disableSchedulerTimeoutInWorkLoop = false; +export const enableDoubleInvokingEffects = false; + // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 32067ec7dfaf0..ab03949d136d3 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -51,6 +51,8 @@ export const enableDiscreteEventFlushingChange = false; export const enableEagerRootListeners = true; export const disableSchedulerTimeoutInWorkLoop = false; +export const enableDoubleInvokingEffects = false; + // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index f763db6595bcc..14d831d5b3049 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -51,6 +51,8 @@ export const enableDiscreteEventFlushingChange = false; export const enableEagerRootListeners = true; export const disableSchedulerTimeoutInWorkLoop = false; +export const enableDoubleInvokingEffects = false; + // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index 71fabc1c706f4..bdf71ac54b3ee 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -51,6 +51,8 @@ export const enableDiscreteEventFlushingChange = false; export const enableEagerRootListeners = true; export const disableSchedulerTimeoutInWorkLoop = false; +export const enableDoubleInvokingEffects = false; + // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 952d506a65aa0..c4bb6b4e3c54f 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -51,6 +51,8 @@ export const enableDiscreteEventFlushingChange = false; export const enableEagerRootListeners = true; export const disableSchedulerTimeoutInWorkLoop = false; +export const enableDoubleInvokingEffects = false; + // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index 87d944781bfc4..9710569716ccd 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -51,6 +51,8 @@ export const enableDiscreteEventFlushingChange = false; export const enableEagerRootListeners = true; export const disableSchedulerTimeoutInWorkLoop = false; +export const enableDoubleInvokingEffects = false; + // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index 1a70e83d5a389..ae72c2ae4f956 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -51,6 +51,8 @@ export const enableDiscreteEventFlushingChange = true; export const enableEagerRootListeners = true; export const disableSchedulerTimeoutInWorkLoop = false; +export const enableDoubleInvokingEffects = false; + // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars type Check<_X, Y: _X, X: Y = _X> = null; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 871da4da31dd0..838f478b218b4 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -48,3 +48,5 @@ export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__; export const enableTrustedTypesIntegration = false; export const disableSchedulerTimeoutBasedOnReactExpirationTime = false; export const disableSchedulerTimeoutInWorkLoop = __VARIANT__; + +export const enableDoubleInvokingEffects = false; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 79caeb87b72b0..3bb82b29e91fa 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -29,6 +29,7 @@ export const { skipUnmountedBoundaries, enableEagerRootListeners, disableSchedulerTimeoutInWorkLoop, + enableDoubleInvokingEffects, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build.