From 358414f82d1db7a3f46d51ece6434fcf02b14f5f Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Fri, 30 Sep 2022 11:03:46 +0100 Subject: [PATCH 1/2] Put modern StrictMode behind a feature flag --- .../src/ReactFiberClassComponent.new.js | 51 +++++++- .../src/ReactFiberClassComponent.old.js | 51 +++++++- .../src/ReactFiberCommitWork.new.js | 110 +++++++++++++++++- .../src/ReactFiberCommitWork.old.js | 110 +++++++++++++++++- .../react-reconciler/src/ReactFiberFlags.js | 54 ++++----- .../src/ReactFiberHooks.new.js | 70 +++++++++-- .../src/ReactFiberHooks.old.js | 70 +++++++++-- .../src/ReactFiberWorkLoop.new.js | 93 ++++++++++++--- .../src/ReactFiberWorkLoop.old.js | 93 ++++++++++++--- .../ReactOffscreenStrictMode-test.js | 2 +- ...StrictEffectsModeDefaults-test.internal.js | 1 + .../src/__tests__/ReactStrictMode-test.js | 2 +- packages/shared/ReactFeatureFlags.js | 5 + .../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 + .../shared/forks/ReactFeatureFlags.www.js | 1 + scripts/jest/TestFlags.js | 1 + 22 files changed, 638 insertions(+), 90 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.new.js b/packages/react-reconciler/src/ReactFiberClassComponent.new.js index 37aef0a84f743..398ffc28f4d33 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.new.js @@ -13,7 +13,12 @@ import type {UpdateQueue} from './ReactFiberClassUpdateQueue.new'; import type {Flags} from './ReactFiberFlags'; import * as React from 'react'; -import {LayoutStatic, Update, Snapshot} from './ReactFiberFlags'; +import { + LayoutStatic, + Update, + Snapshot, + MountLayoutDev, +} from './ReactFiberFlags'; import { debugRenderPhaseSideEffectsForStrictMode, disableLegacyContext, @@ -21,6 +26,7 @@ import { enableSchedulingProfiler, warnAboutDeprecatedLifecycles, enableLazyContextPropagation, + enableStrictEffects, } from 'shared/ReactFeatureFlags'; import ReactStrictModeWarnings from './ReactStrictModeWarnings.new'; import {isMounted} from './ReactFiberTreeReflection'; @@ -33,7 +39,12 @@ import isArray from 'shared/isArray'; import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols'; import {resolveDefaultProps} from './ReactFiberLazyComponent.new'; -import {DebugTracingMode, StrictLegacyMode} from './ReactTypeOfMode'; +import { + DebugTracingMode, + NoMode, + StrictLegacyMode, + StrictEffectsMode, +} from './ReactTypeOfMode'; import { enqueueUpdate, @@ -896,7 +907,14 @@ function mountClassInstance( } if (typeof instance.componentDidMount === 'function') { - const fiberFlags: Flags = Update | LayoutStatic; + let fiberFlags: Flags = Update | LayoutStatic; + if ( + __DEV__ && + enableStrictEffects && + (workInProgress.mode & StrictEffectsMode) !== NoMode + ) { + fiberFlags |= MountLayoutDev; + } workInProgress.flags |= fiberFlags; } } @@ -967,7 +985,14 @@ 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') { - const fiberFlags: Flags = Update | LayoutStatic; + let fiberFlags: Flags = Update | LayoutStatic; + if ( + __DEV__ && + enableStrictEffects && + (workInProgress.mode & StrictEffectsMode) !== NoMode + ) { + fiberFlags |= MountLayoutDev; + } workInProgress.flags |= fiberFlags; } return false; @@ -1011,14 +1036,28 @@ function resumeMountClassInstance( } } if (typeof instance.componentDidMount === 'function') { - const fiberFlags: Flags = Update | LayoutStatic; + let fiberFlags: Flags = Update | LayoutStatic; + if ( + __DEV__ && + enableStrictEffects && + (workInProgress.mode & StrictEffectsMode) !== NoMode + ) { + fiberFlags |= MountLayoutDev; + } workInProgress.flags |= fiberFlags; } } 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') { - const fiberFlags: Flags = Update | LayoutStatic; + let fiberFlags: Flags = Update | LayoutStatic; + if ( + __DEV__ && + enableStrictEffects && + (workInProgress.mode & StrictEffectsMode) !== NoMode + ) { + fiberFlags |= MountLayoutDev; + } workInProgress.flags |= fiberFlags; } diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.old.js b/packages/react-reconciler/src/ReactFiberClassComponent.old.js index 3211717d886f1..a7cd936b1fe6d 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.old.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.old.js @@ -13,7 +13,12 @@ import type {UpdateQueue} from './ReactFiberClassUpdateQueue.old'; import type {Flags} from './ReactFiberFlags'; import * as React from 'react'; -import {LayoutStatic, Update, Snapshot} from './ReactFiberFlags'; +import { + LayoutStatic, + Update, + Snapshot, + MountLayoutDev, +} from './ReactFiberFlags'; import { debugRenderPhaseSideEffectsForStrictMode, disableLegacyContext, @@ -21,6 +26,7 @@ import { enableSchedulingProfiler, warnAboutDeprecatedLifecycles, enableLazyContextPropagation, + enableStrictEffects, } from 'shared/ReactFeatureFlags'; import ReactStrictModeWarnings from './ReactStrictModeWarnings.old'; import {isMounted} from './ReactFiberTreeReflection'; @@ -33,7 +39,12 @@ import isArray from 'shared/isArray'; import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols'; import {resolveDefaultProps} from './ReactFiberLazyComponent.old'; -import {DebugTracingMode, StrictLegacyMode} from './ReactTypeOfMode'; +import { + DebugTracingMode, + NoMode, + StrictLegacyMode, + StrictEffectsMode, +} from './ReactTypeOfMode'; import { enqueueUpdate, @@ -896,7 +907,14 @@ function mountClassInstance( } if (typeof instance.componentDidMount === 'function') { - const fiberFlags: Flags = Update | LayoutStatic; + let fiberFlags: Flags = Update | LayoutStatic; + if ( + __DEV__ && + enableStrictEffects && + (workInProgress.mode & StrictEffectsMode) !== NoMode + ) { + fiberFlags |= MountLayoutDev; + } workInProgress.flags |= fiberFlags; } } @@ -967,7 +985,14 @@ 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') { - const fiberFlags: Flags = Update | LayoutStatic; + let fiberFlags: Flags = Update | LayoutStatic; + if ( + __DEV__ && + enableStrictEffects && + (workInProgress.mode & StrictEffectsMode) !== NoMode + ) { + fiberFlags |= MountLayoutDev; + } workInProgress.flags |= fiberFlags; } return false; @@ -1011,14 +1036,28 @@ function resumeMountClassInstance( } } if (typeof instance.componentDidMount === 'function') { - const fiberFlags: Flags = Update | LayoutStatic; + let fiberFlags: Flags = Update | LayoutStatic; + if ( + __DEV__ && + enableStrictEffects && + (workInProgress.mode & StrictEffectsMode) !== NoMode + ) { + fiberFlags |= MountLayoutDev; + } workInProgress.flags |= fiberFlags; } } 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') { - const fiberFlags: Flags = Update | LayoutStatic; + let fiberFlags: Flags = Update | LayoutStatic; + if ( + __DEV__ && + enableStrictEffects && + (workInProgress.mode & StrictEffectsMode) !== NoMode + ) { + fiberFlags |= MountLayoutDev; + } workInProgress.flags |= fiberFlags; } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index bace5e2a34fbb..dfac5d034f375 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -50,6 +50,7 @@ import { enableCache, enableTransitionTracing, enableUseEventHook, + enableStrictEffects, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -4109,4 +4110,111 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber( } } -export {commitPlacement, commitAttachRef, commitDetachRef}; +function invokeLayoutEffectMountInDEV(fiber: Fiber): void { + if (__DEV__ && enableStrictEffects) { + // We don't need to re-check StrictEffectsMode here. + // This function is only called if that check has already passed. + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + try { + commitHookEffectListMount(HookLayout | HookHasEffect, fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } + break; + } + case ClassComponent: { + const instance = fiber.stateNode; + try { + instance.componentDidMount(); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } + break; + } + } + } +} + +function invokePassiveEffectMountInDEV(fiber: Fiber): void { + if (__DEV__ && enableStrictEffects) { + // We don't need to re-check StrictEffectsMode here. + // This function is only called if that check has already passed. + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + try { + commitHookEffectListMount(HookPassive | HookHasEffect, fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } + break; + } + } + } +} + +function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { + if (__DEV__ && enableStrictEffects) { + // We don't need to re-check StrictEffectsMode here. + // This function is only called if that check has already passed. + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + try { + commitHookEffectListUnmount( + HookLayout | HookHasEffect, + fiber, + fiber.return, + ); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } + break; + } + case ClassComponent: { + const instance = fiber.stateNode; + if (typeof instance.componentWillUnmount === 'function') { + safelyCallComponentWillUnmount(fiber, fiber.return, instance); + } + break; + } + } + } +} + +function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { + if (__DEV__ && enableStrictEffects) { + // We don't need to re-check StrictEffectsMode here. + // This function is only called if that check has already passed. + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + try { + commitHookEffectListUnmount( + HookPassive | HookHasEffect, + fiber, + fiber.return, + ); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } + } + } + } +} + +export { + commitPlacement, + commitAttachRef, + commitDetachRef, + invokeLayoutEffectMountInDEV, + invokeLayoutEffectUnmountInDEV, + invokePassiveEffectMountInDEV, + invokePassiveEffectUnmountInDEV, +}; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 1e14b8ac4161f..f75a7edda07bd 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -50,6 +50,7 @@ import { enableCache, enableTransitionTracing, enableUseEventHook, + enableStrictEffects, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -4109,4 +4110,111 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber( } } -export {commitPlacement, commitAttachRef, commitDetachRef}; +function invokeLayoutEffectMountInDEV(fiber: Fiber): void { + if (__DEV__ && enableStrictEffects) { + // We don't need to re-check StrictEffectsMode here. + // This function is only called if that check has already passed. + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + try { + commitHookEffectListMount(HookLayout | HookHasEffect, fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } + break; + } + case ClassComponent: { + const instance = fiber.stateNode; + try { + instance.componentDidMount(); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } + break; + } + } + } +} + +function invokePassiveEffectMountInDEV(fiber: Fiber): void { + if (__DEV__ && enableStrictEffects) { + // We don't need to re-check StrictEffectsMode here. + // This function is only called if that check has already passed. + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + try { + commitHookEffectListMount(HookPassive | HookHasEffect, fiber); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } + break; + } + } + } +} + +function invokeLayoutEffectUnmountInDEV(fiber: Fiber): void { + if (__DEV__ && enableStrictEffects) { + // We don't need to re-check StrictEffectsMode here. + // This function is only called if that check has already passed. + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + try { + commitHookEffectListUnmount( + HookLayout | HookHasEffect, + fiber, + fiber.return, + ); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } + break; + } + case ClassComponent: { + const instance = fiber.stateNode; + if (typeof instance.componentWillUnmount === 'function') { + safelyCallComponentWillUnmount(fiber, fiber.return, instance); + } + break; + } + } + } +} + +function invokePassiveEffectUnmountInDEV(fiber: Fiber): void { + if (__DEV__ && enableStrictEffects) { + // We don't need to re-check StrictEffectsMode here. + // This function is only called if that check has already passed. + switch (fiber.tag) { + case FunctionComponent: + case ForwardRef: + case SimpleMemoComponent: { + try { + commitHookEffectListUnmount( + HookPassive | HookHasEffect, + fiber, + fiber.return, + ); + } catch (error) { + captureCommitPhaseError(fiber, fiber.return, error); + } + } + } + } +} + +export { + commitPlacement, + commitAttachRef, + commitDetachRef, + invokeLayoutEffectMountInDEV, + invokeLayoutEffectUnmountInDEV, + invokePassiveEffectMountInDEV, + invokePassiveEffectUnmountInDEV, +}; diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index 08c124b5cf6e2..cbc304b5ba7d8 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -12,49 +12,51 @@ import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags'; export type Flags = number; // Don't change these two values. They're used by React Dev Tools. -export const NoFlags = /* */ 0b000000000000000000000000; -export const PerformedWork = /* */ 0b000000000000000000000001; +export const NoFlags = /* */ 0b00000000000000000000000000; +export const PerformedWork = /* */ 0b00000000000000000000000001; // You can change the rest (and add more). -export const Placement = /* */ 0b000000000000000000000010; -export const Update = /* */ 0b000000000000000000000100; -export const ChildDeletion = /* */ 0b000000000000000000001000; -export const ContentReset = /* */ 0b000000000000000000010000; -export const Callback = /* */ 0b000000000000000000100000; -export const DidCapture = /* */ 0b000000000000000001000000; -export const ForceClientRender = /* */ 0b000000000000000010000000; -export const Ref = /* */ 0b000000000000000100000000; -export const Snapshot = /* */ 0b000000000000001000000000; -export const Passive = /* */ 0b000000000000010000000000; -export const Hydrating = /* */ 0b000000000000100000000000; -export const Visibility = /* */ 0b000000000001000000000000; -export const StoreConsistency = /* */ 0b000000000010000000000000; +export const Placement = /* */ 0b00000000000000000000000010; +export const Update = /* */ 0b00000000000000000000000100; +export const ChildDeletion = /* */ 0b00000000000000000000001000; +export const ContentReset = /* */ 0b00000000000000000000010000; +export const Callback = /* */ 0b00000000000000000000100000; +export const DidCapture = /* */ 0b00000000000000000001000000; +export const ForceClientRender = /* */ 0b00000000000000000010000000; +export const Ref = /* */ 0b00000000000000000100000000; +export const Snapshot = /* */ 0b00000000000000001000000000; +export const Passive = /* */ 0b00000000000000010000000000; +export const Hydrating = /* */ 0b00000000000000100000000000; +export const Visibility = /* */ 0b00000000000001000000000000; +export const StoreConsistency = /* */ 0b00000000000010000000000000; export const LifecycleEffectMask = Passive | Update | Callback | Ref | Snapshot | StoreConsistency; // Union of all commit flags (flags with the lifetime of a particular commit) -export const HostEffectMask = /* */ 0b000000000011111111111111; +export const HostEffectMask = /* */ 0b00000000000011111111111111; // These are not really side effects, but we still reuse this field. -export const Incomplete = /* */ 0b000000000100000000000000; -export const ShouldCapture = /* */ 0b000000001000000000000000; -export const ForceUpdateForLegacySuspense = /* */ 0b000000010000000000000000; -export const DidPropagateContext = /* */ 0b000000100000000000000000; -export const NeedsPropagation = /* */ 0b000001000000000000000000; -export const Forked = /* */ 0b000010000000000000000000; +export const Incomplete = /* */ 0b00000000000100000000000000; +export const ShouldCapture = /* */ 0b00000000001000000000000000; +export const ForceUpdateForLegacySuspense = /* */ 0b00000000010000000000000000; +export const DidPropagateContext = /* */ 0b00000000100000000000000000; +export const NeedsPropagation = /* */ 0b00000001000000000000000000; +export const Forked = /* */ 0b00000010000000000000000000; // 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 RefStatic = /* */ 0b000100000000000000000000; -export const LayoutStatic = /* */ 0b001000000000000000000000; -export const PassiveStatic = /* */ 0b010000000000000000000000; +export const RefStatic = /* */ 0b00000100000000000000000000; +export const LayoutStatic = /* */ 0b00001000000000000000000000; +export const PassiveStatic = /* */ 0b00010000000000000000000000; // Flag used to identify newly inserted fibers. It isn't reset after commit unlike `Placement`. -export const PlacementDEV = /* */ 0b100000000000000000000000; +export const PlacementDEV = /* */ 0b00100000000000000000000000; +export const MountLayoutDev = /* */ 0b01000000000000000000000000; +export const MountPassiveDev = /* */ 0b10000000000000000000000000; // Groups of flags that are used in the commit phase to skip over trees that // don't contain effects, by checking subtreeFlags. diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index f5956e27255fc..a1c8fb605fb5c 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -42,13 +42,19 @@ import { enableUseHook, enableUseMemoCacheHook, enableUseEventHook, + enableStrictEffects, } from 'shared/ReactFeatureFlags'; import { REACT_CONTEXT_TYPE, REACT_SERVER_CONTEXT_TYPE, } from 'shared/ReactSymbols'; -import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode'; +import { + NoMode, + ConcurrentMode, + DebugTracingMode, + StrictEffectsMode, +} from './ReactTypeOfMode'; import { NoLane, SyncLane, @@ -81,6 +87,8 @@ import { StaticMask as StaticMaskEffect, Update as UpdateEffect, StoreConsistency, + MountLayoutDev as MountLayoutDevEffect, + MountPassiveDev as MountPassiveDevEffect, } from './ReactFiberFlags'; import { HasEffect as HookHasEffect, @@ -590,7 +598,22 @@ export function bailoutHooks( lanes: Lanes, ) { workInProgress.updateQueue = current.updateQueue; - workInProgress.flags &= ~(PassiveEffect | UpdateEffect); + // TODO: Don't need to reset the flags here, because they're reset in the + // complete phase (bubbleProperties). + if ( + __DEV__ && + enableStrictEffects && + (workInProgress.mode & StrictEffectsMode) !== NoMode + ) { + workInProgress.flags &= ~( + MountPassiveDevEffect | + MountLayoutDevEffect | + PassiveEffect | + UpdateEffect + ); + } else { + workInProgress.flags &= ~(PassiveEffect | UpdateEffect); + } current.lanes = removeLanes(current.lanes, lanes); } @@ -1859,12 +1882,25 @@ function mountEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { - return mountEffectImpl( - PassiveEffect | PassiveStaticEffect, - HookPassive, - create, - deps, - ); + if ( + __DEV__ && + enableStrictEffects && + (currentlyRenderingFiber.mode & StrictEffectsMode) !== NoMode + ) { + return mountEffectImpl( + MountPassiveDevEffect | PassiveEffect | PassiveStaticEffect, + HookPassive, + create, + deps, + ); + } else { + return mountEffectImpl( + PassiveEffect | PassiveStaticEffect, + HookPassive, + create, + deps, + ); + } } function updateEffect( @@ -1940,7 +1976,14 @@ function mountLayoutEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { - const fiberFlags: Flags = UpdateEffect | LayoutStaticEffect; + let fiberFlags: Flags = UpdateEffect | LayoutStaticEffect; + if ( + __DEV__ && + enableStrictEffects && + (currentlyRenderingFiber.mode & StrictEffectsMode) !== NoMode + ) { + fiberFlags |= MountLayoutDevEffect; + } return mountEffectImpl(fiberFlags, HookLayout, create, deps); } @@ -2000,7 +2043,14 @@ function mountImperativeHandle( const effectDeps = deps !== null && deps !== undefined ? deps.concat([ref]) : null; - const fiberFlags: Flags = UpdateEffect | LayoutStaticEffect; + let fiberFlags: Flags = UpdateEffect | LayoutStaticEffect; + if ( + __DEV__ && + enableStrictEffects && + (currentlyRenderingFiber.mode & StrictEffectsMode) !== NoMode + ) { + fiberFlags |= MountLayoutDevEffect; + } return mountEffectImpl( fiberFlags, HookLayout, diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index 88b608694bcac..d536ab25fb96c 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -42,13 +42,19 @@ import { enableUseHook, enableUseMemoCacheHook, enableUseEventHook, + enableStrictEffects, } from 'shared/ReactFeatureFlags'; import { REACT_CONTEXT_TYPE, REACT_SERVER_CONTEXT_TYPE, } from 'shared/ReactSymbols'; -import {NoMode, ConcurrentMode, DebugTracingMode} from './ReactTypeOfMode'; +import { + NoMode, + ConcurrentMode, + DebugTracingMode, + StrictEffectsMode, +} from './ReactTypeOfMode'; import { NoLane, SyncLane, @@ -81,6 +87,8 @@ import { StaticMask as StaticMaskEffect, Update as UpdateEffect, StoreConsistency, + MountLayoutDev as MountLayoutDevEffect, + MountPassiveDev as MountPassiveDevEffect, } from './ReactFiberFlags'; import { HasEffect as HookHasEffect, @@ -590,7 +598,22 @@ export function bailoutHooks( lanes: Lanes, ) { workInProgress.updateQueue = current.updateQueue; - workInProgress.flags &= ~(PassiveEffect | UpdateEffect); + // TODO: Don't need to reset the flags here, because they're reset in the + // complete phase (bubbleProperties). + if ( + __DEV__ && + enableStrictEffects && + (workInProgress.mode & StrictEffectsMode) !== NoMode + ) { + workInProgress.flags &= ~( + MountPassiveDevEffect | + MountLayoutDevEffect | + PassiveEffect | + UpdateEffect + ); + } else { + workInProgress.flags &= ~(PassiveEffect | UpdateEffect); + } current.lanes = removeLanes(current.lanes, lanes); } @@ -1859,12 +1882,25 @@ function mountEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { - return mountEffectImpl( - PassiveEffect | PassiveStaticEffect, - HookPassive, - create, - deps, - ); + if ( + __DEV__ && + enableStrictEffects && + (currentlyRenderingFiber.mode & StrictEffectsMode) !== NoMode + ) { + return mountEffectImpl( + MountPassiveDevEffect | PassiveEffect | PassiveStaticEffect, + HookPassive, + create, + deps, + ); + } else { + return mountEffectImpl( + PassiveEffect | PassiveStaticEffect, + HookPassive, + create, + deps, + ); + } } function updateEffect( @@ -1940,7 +1976,14 @@ function mountLayoutEffect( create: () => (() => void) | void, deps: Array | void | null, ): void { - const fiberFlags: Flags = UpdateEffect | LayoutStaticEffect; + let fiberFlags: Flags = UpdateEffect | LayoutStaticEffect; + if ( + __DEV__ && + enableStrictEffects && + (currentlyRenderingFiber.mode & StrictEffectsMode) !== NoMode + ) { + fiberFlags |= MountLayoutDevEffect; + } return mountEffectImpl(fiberFlags, HookLayout, create, deps); } @@ -2000,7 +2043,14 @@ function mountImperativeHandle( const effectDeps = deps !== null && deps !== undefined ? deps.concat([ref]) : null; - const fiberFlags: Flags = UpdateEffect | LayoutStaticEffect; + let fiberFlags: Flags = UpdateEffect | LayoutStaticEffect; + if ( + __DEV__ && + enableStrictEffects && + (currentlyRenderingFiber.mode & StrictEffectsMode) !== NoMode + ) { + fiberFlags |= MountLayoutDevEffect; + } return mountEffectImpl( fiberFlags, HookLayout, diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 95381d2558428..8f9e856772d5e 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -40,6 +40,7 @@ import { enableUpdaterTracking, enableCache, enableTransitionTracing, + useModernStrictMode, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import is from 'shared/objectIs'; @@ -113,6 +114,7 @@ import { Profiler, } from './ReactWorkTags'; import {ConcurrentRoot, LegacyRoot} from './ReactRootTags'; +import type {Flags} from './ReactFiberFlags'; import { NoFlags, Incomplete, @@ -125,6 +127,8 @@ import { PassiveMask, PlacementDEV, Visibility, + MountPassiveDev, + MountLayoutDev, } from './ReactFiberFlags'; import { NoLanes, @@ -189,6 +193,10 @@ import { reappearLayoutEffects, disconnectPassiveEffect, reportUncaughtErrorInDEV, + invokeLayoutEffectMountInDEV, + invokePassiveEffectMountInDEV, + invokeLayoutEffectUnmountInDEV, + invokePassiveEffectUnmountInDEV, } from './ReactFiberCommitWork.new'; import {enqueueUpdate} from './ReactFiberClassUpdateQueue.new'; import {resetContextDependencies} from './ReactFiberNewContext.new'; @@ -2570,7 +2578,7 @@ function commitRootImpl( if (__DEV__ && enableStrictEffects) { if (!rootDidHavePassiveEffects) { - commitDoubleInvokeEffectsInDEV(root); + commitDoubleInvokeEffectsInDEV(root, false); } } @@ -2846,7 +2854,7 @@ function flushPassiveEffectsImpl() { } if (__DEV__ && enableStrictEffects) { - commitDoubleInvokeEffectsInDEV(root); + commitDoubleInvokeEffectsInDEV(root, true); } executionContext = prevExecutionContext; @@ -3309,24 +3317,81 @@ function doubleInvokeEffectsInDEVIfNecessary( } } -function commitDoubleInvokeEffectsInDEV(root: FiberRoot) { +function commitDoubleInvokeEffectsInDEV( + root: FiberRoot, + hasPassiveEffects: boolean, +) { if (__DEV__ && enableStrictEffects) { - let doubleInvokeEffects = true; + if (useModernStrictMode) { + let doubleInvokeEffects = true; - if (root.tag === LegacyRoot && !(root.current.mode & StrictLegacyMode)) { - doubleInvokeEffects = false; + if (root.tag === LegacyRoot && !(root.current.mode & StrictLegacyMode)) { + doubleInvokeEffects = false; + } + if ( + root.tag === ConcurrentRoot && + !(root.current.mode & (StrictLegacyMode | StrictEffectsMode)) + ) { + doubleInvokeEffects = false; + } + recursivelyTraverseAndDoubleInvokeEffectsInDEV( + root, + root.current, + doubleInvokeEffects, + ); + } else { + legacyCommitDoubleInvokeEffectsInDEV(root.current, hasPassiveEffects); } + } +} + +function legacyCommitDoubleInvokeEffectsInDEV( + fiber: Fiber, + hasPassiveEffects: boolean, +) { + // TODO (StrictEffects) Should we set a marker on the root if it contains strict effects + // so we don't traverse unnecessarily? similar to subtreeFlags but just at the root level. + // Maybe not a big deal since this is DEV only behavior. + + 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, +) { + let current = firstChild; + let subtreeRoot = null; + while (current != null) { + const primarySubtreeFlag = current.subtreeFlags & fiberFlags; if ( - root.tag === ConcurrentRoot && - !(root.current.mode & (StrictLegacyMode | StrictEffectsMode)) + current !== subtreeRoot && + current.child != null && + primarySubtreeFlag !== NoFlags ) { - doubleInvokeEffects = false; + current = current.child; + } else { + if ((current.flags & fiberFlags) !== NoFlags) { + invokeEffectFn(current); + } + + if (current.sibling !== null) { + current = current.sibling; + } else { + current = subtreeRoot = current.return; + } } - recursivelyTraverseAndDoubleInvokeEffectsInDEV( - root, - root.current, - doubleInvokeEffects, - ); } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index ac07138054a0c..ef4f355909c36 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -40,6 +40,7 @@ import { enableUpdaterTracking, enableCache, enableTransitionTracing, + useModernStrictMode, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import is from 'shared/objectIs'; @@ -113,6 +114,7 @@ import { Profiler, } from './ReactWorkTags'; import {ConcurrentRoot, LegacyRoot} from './ReactRootTags'; +import type {Flags} from './ReactFiberFlags'; import { NoFlags, Incomplete, @@ -125,6 +127,8 @@ import { PassiveMask, PlacementDEV, Visibility, + MountPassiveDev, + MountLayoutDev, } from './ReactFiberFlags'; import { NoLanes, @@ -189,6 +193,10 @@ import { reappearLayoutEffects, disconnectPassiveEffect, reportUncaughtErrorInDEV, + invokeLayoutEffectMountInDEV, + invokePassiveEffectMountInDEV, + invokeLayoutEffectUnmountInDEV, + invokePassiveEffectUnmountInDEV, } from './ReactFiberCommitWork.old'; import {enqueueUpdate} from './ReactFiberClassUpdateQueue.old'; import {resetContextDependencies} from './ReactFiberNewContext.old'; @@ -2570,7 +2578,7 @@ function commitRootImpl( if (__DEV__ && enableStrictEffects) { if (!rootDidHavePassiveEffects) { - commitDoubleInvokeEffectsInDEV(root); + commitDoubleInvokeEffectsInDEV(root, false); } } @@ -2846,7 +2854,7 @@ function flushPassiveEffectsImpl() { } if (__DEV__ && enableStrictEffects) { - commitDoubleInvokeEffectsInDEV(root); + commitDoubleInvokeEffectsInDEV(root, true); } executionContext = prevExecutionContext; @@ -3309,24 +3317,81 @@ function doubleInvokeEffectsInDEVIfNecessary( } } -function commitDoubleInvokeEffectsInDEV(root: FiberRoot) { +function commitDoubleInvokeEffectsInDEV( + root: FiberRoot, + hasPassiveEffects: boolean, +) { if (__DEV__ && enableStrictEffects) { - let doubleInvokeEffects = true; + if (useModernStrictMode) { + let doubleInvokeEffects = true; - if (root.tag === LegacyRoot && !(root.current.mode & StrictLegacyMode)) { - doubleInvokeEffects = false; + if (root.tag === LegacyRoot && !(root.current.mode & StrictLegacyMode)) { + doubleInvokeEffects = false; + } + if ( + root.tag === ConcurrentRoot && + !(root.current.mode & (StrictLegacyMode | StrictEffectsMode)) + ) { + doubleInvokeEffects = false; + } + recursivelyTraverseAndDoubleInvokeEffectsInDEV( + root, + root.current, + doubleInvokeEffects, + ); + } else { + legacyCommitDoubleInvokeEffectsInDEV(root.current, hasPassiveEffects); } + } +} + +function legacyCommitDoubleInvokeEffectsInDEV( + fiber: Fiber, + hasPassiveEffects: boolean, +) { + // TODO (StrictEffects) Should we set a marker on the root if it contains strict effects + // so we don't traverse unnecessarily? similar to subtreeFlags but just at the root level. + // Maybe not a big deal since this is DEV only behavior. + + 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, +) { + let current = firstChild; + let subtreeRoot = null; + while (current != null) { + const primarySubtreeFlag = current.subtreeFlags & fiberFlags; if ( - root.tag === ConcurrentRoot && - !(root.current.mode & (StrictLegacyMode | StrictEffectsMode)) + current !== subtreeRoot && + current.child != null && + primarySubtreeFlag !== NoFlags ) { - doubleInvokeEffects = false; + current = current.child; + } else { + if ((current.flags & fiberFlags) !== NoFlags) { + invokeEffectFn(current); + } + + if (current.sibling !== null) { + current = current.sibling; + } else { + current = subtreeRoot = current.return; + } } - recursivelyTraverseAndDoubleInvokeEffectsInDEV( - root, - root.current, - doubleInvokeEffects, - ); } } diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js index b398032d66917..0771e596d9111 100644 --- a/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js +++ b/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js @@ -55,7 +55,7 @@ describe('ReactOffscreenStrictMode', () => { ]); }); - // @gate __DEV__ && enableStrictEffects && enableOffscreen + // @gate __DEV__ && enableStrictEffects && enableOffscreen && useModernStrictMode it('should not trigger strict effects when offscreen is hidden', () => { act(() => { ReactNoop.render( diff --git a/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js b/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js index 54150ad2d4be1..3090fcc99138d 100644 --- a/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js +++ b/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js @@ -379,6 +379,7 @@ describe('StrictEffectsMode defaults', () => { expect(Scheduler).toHaveYielded([]); }); + //@gate useModernStrictMode it('disconnects refs during double invoking', () => { const onRefMock = jest.fn(); function App({text}) { diff --git a/packages/react/src/__tests__/ReactStrictMode-test.js b/packages/react/src/__tests__/ReactStrictMode-test.js index 8aebe33426a0a..0f3ba9a9e1cb9 100644 --- a/packages/react/src/__tests__/ReactStrictMode-test.js +++ b/packages/react/src/__tests__/ReactStrictMode-test.js @@ -67,7 +67,7 @@ describe('ReactStrictMode', () => { ); }); - // @gate __DEV__ && !enableStrictEffects + // @gate __DEV__ it('should invoke precommit lifecycle methods twice', () => { let log = []; let shouldComponentUpdate = false; diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index e4863bbfc299d..67a9ac2014d6e 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -260,3 +260,8 @@ export const enableGetInspectorDataForInstanceInProduction = false; export const enableProfilerNestedUpdateScheduledHook = false; export const consoleManagedByDevToolsDuringStrictMode = true; + +// Modern behaviour aligns more with what components +// components will encounter in production, especially when used With . +// TODO: clean up legacy once tests pass WWW. +export const useModernStrictMode = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 500c4bde865fa..821f947415dcd 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -84,6 +84,8 @@ export const enableUseMutableSource = true; export const enableTransitionTracing = false; export const enableFloat = false; + +export const useModernStrictMode = 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 fb889cc65e736..c63c2187005ae 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -73,6 +73,8 @@ export const enableUseMutableSource = false; export const enableTransitionTracing = false; export const enableFloat = false; + +export const useModernStrictMode = 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 53ed7ca5a14c6..665c42cb8f84d 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -73,6 +73,8 @@ export const enableUseMutableSource = false; export const enableTransitionTracing = false; export const enableFloat = false; + +export const useModernStrictMode = 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 034a605979ab1..c48df6576ef6b 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -71,6 +71,8 @@ export const enableUseMutableSource = false; export const enableTransitionTracing = false; export const enableFloat = false; + +export const useModernStrictMode = 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 239f67b9d54ae..d4e135d655a05 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -75,6 +75,8 @@ export const enableUseMutableSource = true; export const enableTransitionTracing = false; export const enableFloat = false; + +export const useModernStrictMode = 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 6b078b93c26f7..f77ed6fa13620 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -73,6 +73,8 @@ export const enableUseMutableSource = false; export const enableTransitionTracing = false; export const enableFloat = false; + +export const useModernStrictMode = 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 b16916483b3fd..1c78538140e1b 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -74,6 +74,8 @@ export const enableUseMutableSource = true; export const enableTransitionTracing = false; export const enableFloat = false; + +export const useModernStrictMode = 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.js b/packages/shared/forks/ReactFeatureFlags.www.js index 605282c150c9b..f8566f9ecb2bd 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -111,6 +111,7 @@ export const enableUseMutableSource = true; export const enableCustomElementPropertySupport = __EXPERIMENTAL__; +export const useModernStrictMode = 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/scripts/jest/TestFlags.js b/scripts/jest/TestFlags.js index ee093653182ac..01e4c6795d8bb 100644 --- a/scripts/jest/TestFlags.js +++ b/scripts/jest/TestFlags.js @@ -88,6 +88,7 @@ function getTestFlags() { enableUseSyncExternalStoreShim: !__VARIANT__, enableSuspenseList: releaseChannel === 'experimental' || www, enableOffscreen: releaseChannel === 'experimental' || www, + useModernStrictMode: releaseChannel === 'experimental', enableLegacyHidden: www, // If there's a naming conflict between scheduler and React feature flags, the From 201f893db048ad1a1ddbfeed84df2f718e7033fb Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Fri, 30 Sep 2022 16:27:24 +0100 Subject: [PATCH 2/2] Remove unneeded flag --- scripts/jest/TestFlags.js | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/jest/TestFlags.js b/scripts/jest/TestFlags.js index 01e4c6795d8bb..ee093653182ac 100644 --- a/scripts/jest/TestFlags.js +++ b/scripts/jest/TestFlags.js @@ -88,7 +88,6 @@ function getTestFlags() { enableUseSyncExternalStoreShim: !__VARIANT__, enableSuspenseList: releaseChannel === 'experimental' || www, enableOffscreen: releaseChannel === 'experimental' || www, - useModernStrictMode: releaseChannel === 'experimental', enableLegacyHidden: www, // If there's a naming conflict between scheduler and React feature flags, the