diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.old.js b/packages/react-reconciler/src/ReactFiberClassComponent.old.js index f055bac71ca94..6ea7435f3757d 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.old.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.old.js @@ -12,13 +12,14 @@ import type {Lanes} from './ReactFiberLane.old'; import type {UpdateQueue} from './ReactUpdateQueue.old'; 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.old'; import {isMounted} from './ReactFiberTreeReflection'; @@ -29,7 +30,7 @@ import invariant from 'shared/invariant'; import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols'; import {resolveDefaultProps} from './ReactFiberLazyComponent.old'; -import {DebugTracingMode, StrictMode} from './ReactTypeOfMode'; +import {DebugTracingMode, NoMode, StrictMode} from './ReactTypeOfMode'; import { enqueueUpdate, @@ -890,7 +891,15 @@ function mountClassInstance( } if (typeof instance.componentDidMount === 'function') { - workInProgress.flags |= Update; + if ( + __DEV__ && + enableDoubleInvokingEffects && + (workInProgress.mode & StrictMode) !== NoMode + ) { + workInProgress.flags |= MountLayoutDev | Update; + } else { + workInProgress.flags |= Update; + } } } @@ -960,7 +969,15 @@ 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.mode & StrictMode) !== NoMode + ) { + workInProgress.flags |= MountLayoutDev | Update; + } else { + workInProgress.flags |= Update; + } } return false; } @@ -1003,13 +1020,29 @@ function resumeMountClassInstance( } } if (typeof instance.componentDidMount === 'function') { - workInProgress.flags |= Update; + if ( + __DEV__ && + enableDoubleInvokingEffects && + (workInProgress.mode & StrictMode) !== NoMode + ) { + 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.mode & StrictMode) !== NoMode + ) { + 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.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index c134ecccddbb9..2d06431081f84 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -35,6 +35,7 @@ import { enableFundamentalAPI, enableSuspenseCallback, enableScopeAPI, + enableDoubleInvokingEffects, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -1797,6 +1798,116 @@ function commitResetTextContent(current: Fiber) { resetTextContent(current.stateNode); } +function invokeLayoutEffectMountInDEV(fiber: Fiber): void { + if (__DEV__ && enableDoubleInvokingEffects) { + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + invokeGuardedCallback( + null, + commitHookEffectListMount, + null, + HookLayout | HookHasEffect, + fiber, + ); + if (hasCaughtError()) { + const mountError = clearCaughtError(); + captureCommitPhaseError(fiber, mountError); + } + break; + } + case ClassComponent: { + const instance = fiber.stateNode; + invokeGuardedCallback(null, instance.componentDidMount, instance); + if (hasCaughtError()) { + const mountError = clearCaughtError(); + captureCommitPhaseError(fiber, mountError); + } + break; + } + } + } +} + +function invokePassiveEffectMountInDEV(fiber: Fiber): void { + if (__DEV__ && enableDoubleInvokingEffects) { + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + invokeGuardedCallback( + null, + commitHookEffectListMount, + null, + HookPassive | HookHasEffect, + fiber, + ); + if (hasCaughtError()) { + const mountError = clearCaughtError(); + captureCommitPhaseError(fiber, mountError); + } + break; + } + } + } +} + +function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { + if (__DEV__ && enableDoubleInvokingEffects) { + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + invokeGuardedCallback( + null, + commitHookEffectListUnmount, + null, + HookLayout | HookHasEffect, + fiber, + fiber.return, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(fiber, unmountError); + } + break; + } + case ClassComponent: { + const instance = fiber.stateNode; + if (typeof instance.componentWillUnmount === 'function') { + safelyCallComponentWillUnmount(fiber, instance); + } + break; + } + } + } +} + +function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { + if (__DEV__ && enableDoubleInvokingEffects) { + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + invokeGuardedCallback( + null, + commitHookEffectListUnmount, + null, + HookPassive | HookHasEffect, + fiber, + fiber.return, + ); + if (hasCaughtError()) { + const unmountError = clearCaughtError(); + captureCommitPhaseError(fiber, unmountError); + } + break; + } + } + } +} + export { commitBeforeMutationLifeCycles, commitResetTextContent, @@ -1806,4 +1917,8 @@ export { commitLifeCycles, commitAttachRef, commitDetachRef, + invokeLayoutEffectMountInDEV, + invokeLayoutEffectUnmountInDEV, + invokePassiveEffectMountInDEV, + invokePassiveEffectUnmountInDEV, }; diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index 8206eb00f67e0..529ed75814d31 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -28,9 +28,15 @@ import { enableCache, decoupleUpdatePriorityFromScheduler, enableUseRefAccessWarning, + enableDoubleInvokingEffects, } from 'shared/ReactFeatureFlags'; -import {NoMode, BlockingMode, DebugTracingMode} from './ReactTypeOfMode'; +import { + NoMode, + BlockingMode, + DebugTracingMode, + StrictMode, +} from './ReactTypeOfMode'; import { NoLane, NoLanes, @@ -49,6 +55,8 @@ import {readContext} from './ReactFiberNewContext.old'; import { Update as UpdateEffect, Passive as PassiveEffect, + MountLayoutDev as MountLayoutDevEffect, + MountPassiveDev as MountPassiveDevEffect, } from './ReactFiberFlags'; import { HasEffect as HookHasEffect, @@ -467,7 +475,20 @@ export function bailoutHooks( lanes: Lanes, ) { workInProgress.updateQueue = current.updateQueue; - workInProgress.flags &= ~(PassiveEffect | UpdateEffect); + if ( + __DEV__ && + enableDoubleInvokingEffects && + (workInProgress.mode & StrictMode) !== NoMode + ) { + workInProgress.flags &= ~( + MountLayoutDevEffect | + MountPassiveDevEffect | + PassiveEffect | + UpdateEffect + ); + } else { + workInProgress.flags &= ~(PassiveEffect | UpdateEffect); + } current.lanes = removeLanes(current.lanes, lanes); } @@ -1303,12 +1324,26 @@ function mountEffect( warnIfNotCurrentlyActingEffectsInDEV(currentlyRenderingFiber); } } - return mountEffectImpl( - UpdateEffect | PassiveEffect, - HookPassive, - create, - deps, - ); + + if ( + __DEV__ && + enableDoubleInvokingEffects && + (currentlyRenderingFiber.mode & StrictMode) !== NoMode + ) { + return mountEffectImpl( + UpdateEffect | PassiveEffect | MountPassiveDevEffect, + HookPassive, + create, + deps, + ); + } else { + return mountEffectImpl( + UpdateEffect | PassiveEffect, + HookPassive, + create, + deps, + ); + } } function updateEffect( @@ -1333,7 +1368,20 @@ function mountLayoutEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { - return mountEffectImpl(UpdateEffect, HookLayout, create, deps); + if ( + __DEV__ && + enableDoubleInvokingEffects && + (currentlyRenderingFiber.mode & StrictMode) !== NoMode + ) { + return mountEffectImpl( + UpdateEffect | MountLayoutDevEffect, + HookLayout, + create, + deps, + ); + } else { + return mountEffectImpl(UpdateEffect, HookLayout, create, deps); + } } function updateLayoutEffect( @@ -1392,12 +1440,25 @@ 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 && + (currentlyRenderingFiber.mode & StrictMode) !== NoMode + ) { + return mountEffectImpl( + UpdateEffect | MountLayoutDevEffect, + HookLayout, + imperativeHandleEffect.bind(null, create, ref), + effectDeps, + ); + } else { + return mountEffectImpl( + UpdateEffect, + HookLayout, + imperativeHandleEffect.bind(null, create, ref), + effectDeps, + ); + } } function updateImperativeHandle( @@ -1678,7 +1739,17 @@ function mountOpaqueIdentifier(): OpaqueIDType | void { const setId = mountState(id)[1]; if ((currentlyRenderingFiber.mode & BlockingMode) === NoMode) { - currentlyRenderingFiber.flags |= UpdateEffect | PassiveEffect; + if ( + __DEV__ && + enableDoubleInvokingEffects && + (currentlyRenderingFiber.mode & StrictMode) !== NoMode + ) { + currentlyRenderingFiber.flags |= + UpdateEffect | PassiveEffect | MountPassiveDevEffect; + } else { + currentlyRenderingFiber.flags |= UpdateEffect | PassiveEffect; + } + pushEffect( HookHasEffect | HookPassive, () => { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 6e97ab4a33514..68218f4f76833 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -14,6 +14,7 @@ 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 {Flags} from './ReactFiberFlags'; import type {StackCursor} from './ReactFiberStack.old'; import { @@ -31,6 +32,7 @@ import { enableDebugTracing, enableSchedulingProfiler, enableScopeAPI, + enableDoubleInvokingEffects, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import invariant from 'shared/invariant'; @@ -131,6 +133,8 @@ import { HostEffectMask, Hydrating, HydratingAndUpdate, + MountPassiveDev, + MountLayoutDev, } from './ReactFiberFlags'; import { NoLanePriority, @@ -191,6 +195,10 @@ import { commitPassiveEffectDurations, commitResetTextContent, isSuspenseBoundaryBeingHidden, + invokeLayoutEffectMountInDEV, + invokePassiveEffectMountInDEV, + invokeLayoutEffectUnmountInDEV, + invokePassiveEffectUnmountInDEV, } from './ReactFiberCommitWork.old'; import {enqueueUpdate} from './ReactUpdateQueue.old'; import {resetContextDependencies} from './ReactFiberNewContext.old'; @@ -2113,6 +2121,10 @@ function commitRootImpl(root, renderPriorityLevel) { pendingPassiveEffectsLanes = lanes; pendingPassiveEffectsRenderPriority = renderPriorityLevel; } else { + if (__DEV__ && enableDoubleInvokingEffects) { + commitDoubleInvokeEffectsInDEV(root, false); + } + // 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. @@ -2660,6 +2672,29 @@ function flushPassiveEffectsImpl() { } } + if (enableProfilerTimer && enableProfilerCommitHooks) { + const profilerEffects = pendingPassiveProfilerEffects; + pendingPassiveProfilerEffects = []; + for (let i = 0; i < profilerEffects.length; i++) { + const fiber = ((profilerEffects[i]: any): Fiber); + commitPassiveEffectDurations(root, fiber); + } + } + + if (__DEV__) { + if (enableDebugTracing) { + logPassiveEffectsStopped(); + } + } + + if (enableSchedulingProfiler) { + markPassiveEffectsStopped(); + } + + if (__DEV__ && enableDoubleInvokingEffects) { + commitDoubleInvokeEffectsInDEV(root, true); + } + // Note: This currently assumes there are no passive effects on the root fiber // because the root is not part of its own effect list. // This could change in the future. @@ -2674,13 +2709,8 @@ function flushPassiveEffectsImpl() { effect = nextNextEffect; } - if (enableProfilerTimer && enableProfilerCommitHooks) { - const profilerEffects = pendingPassiveProfilerEffects; - pendingPassiveProfilerEffects = []; - for (let i = 0; i < profilerEffects.length; i++) { - const fiber = ((profilerEffects[i]: any): Fiber); - commitPassiveEffectDurations(root, fiber); - } + if (__DEV__) { + isFlushingPassiveEffects = false; } if (enableSchedulerTracing) { @@ -2688,20 +2718,6 @@ function flushPassiveEffectsImpl() { finishPendingInteractions(root, lanes); } - if (__DEV__) { - isFlushingPassiveEffects = false; - } - - if (__DEV__) { - if (enableDebugTracing) { - logPassiveEffectsStopped(); - } - } - - if (enableSchedulingProfiler) { - markPassiveEffectsStopped(); - } - executionContext = prevExecutionContext; flushSyncCallbackQueue(); @@ -2985,6 +3001,49 @@ function flushRenderPhaseStrictModeWarningsInDEV() { } } +function commitDoubleInvokeEffectsInDEV( + root: FiberRoot, + hasPassiveEffects: boolean, +) { + if (__DEV__ && enableDoubleInvokingEffects) { + invokeEffectsInDev(root, MountLayoutDev, invokeLayoutEffectUnmountInDEV); + if (hasPassiveEffects) { + invokeEffectsInDev( + root, + MountPassiveDev, + invokePassiveEffectUnmountInDEV, + ); + } + + invokeEffectsInDev(root, MountLayoutDev, invokeLayoutEffectMountInDEV); + if (hasPassiveEffects) { + invokeEffectsInDev(root, MountPassiveDev, invokePassiveEffectMountInDEV); + } + } +} + +function invokeEffectsInDev( + root: FiberRoot, + fiberFlags: Flags, + invokeEffectFn: (fiber: Fiber) => void, +): void { + if (__DEV__ && enableDoubleInvokingEffects) { + // Note: This currently assumes there are no passive effects on the root fiber + // because the root is not part of its own effect list. + // This could change in the future. + let currentEffect = root.current.firstEffect; + while (currentEffect !== null) { + const flags = currentEffect.flags; + if ((flags & fiberFlags) !== NoFlags) { + setCurrentDebugFiberInDEV(currentEffect); + invokeEffectFn(currentEffect); + resetCurrentDebugFiberInDEV(); + } + currentEffect = currentEffect.nextEffect; + } + } +} + 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..802d08a946704 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.internal.js @@ -0,0 +1,771 @@ +/** + * 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; + +function shouldDoubleInvokingEffects() { + // For now, this feature only exists in the old fork (while the new fork is being bisected). + // Eventually we'll land it in both forks. + return __DEV__ && !__VARIANT__; +} + +describe('ReactDoubleInvokeEvents', () => { + beforeEach(() => { + jest.resetModules(); + React = require('react'); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + + ReactFeatureFlags.enableDoubleInvokingEffects = shouldDoubleInvokingEffects(); + }); + + it('should not double invoke effects outside of StrictMode', () => { + 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.renderLegacySyncRoot(); + }); + + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect mount', + 'useEffect mount', + ]); + }); + + it('should double invoke effects only within StrictMode subtrees', () => { + function ComponentWithEffects({label}) { + React.useEffect(() => { + Scheduler.unstable_yieldValue(`useEffect mount "${label}"`); + return () => + Scheduler.unstable_yieldValue(`useEffect unmount "${label}"`); + }); + + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue(`useLayoutEffect mount "${label}"`); + return () => + Scheduler.unstable_yieldValue(`useLayoutEffect unmount "${label}"`); + }); + + return label; + } + + ReactNoop.act(() => { + ReactNoop.renderLegacySyncRoot( + <> + + + + + , + ); + }); + + if (shouldDoubleInvokingEffects()) { + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect mount "loose"', + 'useLayoutEffect mount "strict"', + 'useEffect mount "loose"', + 'useEffect mount "strict"', + + 'useLayoutEffect unmount "strict"', + 'useEffect unmount "strict"', + 'useLayoutEffect mount "strict"', + 'useEffect mount "strict"', + ]); + } else { + expect(Scheduler).toHaveYielded([ + 'useLayoutEffect mount "loose"', + 'useLayoutEffect mount "strict"', + 'useEffect mount "loose"', + 'useEffect mount "strict"', + ]); + } + }); + + it('should flush double-invoked effects within the same frame as layout effects if there are no passive effects', () => { + function ComponentWithEffects({label}) { + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue(`useLayoutEffect mount "${label}"`); + return () => + Scheduler.unstable_yieldValue(`useLayoutEffect unmount "${label}"`); + }); + + return label; + } + + ReactNoop.act(() => { + ReactNoop.renderLegacySyncRoot( + + + , + ); + + if (shouldDoubleInvokingEffects()) { + expect(Scheduler).toFlushUntilNextPaint([ + 'useLayoutEffect mount "one"', + 'useLayoutEffect unmount "one"', + 'useLayoutEffect mount "one"', + ]); + } else { + expect(Scheduler).toFlushUntilNextPaint([ + 'useLayoutEffect mount "one"', + ]); + } + }); + + ReactNoop.act(() => { + ReactNoop.renderLegacySyncRoot( + + + + , + ); + + if (shouldDoubleInvokingEffects()) { + expect(Scheduler).toFlushUntilNextPaint([ + // Cleanup and re-run "one" (and "two") since there is no dependencies array. + 'useLayoutEffect unmount "one"', + 'useLayoutEffect mount "one"', + 'useLayoutEffect mount "two"', + + // Since "two" is new, it should be double-invoked. + 'useLayoutEffect unmount "two"', + 'useLayoutEffect mount "two"', + ]); + } else { + expect(Scheduler).toFlushUntilNextPaint([ + 'useLayoutEffect unmount "one"', + 'useLayoutEffect mount "one"', + 'useLayoutEffect mount "two"', + ]); + } + }); + }); + + // This test also verifies that double-invoked effects flush synchronously + // within the same frame as passive effects. + it('should double invoke effects only for newly mounted components', () => { + function ComponentWithEffects({label}) { + React.useEffect(() => { + Scheduler.unstable_yieldValue(`useEffect mount "${label}"`); + return () => + Scheduler.unstable_yieldValue(`useEffect unmount "${label}"`); + }); + + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue(`useLayoutEffect mount "${label}"`); + return () => + Scheduler.unstable_yieldValue(`useLayoutEffect unmount "${label}"`); + }); + + return label; + } + + ReactNoop.act(() => { + ReactNoop.renderLegacySyncRoot( + + + , + ); + + if (shouldDoubleInvokingEffects()) { + expect(Scheduler).toFlushAndYieldThrough([ + 'useLayoutEffect mount "one"', + ]); + expect(Scheduler).toFlushAndYield([ + 'useEffect mount "one"', + 'useLayoutEffect unmount "one"', + 'useEffect unmount "one"', + 'useLayoutEffect mount "one"', + 'useEffect mount "one"', + ]); + } else { + expect(Scheduler).toFlushAndYieldThrough([ + 'useLayoutEffect mount "one"', + ]); + expect(Scheduler).toFlushAndYield(['useEffect mount "one"']); + } + }); + + ReactNoop.act(() => { + ReactNoop.renderLegacySyncRoot( + + + + , + ); + + if (shouldDoubleInvokingEffects()) { + expect(Scheduler).toFlushAndYieldThrough([ + // Cleanup and re-run "one" (and "two") since there is no dependencies array. + 'useLayoutEffect unmount "one"', + 'useLayoutEffect mount "one"', + 'useLayoutEffect mount "two"', + ]); + expect(Scheduler).toFlushAndYield([ + 'useEffect unmount "one"', + 'useEffect mount "one"', + 'useEffect mount "two"', + + // Since "two" is new, it should be double-invoked. + 'useLayoutEffect unmount "two"', + 'useEffect unmount "two"', + 'useLayoutEffect mount "two"', + 'useEffect mount "two"', + ]); + } else { + expect(Scheduler).toFlushAndYieldThrough([ + 'useLayoutEffect unmount "one"', + 'useLayoutEffect mount "one"', + 'useLayoutEffect mount "two"', + ]); + expect(Scheduler).toFlushAndYield([ + 'useEffect unmount "one"', + 'useEffect mount "one"', + 'useEffect mount "two"', + ]); + } + }); + }); + + it('double invoking for effects for modern roots', () => { + 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 (shouldDoubleInvokingEffects()) { + 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 (shouldDoubleInvokingEffects()) { + 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 (shouldDoubleInvokingEffects()) { + 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 (shouldDoubleInvokingEffects()) { + 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('passes the right context to class component lifecycles', () => { + class App extends React.PureComponent { + test() {} + + componentDidMount() { + this.test(); + Scheduler.unstable_yieldValue('componentDidMount'); + } + + componentDidUpdate() { + this.test(); + Scheduler.unstable_yieldValue('componentDidUpdate'); + } + + componentWillUnmount() { + this.test(); + Scheduler.unstable_yieldValue('componentWillUnmount'); + } + + render() { + return null; + } + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + if (shouldDoubleInvokingEffects()) { + expect(Scheduler).toHaveYielded([ + 'componentDidMount', + 'componentWillUnmount', + 'componentDidMount', + ]); + } else { + expect(Scheduler).toHaveYielded(['componentDidMount']); + } + }); + + 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 (shouldDoubleInvokingEffects()) { + 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 (shouldDoubleInvokingEffects()) { + 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 (shouldDoubleInvokingEffects()) { + 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 (shouldDoubleInvokingEffects()) { + 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 (shouldDoubleInvokingEffects()) { + 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-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.js b/packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.js index 5b6abbc4f6593..6a1f00b7dd0bb 100644 --- a/packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.js +++ b/packages/react-reconciler/src/__tests__/ReactDoubleInvokeEvents-test.js @@ -26,9 +26,7 @@ describe('ReactDoubleInvokeEvents', () => { function supportsDoubleInvokeEffects() { return gate( flags => - flags.build === 'development' && - flags.enableDoubleInvokingEffects && - flags.dfsEffectsRefactor, + flags.build === 'development' && flags.enableDoubleInvokingEffects, ); } diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index eb0c69537df4b..972dacda7a119 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -4874,10 +4874,8 @@ describe('Profiler', () => { }); if (__DEV__) { - // @gate dfsEffectsRefactor - // @gate enableDoubleInvokingEffects it('double invoking does not disconnect wrapped async work', () => { - ReactFeatureFlags.enableDoubleInvokingEffects = true; + ReactFeatureFlags.enableDoubleInvokingEffects = !__VARIANT__; const callback = jest.fn(() => { const wrappedInteractions = SchedulerTracing.unstable_getCurrent(); @@ -4915,7 +4913,11 @@ describe('Profiler', () => { jest.runAllTimers(); - expect(callback).toHaveBeenCalledTimes(4); // 2x per effect + if (ReactFeatureFlags.enableDoubleInvokingEffects) { + expect(callback).toHaveBeenCalledTimes(4); // 2x per effect + } else { + expect(callback).toHaveBeenCalledTimes(2); + } expect(onInteractionScheduledWorkCompleted).toHaveBeenCalledTimes(1); });