From e19bec55d4f70a0f251c3503e36dbc4cf7e7141b Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Tue, 23 Aug 2022 18:19:07 +0100 Subject: [PATCH] Align StrictMode behaviour with production (#25049) * Skip double invoking effects in Offscreen * Run yarn replace-fork * Use executionContext to disable profiler timer * Restructure recursion into two functions * Fix ReactStrictMode test * Use gate pragma in ReacetOffscreenStrictMode test * Set and reset current debug fiber in dev * Skip over paths that don't include any insertions * Extract common logic to check for profiling to a helper function * Remove hasPassiveEffects flag from StrictMode * Fix flow issues * Revert "Skip over paths that don't include any insertions" --- .../src/ReactChildFiber.new.js | 13 +- .../src/ReactChildFiber.old.js | 13 +- .../src/ReactFiberCommitWork.new.js | 211 +++--------------- .../src/ReactFiberCommitWork.old.js | 211 +++--------------- .../react-reconciler/src/ReactFiberFlags.js | 55 ++--- .../src/ReactFiberWorkLoop.new.js | 117 +++++----- .../src/ReactFiberWorkLoop.old.js | 117 +++++----- .../ReactOffscreenStrictMode-test.js | 95 ++++++++ ...StrictEffectsModeDefaults-test.internal.js | 27 ++- .../src/__tests__/ReactStrictMode-test.js | 74 +++--- 10 files changed, 376 insertions(+), 557 deletions(-) create mode 100644 packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js diff --git a/packages/react-reconciler/src/ReactChildFiber.new.js b/packages/react-reconciler/src/ReactChildFiber.new.js index e7eb67328af09..71971c03892ca 100644 --- a/packages/react-reconciler/src/ReactChildFiber.new.js +++ b/packages/react-reconciler/src/ReactChildFiber.new.js @@ -13,7 +13,12 @@ import type {Fiber} from './ReactInternalTypes'; import type {Lanes} from './ReactFiberLane.new'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; -import {Placement, ChildDeletion, Forked} from './ReactFiberFlags'; +import { + Placement, + ChildDeletion, + Forked, + PlacementDEV, +} from './ReactFiberFlags'; import { getIteratorFn, REACT_ELEMENT_TYPE, @@ -343,7 +348,7 @@ function ChildReconciler(shouldTrackSideEffects) { const oldIndex = current.index; if (oldIndex < lastPlacedIndex) { // This is a move. - newFiber.flags |= Placement; + newFiber.flags |= Placement | PlacementDEV; return lastPlacedIndex; } else { // This item can stay in place. @@ -351,7 +356,7 @@ function ChildReconciler(shouldTrackSideEffects) { } } else { // This is an insertion. - newFiber.flags |= Placement; + newFiber.flags |= Placement | PlacementDEV; return lastPlacedIndex; } } @@ -360,7 +365,7 @@ function ChildReconciler(shouldTrackSideEffects) { // This is simpler for the single child case. We only need to do a // placement for inserting new children. if (shouldTrackSideEffects && newFiber.alternate === null) { - newFiber.flags |= Placement; + newFiber.flags |= Placement | PlacementDEV; } return newFiber; } diff --git a/packages/react-reconciler/src/ReactChildFiber.old.js b/packages/react-reconciler/src/ReactChildFiber.old.js index 320924d6030ec..86f75fbf65ea8 100644 --- a/packages/react-reconciler/src/ReactChildFiber.old.js +++ b/packages/react-reconciler/src/ReactChildFiber.old.js @@ -13,7 +13,12 @@ import type {Fiber} from './ReactInternalTypes'; import type {Lanes} from './ReactFiberLane.old'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; -import {Placement, ChildDeletion, Forked} from './ReactFiberFlags'; +import { + Placement, + ChildDeletion, + Forked, + PlacementDEV, +} from './ReactFiberFlags'; import { getIteratorFn, REACT_ELEMENT_TYPE, @@ -343,7 +348,7 @@ function ChildReconciler(shouldTrackSideEffects) { const oldIndex = current.index; if (oldIndex < lastPlacedIndex) { // This is a move. - newFiber.flags |= Placement; + newFiber.flags |= Placement | PlacementDEV; return lastPlacedIndex; } else { // This item can stay in place. @@ -351,7 +356,7 @@ function ChildReconciler(shouldTrackSideEffects) { } } else { // This is an insertion. - newFiber.flags |= Placement; + newFiber.flags |= Placement | PlacementDEV; return lastPlacedIndex; } } @@ -360,7 +365,7 @@ function ChildReconciler(shouldTrackSideEffects) { // This is simpler for the single child case. We only need to do a // placement for inserting new children. if (shouldTrackSideEffects && newFiber.alternate === null) { - newFiber.flags |= Placement; + newFiber.flags |= Placement | PlacementDEV; } return newFiber; } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index eb696d06eb4af..5d944a4d88d49 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -40,7 +40,6 @@ import { enableSchedulingProfiler, enableSuspenseCallback, enableScopeAPI, - enableStrictEffects, deletedTreeCleanUpLevel, enableUpdaterTracking, enableCache, @@ -148,6 +147,9 @@ import { addMarkerProgressCallbackToPendingTransition, addMarkerCompleteCallbackToPendingTransition, setIsRunningInsertionEffect, + getExecutionContext, + CommitContext, + NoContext, } from './ReactFiberWorkLoop.new'; import { NoFlags as NoHookEffect, @@ -200,6 +202,15 @@ let nextEffect: Fiber | null = null; let inProgressLanes: Lanes | null = null; let inProgressRoot: FiberRoot | null = null; +function shouldProfile(current: Fiber): boolean { + return ( + enableProfilerTimer && + enableProfilerCommitHooks && + (current.mode & ProfileMode) !== NoMode && + (getExecutionContext() & CommitContext) !== NoContext + ); +} + export function reportUncaughtErrorInDEV(error: mixed) { // Wrapping each small part of the commit phase into a guarded // callback is a bit too slow (https://github.com/facebook/react/pull/21666). @@ -217,11 +228,7 @@ export function reportUncaughtErrorInDEV(error: mixed) { const callComponentWillUnmountWithTimer = function(current, instance) { instance.props = current.memoizedProps; instance.state = current.memoizedState; - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - current.mode & ProfileMode - ) { + if (shouldProfile(current)) { try { startLayoutEffectTimer(); instance.componentWillUnmount(); @@ -261,11 +268,7 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { if (typeof ref === 'function') { let retVal; try { - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - current.mode & ProfileMode - ) { + if (shouldProfile(current)) { try { startLayoutEffectTimer(); retVal = ref(null); @@ -641,7 +644,11 @@ export function commitPassiveEffectDurations( finishedRoot: FiberRoot, finishedWork: Fiber, ): void { - if (enableProfilerTimer && enableProfilerCommitHooks) { + if ( + enableProfilerTimer && + enableProfilerCommitHooks && + getExecutionContext() & CommitContext + ) { // Only Profilers with work in their subtree will have an Update effect scheduled. if ((finishedWork.flags & Update) !== NoFlags) { switch (finishedWork.tag) { @@ -694,11 +701,7 @@ function commitHookLayoutEffects(finishedWork: Fiber, hookFlags: HookFlags) { // This is done to prevent sibling component effects from interfering with each other, // e.g. a destroy function in one component should never override a ref set // by a create function in another component during the same commit. - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { try { startLayoutEffectTimer(); commitHookEffectListMount(hookFlags, finishedWork); @@ -751,11 +754,7 @@ function commitClassLayoutLifecycles( } } } - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { try { startLayoutEffectTimer(); instance.componentDidMount(); @@ -806,11 +805,7 @@ function commitClassLayoutLifecycles( } } } - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { try { startLayoutEffectTimer(); instance.componentDidUpdate( @@ -892,7 +887,7 @@ function commitHostComponentMount(finishedWork: Fiber) { } function commitProfilerUpdate(finishedWork: Fiber, current: Fiber | null) { - if (enableProfilerTimer) { + if (enableProfilerTimer && getExecutionContext() & CommitContext) { try { const {onCommit, onRender} = finishedWork.memoizedProps; const {effectDuration} = finishedWork.stateNode; @@ -1341,11 +1336,7 @@ function commitAttachRef(finishedWork: Fiber) { } if (typeof ref === 'function') { let retVal; - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { try { startLayoutEffectTimer(); retVal = ref(instanceToUse); @@ -1384,11 +1375,7 @@ function commitDetachRef(current: Fiber) { const currentRef = current.ref; if (currentRef !== null) { if (typeof currentRef === 'function') { - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - current.mode & ProfileMode - ) { + if (shouldProfile(current)) { try { startLayoutEffectTimer(); currentRef(null); @@ -1914,11 +1901,7 @@ function commitDeletionEffectsOnFiber( markComponentLayoutEffectUnmountStarted(deletedFiber); } - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - deletedFiber.mode & ProfileMode - ) { + if (shouldProfile(deletedFiber)) { startLayoutEffectTimer(); safelyCallDestroy( deletedFiber, @@ -2237,11 +2220,7 @@ function commitMutationEffectsOnFiber( // This prevents sibling component effects from interfering with each other, // e.g. a destroy function in one component should never override a ref set // by a create function in another component during the same commit. - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { try { startLayoutEffectTimer(); commitHookEffectListUnmount( @@ -2627,18 +2606,14 @@ function recursivelyTraverseLayoutEffects( setCurrentDebugFiberInDEV(prevDebugFiber); } -function disappearLayoutEffects(finishedWork: Fiber) { +export function disappearLayoutEffects(finishedWork: Fiber) { switch (finishedWork.tag) { case FunctionComponent: case ForwardRef: case MemoComponent: case SimpleMemoComponent: { // TODO (Offscreen) Check: flags & LayoutStatic - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { try { startLayoutEffectTimer(); commitHookEffectListUnmount( @@ -2709,7 +2684,7 @@ function recursivelyTraverseDisappearLayoutEffects(parentFiber: Fiber) { } } -function reappearLayoutEffects( +export function reappearLayoutEffects( finishedRoot: FiberRoot, current: Fiber | null, finishedWork: Fiber, @@ -2876,11 +2851,7 @@ function commitHookPassiveMountEffects( finishedWork: Fiber, hookFlags: HookFlags, ) { - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { startPassiveEffectTimer(); try { commitHookEffectListMount(hookFlags, finishedWork); @@ -3304,7 +3275,7 @@ function recursivelyTraverseReconnectPassiveEffects( setCurrentDebugFiberInDEV(prevDebugFiber); } -function reconnectPassiveEffects( +export function reconnectPassiveEffects( finishedRoot: FiberRoot, finishedWork: Fiber, committedLanes: Lanes, @@ -3585,11 +3556,7 @@ function commitHookPassiveUnmountEffects( nearestMountedAncestor, hookFlags: HookFlags, ) { - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { startPassiveEffectTimer(); commitHookEffectListUnmount( hookFlags, @@ -3718,7 +3685,7 @@ function recursivelyTraverseDisconnectPassiveEffects(parentFiber: Fiber): void { setCurrentDebugFiberInDEV(prevDebugFiber); } -function disconnectPassiveEffect(finishedWork: Fiber): void { +export function disconnectPassiveEffect(finishedWork: Fiber): void { switch (finishedWork.tag) { case FunctionComponent: case ForwardRef: @@ -3870,112 +3837,4 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber( } } -// TODO: Reuse reappearLayoutEffects traversal here? -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, -}; +export {commitPlacement, commitAttachRef, commitDetachRef}; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 6c17402e08460..8afa97a5ab7b6 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -40,7 +40,6 @@ import { enableSchedulingProfiler, enableSuspenseCallback, enableScopeAPI, - enableStrictEffects, deletedTreeCleanUpLevel, enableUpdaterTracking, enableCache, @@ -148,6 +147,9 @@ import { addMarkerProgressCallbackToPendingTransition, addMarkerCompleteCallbackToPendingTransition, setIsRunningInsertionEffect, + getExecutionContext, + CommitContext, + NoContext, } from './ReactFiberWorkLoop.old'; import { NoFlags as NoHookEffect, @@ -200,6 +202,15 @@ let nextEffect: Fiber | null = null; let inProgressLanes: Lanes | null = null; let inProgressRoot: FiberRoot | null = null; +function shouldProfile(current: Fiber): boolean { + return ( + enableProfilerTimer && + enableProfilerCommitHooks && + (current.mode & ProfileMode) !== NoMode && + (getExecutionContext() & CommitContext) !== NoContext + ); +} + export function reportUncaughtErrorInDEV(error: mixed) { // Wrapping each small part of the commit phase into a guarded // callback is a bit too slow (https://github.com/facebook/react/pull/21666). @@ -217,11 +228,7 @@ export function reportUncaughtErrorInDEV(error: mixed) { const callComponentWillUnmountWithTimer = function(current, instance) { instance.props = current.memoizedProps; instance.state = current.memoizedState; - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - current.mode & ProfileMode - ) { + if (shouldProfile(current)) { try { startLayoutEffectTimer(); instance.componentWillUnmount(); @@ -261,11 +268,7 @@ function safelyDetachRef(current: Fiber, nearestMountedAncestor: Fiber | null) { if (typeof ref === 'function') { let retVal; try { - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - current.mode & ProfileMode - ) { + if (shouldProfile(current)) { try { startLayoutEffectTimer(); retVal = ref(null); @@ -641,7 +644,11 @@ export function commitPassiveEffectDurations( finishedRoot: FiberRoot, finishedWork: Fiber, ): void { - if (enableProfilerTimer && enableProfilerCommitHooks) { + if ( + enableProfilerTimer && + enableProfilerCommitHooks && + getExecutionContext() & CommitContext + ) { // Only Profilers with work in their subtree will have an Update effect scheduled. if ((finishedWork.flags & Update) !== NoFlags) { switch (finishedWork.tag) { @@ -694,11 +701,7 @@ function commitHookLayoutEffects(finishedWork: Fiber, hookFlags: HookFlags) { // This is done to prevent sibling component effects from interfering with each other, // e.g. a destroy function in one component should never override a ref set // by a create function in another component during the same commit. - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { try { startLayoutEffectTimer(); commitHookEffectListMount(hookFlags, finishedWork); @@ -751,11 +754,7 @@ function commitClassLayoutLifecycles( } } } - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { try { startLayoutEffectTimer(); instance.componentDidMount(); @@ -806,11 +805,7 @@ function commitClassLayoutLifecycles( } } } - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { try { startLayoutEffectTimer(); instance.componentDidUpdate( @@ -892,7 +887,7 @@ function commitHostComponentMount(finishedWork: Fiber) { } function commitProfilerUpdate(finishedWork: Fiber, current: Fiber | null) { - if (enableProfilerTimer) { + if (enableProfilerTimer && getExecutionContext() & CommitContext) { try { const {onCommit, onRender} = finishedWork.memoizedProps; const {effectDuration} = finishedWork.stateNode; @@ -1341,11 +1336,7 @@ function commitAttachRef(finishedWork: Fiber) { } if (typeof ref === 'function') { let retVal; - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { try { startLayoutEffectTimer(); retVal = ref(instanceToUse); @@ -1384,11 +1375,7 @@ function commitDetachRef(current: Fiber) { const currentRef = current.ref; if (currentRef !== null) { if (typeof currentRef === 'function') { - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - current.mode & ProfileMode - ) { + if (shouldProfile(current)) { try { startLayoutEffectTimer(); currentRef(null); @@ -1914,11 +1901,7 @@ function commitDeletionEffectsOnFiber( markComponentLayoutEffectUnmountStarted(deletedFiber); } - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - deletedFiber.mode & ProfileMode - ) { + if (shouldProfile(deletedFiber)) { startLayoutEffectTimer(); safelyCallDestroy( deletedFiber, @@ -2237,11 +2220,7 @@ function commitMutationEffectsOnFiber( // This prevents sibling component effects from interfering with each other, // e.g. a destroy function in one component should never override a ref set // by a create function in another component during the same commit. - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { try { startLayoutEffectTimer(); commitHookEffectListUnmount( @@ -2627,18 +2606,14 @@ function recursivelyTraverseLayoutEffects( setCurrentDebugFiberInDEV(prevDebugFiber); } -function disappearLayoutEffects(finishedWork: Fiber) { +export function disappearLayoutEffects(finishedWork: Fiber) { switch (finishedWork.tag) { case FunctionComponent: case ForwardRef: case MemoComponent: case SimpleMemoComponent: { // TODO (Offscreen) Check: flags & LayoutStatic - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { try { startLayoutEffectTimer(); commitHookEffectListUnmount( @@ -2709,7 +2684,7 @@ function recursivelyTraverseDisappearLayoutEffects(parentFiber: Fiber) { } } -function reappearLayoutEffects( +export function reappearLayoutEffects( finishedRoot: FiberRoot, current: Fiber | null, finishedWork: Fiber, @@ -2876,11 +2851,7 @@ function commitHookPassiveMountEffects( finishedWork: Fiber, hookFlags: HookFlags, ) { - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { startPassiveEffectTimer(); try { commitHookEffectListMount(hookFlags, finishedWork); @@ -3304,7 +3275,7 @@ function recursivelyTraverseReconnectPassiveEffects( setCurrentDebugFiberInDEV(prevDebugFiber); } -function reconnectPassiveEffects( +export function reconnectPassiveEffects( finishedRoot: FiberRoot, finishedWork: Fiber, committedLanes: Lanes, @@ -3585,11 +3556,7 @@ function commitHookPassiveUnmountEffects( nearestMountedAncestor, hookFlags: HookFlags, ) { - if ( - enableProfilerTimer && - enableProfilerCommitHooks && - finishedWork.mode & ProfileMode - ) { + if (shouldProfile(finishedWork)) { startPassiveEffectTimer(); commitHookEffectListUnmount( hookFlags, @@ -3718,7 +3685,7 @@ function recursivelyTraverseDisconnectPassiveEffects(parentFiber: Fiber): void { setCurrentDebugFiberInDEV(prevDebugFiber); } -function disconnectPassiveEffect(finishedWork: Fiber): void { +export function disconnectPassiveEffect(finishedWork: Fiber): void { switch (finishedWork.tag) { case FunctionComponent: case ForwardRef: @@ -3870,112 +3837,4 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber( } } -// TODO: Reuse reappearLayoutEffects traversal here? -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, -}; +export {commitPlacement, commitAttachRef, commitDetachRef}; diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index 0f755d865f420..9059d115c1293 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -12,52 +12,53 @@ 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 = /* */ 0b0000000000000000000000000; -export const PerformedWork = /* */ 0b0000000000000000000000001; +export const NoFlags = /* */ 0b00000000000000000000000000; +export const PerformedWork = /* */ 0b00000000000000000000000001; // You can change the rest (and add more). -export const Placement = /* */ 0b0000000000000000000000010; -export const Update = /* */ 0b0000000000000000000000100; -export const ChildDeletion = /* */ 0b0000000000000000000001000; -export const ContentReset = /* */ 0b0000000000000000000010000; -export const Callback = /* */ 0b0000000000000000000100000; -export const DidCapture = /* */ 0b0000000000000000001000000; -export const ForceClientRender = /* */ 0b0000000000000000010000000; -export const Ref = /* */ 0b0000000000000000100000000; -export const Snapshot = /* */ 0b0000000000000001000000000; -export const Passive = /* */ 0b0000000000000010000000000; -export const Hydrating = /* */ 0b0000000000000100000000000; -export const Visibility = /* */ 0b0000000000001000000000000; -export const StoreConsistency = /* */ 0b0000000000010000000000000; +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 = /* */ 0b0000000000011111111111111; +export const HostEffectMask = /* */ 0b00000000000011111111111111; // These are not really side effects, but we still reuse this field. -export const Incomplete = /* */ 0b0000000000100000000000000; -export const ShouldCapture = /* */ 0b0000000001000000000000000; -export const ForceUpdateForLegacySuspense = /* */ 0b0000000010000000000000000; -export const DidPropagateContext = /* */ 0b0000000100000000000000000; -export const NeedsPropagation = /* */ 0b0000001000000000000000000; -export const Forked = /* */ 0b0000010000000000000000000; +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 = /* */ 0b0000100000000000000000000; -export const LayoutStatic = /* */ 0b0001000000000000000000000; -export const PassiveStatic = /* */ 0b0010000000000000000000000; +export const RefStatic = /* */ 0b00000100000000000000000000; +export const LayoutStatic = /* */ 0b00001000000000000000000000; +export const PassiveStatic = /* */ 0b00010000000000000000000000; // 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 = /* */ 0b0100000000000000000000000; -export const MountPassiveDev = /* */ 0b1000000000000000000000000; +export const MountLayoutDev = /* */ 0b00100000000000000000000000; +export const MountPassiveDev = /* */ 0b01000000000000000000000000; +export const PlacementDEV = /* */ 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/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index a8844d2c1a411..a1416e6c4cf06 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -7,11 +7,12 @@ * @flow */ +import {REACT_STRICT_MODE_TYPE} from 'shared/ReactSymbols'; + import type {Wakeable} from 'shared/ReactTypes'; import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane.new'; import type {SuspenseState} from './ReactFiberSuspenseComponent.new'; -import type {Flags} from './ReactFiberFlags'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new'; import type {EventPriority} from './ReactEventPriorities.new'; import type { @@ -90,7 +91,13 @@ import { } from './ReactFiber.new'; import {isRootDehydrated} from './ReactFiberShellHydration'; import {didSuspendOrErrorWhileHydratingDEV} from './ReactFiberHydrationContext.new'; -import {NoMode, ProfileMode, ConcurrentMode} from './ReactTypeOfMode'; +import { + NoMode, + ProfileMode, + ConcurrentMode, + StrictLegacyMode, + StrictEffectsMode, +} from './ReactTypeOfMode'; import { HostRoot, IndeterminateComponent, @@ -104,7 +111,7 @@ import { SimpleMemoComponent, Profiler, } from './ReactWorkTags'; -import {LegacyRoot} from './ReactRootTags'; +import {ConcurrentRoot, LegacyRoot} from './ReactRootTags'; import { NoFlags, Incomplete, @@ -115,8 +122,7 @@ import { MutationMask, LayoutMask, PassiveMask, - MountPassiveDev, - MountLayoutDev, + PlacementDEV, } from './ReactFiberFlags'; import { NoLanes, @@ -176,10 +182,10 @@ import { commitPassiveEffectDurations, commitPassiveMountEffects, commitPassiveUnmountEffects, - invokeLayoutEffectMountInDEV, - invokePassiveEffectMountInDEV, - invokeLayoutEffectUnmountInDEV, - invokePassiveEffectUnmountInDEV, + disappearLayoutEffects, + reconnectPassiveEffects, + reappearLayoutEffects, + disconnectPassiveEffect, reportUncaughtErrorInDEV, } from './ReactFiberCommitWork.new'; import {enqueueUpdate} from './ReactFiberClassUpdateQueue.new'; @@ -267,7 +273,7 @@ type ExecutionContext = number; export const NoContext = /* */ 0b000; const BatchedContext = /* */ 0b001; const RenderContext = /* */ 0b010; -const CommitContext = /* */ 0b100; +export const CommitContext = /* */ 0b100; type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5 | 6; const RootInProgress = 0; @@ -2422,7 +2428,7 @@ function commitRootImpl( if (__DEV__ && enableStrictEffects) { if (!rootDidHavePassiveEffects) { - commitDoubleInvokeEffectsInDEV(root.current, false); + commitDoubleInvokeEffectsInDEV(root); } } @@ -2639,7 +2645,7 @@ function flushPassiveEffectsImpl() { } if (__DEV__ && enableStrictEffects) { - commitDoubleInvokeEffectsInDEV(root.current, true); + commitDoubleInvokeEffectsInDEV(root); } executionContext = prevExecutionContext; @@ -2993,64 +2999,57 @@ function flushRenderPhaseStrictModeWarningsInDEV() { } } -function commitDoubleInvokeEffectsInDEV( - fiber: Fiber, - hasPassiveEffects: boolean, +function recursivelyTraverseAndDoubleInvokeEffectsInDEV( + root: FiberRoot, + parentFiber: Fiber, + isInStrictMode: boolean, ) { - if (__DEV__ && enableStrictEffects) { - // 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. + let child = parentFiber.child; + while (child !== null) { + doubleInvokeEffectsInDEV(root, child, isInStrictMode); + child = child.sibling; + } +} +function doubleInvokeEffectsInDEV( + root: FiberRoot, + fiber: Fiber, + parentIsInStrictMode: boolean, +) { + const isStrictModeFiber = fiber.type === REACT_STRICT_MODE_TYPE; + const isInStrictMode = parentIsInStrictMode || isStrictModeFiber; + if (fiber.flags & PlacementDEV || fiber.tag === OffscreenComponent) { setCurrentDebugFiberInDEV(fiber); - invokeEffectsInDev(fiber, MountLayoutDev, invokeLayoutEffectUnmountInDEV); - if (hasPassiveEffects) { - invokeEffectsInDev( - fiber, - MountPassiveDev, - invokePassiveEffectUnmountInDEV, - ); - } - - invokeEffectsInDev(fiber, MountLayoutDev, invokeLayoutEffectMountInDEV); - if (hasPassiveEffects) { - invokeEffectsInDev(fiber, MountPassiveDev, invokePassiveEffectMountInDEV); + if (isInStrictMode) { + disappearLayoutEffects(fiber); + disconnectPassiveEffect(fiber); + reappearLayoutEffects(root, fiber.alternate, fiber, false); + reconnectPassiveEffects(root, fiber, NoLanes, null, false); } resetCurrentDebugFiberInDEV(); + } else { + recursivelyTraverseAndDoubleInvokeEffectsInDEV(root, fiber, isInStrictMode); } } -function invokeEffectsInDev( - firstChild: Fiber, - fiberFlags: Flags, - invokeEffectFn: (fiber: Fiber) => void, -): void { +function commitDoubleInvokeEffectsInDEV(root: FiberRoot) { if (__DEV__ && enableStrictEffects) { - // We don't need to re-check StrictEffectsMode here. - // This function is only called if that check has already passed. + let doubleInvokeEffects = true; - let current = firstChild; - let subtreeRoot = null; - while (current !== null) { - const primarySubtreeFlag = current.subtreeFlags & fiberFlags; - if ( - current !== subtreeRoot && - current.child !== null && - primarySubtreeFlag !== NoFlags - ) { - current = current.child; - } else { - if ((current.flags & fiberFlags) !== NoFlags) { - invokeEffectFn(current); - } - - if (current.sibling !== null) { - current = current.sibling; - } else { - current = subtreeRoot = current.return; - } - } + 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, + ); } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 14e378794bfbc..8343bad2ab488 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -7,11 +7,12 @@ * @flow */ +import {REACT_STRICT_MODE_TYPE} from 'shared/ReactSymbols'; + import type {Wakeable} from 'shared/ReactTypes'; import type {Fiber, FiberRoot} from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane.old'; import type {SuspenseState} from './ReactFiberSuspenseComponent.old'; -import type {Flags} from './ReactFiberFlags'; import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.old'; import type {EventPriority} from './ReactEventPriorities.old'; import type { @@ -90,7 +91,13 @@ import { } from './ReactFiber.old'; import {isRootDehydrated} from './ReactFiberShellHydration'; import {didSuspendOrErrorWhileHydratingDEV} from './ReactFiberHydrationContext.old'; -import {NoMode, ProfileMode, ConcurrentMode} from './ReactTypeOfMode'; +import { + NoMode, + ProfileMode, + ConcurrentMode, + StrictLegacyMode, + StrictEffectsMode, +} from './ReactTypeOfMode'; import { HostRoot, IndeterminateComponent, @@ -104,7 +111,7 @@ import { SimpleMemoComponent, Profiler, } from './ReactWorkTags'; -import {LegacyRoot} from './ReactRootTags'; +import {ConcurrentRoot, LegacyRoot} from './ReactRootTags'; import { NoFlags, Incomplete, @@ -115,8 +122,7 @@ import { MutationMask, LayoutMask, PassiveMask, - MountPassiveDev, - MountLayoutDev, + PlacementDEV, } from './ReactFiberFlags'; import { NoLanes, @@ -176,10 +182,10 @@ import { commitPassiveEffectDurations, commitPassiveMountEffects, commitPassiveUnmountEffects, - invokeLayoutEffectMountInDEV, - invokePassiveEffectMountInDEV, - invokeLayoutEffectUnmountInDEV, - invokePassiveEffectUnmountInDEV, + disappearLayoutEffects, + reconnectPassiveEffects, + reappearLayoutEffects, + disconnectPassiveEffect, reportUncaughtErrorInDEV, } from './ReactFiberCommitWork.old'; import {enqueueUpdate} from './ReactFiberClassUpdateQueue.old'; @@ -267,7 +273,7 @@ type ExecutionContext = number; export const NoContext = /* */ 0b000; const BatchedContext = /* */ 0b001; const RenderContext = /* */ 0b010; -const CommitContext = /* */ 0b100; +export const CommitContext = /* */ 0b100; type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5 | 6; const RootInProgress = 0; @@ -2422,7 +2428,7 @@ function commitRootImpl( if (__DEV__ && enableStrictEffects) { if (!rootDidHavePassiveEffects) { - commitDoubleInvokeEffectsInDEV(root.current, false); + commitDoubleInvokeEffectsInDEV(root); } } @@ -2639,7 +2645,7 @@ function flushPassiveEffectsImpl() { } if (__DEV__ && enableStrictEffects) { - commitDoubleInvokeEffectsInDEV(root.current, true); + commitDoubleInvokeEffectsInDEV(root); } executionContext = prevExecutionContext; @@ -2993,64 +2999,57 @@ function flushRenderPhaseStrictModeWarningsInDEV() { } } -function commitDoubleInvokeEffectsInDEV( - fiber: Fiber, - hasPassiveEffects: boolean, +function recursivelyTraverseAndDoubleInvokeEffectsInDEV( + root: FiberRoot, + parentFiber: Fiber, + isInStrictMode: boolean, ) { - if (__DEV__ && enableStrictEffects) { - // 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. + let child = parentFiber.child; + while (child !== null) { + doubleInvokeEffectsInDEV(root, child, isInStrictMode); + child = child.sibling; + } +} +function doubleInvokeEffectsInDEV( + root: FiberRoot, + fiber: Fiber, + parentIsInStrictMode: boolean, +) { + const isStrictModeFiber = fiber.type === REACT_STRICT_MODE_TYPE; + const isInStrictMode = parentIsInStrictMode || isStrictModeFiber; + if (fiber.flags & PlacementDEV || fiber.tag === OffscreenComponent) { setCurrentDebugFiberInDEV(fiber); - invokeEffectsInDev(fiber, MountLayoutDev, invokeLayoutEffectUnmountInDEV); - if (hasPassiveEffects) { - invokeEffectsInDev( - fiber, - MountPassiveDev, - invokePassiveEffectUnmountInDEV, - ); - } - - invokeEffectsInDev(fiber, MountLayoutDev, invokeLayoutEffectMountInDEV); - if (hasPassiveEffects) { - invokeEffectsInDev(fiber, MountPassiveDev, invokePassiveEffectMountInDEV); + if (isInStrictMode) { + disappearLayoutEffects(fiber); + disconnectPassiveEffect(fiber); + reappearLayoutEffects(root, fiber.alternate, fiber, false); + reconnectPassiveEffects(root, fiber, NoLanes, null, false); } resetCurrentDebugFiberInDEV(); + } else { + recursivelyTraverseAndDoubleInvokeEffectsInDEV(root, fiber, isInStrictMode); } } -function invokeEffectsInDev( - firstChild: Fiber, - fiberFlags: Flags, - invokeEffectFn: (fiber: Fiber) => void, -): void { +function commitDoubleInvokeEffectsInDEV(root: FiberRoot) { if (__DEV__ && enableStrictEffects) { - // We don't need to re-check StrictEffectsMode here. - // This function is only called if that check has already passed. + let doubleInvokeEffects = true; - let current = firstChild; - let subtreeRoot = null; - while (current !== null) { - const primarySubtreeFlag = current.subtreeFlags & fiberFlags; - if ( - current !== subtreeRoot && - current.child !== null && - primarySubtreeFlag !== NoFlags - ) { - current = current.child; - } else { - if ((current.flags & fiberFlags) !== NoFlags) { - invokeEffectFn(current); - } - - if (current.sibling !== null) { - current = current.sibling; - } else { - current = subtreeRoot = current.return; - } - } + 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, + ); } } diff --git a/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js b/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js new file mode 100644 index 0000000000000..f750a9b7ef0d6 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactOffscreenStrictMode-test.js @@ -0,0 +1,95 @@ +let React; +let Offscreen; +let ReactNoop; +let act; +let log; + +describe('ReactOffscreenStrictMode', () => { + beforeEach(() => { + jest.resetModules(); + log = []; + + React = require('react'); + Offscreen = React.unstable_Offscreen; + ReactNoop = require('react-noop-renderer'); + act = require('jest-react').act; + }); + + function Component({label}) { + React.useEffect(() => { + log.push(`${label}: useEffect mount`); + return () => log.push(`${label}: useEffect unmount`); + }); + + React.useLayoutEffect(() => { + log.push(`${label}: useLayoutEffect mount`); + return () => log.push(`${label}: useLayoutEffect unmount`); + }); + + log.push(`${label}: render`); + + return label; + } + + // @gate __DEV__ && enableStrictEffects && enableOffscreen + it('should trigger strict effects when offscreen is visible', () => { + act(() => { + ReactNoop.render( + + + + + , + ); + }); + + expect(log).toEqual([ + 'A: render', + 'A: render', + 'A: useLayoutEffect mount', + 'A: useEffect mount', + 'A: useLayoutEffect unmount', + 'A: useEffect unmount', + 'A: useLayoutEffect mount', + 'A: useEffect mount', + ]); + }); + + // @gate __DEV__ && enableStrictEffects && enableOffscreen + it('should not trigger strict effects when offscreen is hidden', () => { + act(() => { + ReactNoop.render( + + + + + , + ); + }); + + expect(log).toEqual(['A: render', 'A: render']); + + log = []; + + act(() => { + ReactNoop.render( + + + + + , + ); + }); + + expect(log).toEqual([ + 'A: render', + 'A: render', + 'A: useLayoutEffect mount', + 'A: useEffect mount', + 'A: useLayoutEffect unmount', + 'A: useEffect unmount', + 'A: useLayoutEffect mount', + 'A: useEffect mount', + ]); + }); +}); diff --git a/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js b/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js index f2041b4ba1e32..54150ad2d4be1 100644 --- a/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js +++ b/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js @@ -153,10 +153,8 @@ describe('StrictEffectsMode defaults', () => { , ); - expect(Scheduler).toFlushAndYieldThrough([ - 'useLayoutEffect mount "one"', - ]); expect(Scheduler).toFlushAndYield([ + 'useLayoutEffect mount "one"', 'useEffect mount "one"', 'useLayoutEffect unmount "one"', 'useEffect unmount "one"', @@ -381,6 +379,29 @@ describe('StrictEffectsMode defaults', () => { expect(Scheduler).toHaveYielded([]); }); + it('disconnects refs during double invoking', () => { + const onRefMock = jest.fn(); + function App({text}) { + return ( + { + onRefMock(ref); + }}> + text + + ); + } + + act(() => { + ReactNoop.render(); + }); + + expect(onRefMock.mock.calls.length).toBe(3); + expect(onRefMock.mock.calls[0][0]).not.toBeNull(); + expect(onRefMock.mock.calls[1][0]).toBe(null); + expect(onRefMock.mock.calls[2][0]).not.toBeNull(); + }); + it('passes the right context to class component lifecycles', () => { class App extends React.PureComponent { test() {} diff --git a/packages/react/src/__tests__/ReactStrictMode-test.js b/packages/react/src/__tests__/ReactStrictMode-test.js index 81416f260ae90..8aebe33426a0a 100644 --- a/packages/react/src/__tests__/ReactStrictMode-test.js +++ b/packages/react/src/__tests__/ReactStrictMode-test.js @@ -67,6 +67,7 @@ describe('ReactStrictMode', () => { ); }); + // @gate __DEV__ && !enableStrictEffects it('should invoke precommit lifecycle methods twice', () => { let log = []; let shouldComponentUpdate = false; @@ -107,24 +108,15 @@ describe('ReactStrictMode', () => { container, ); - if (__DEV__) { - expect(log).toEqual([ - 'constructor', - 'constructor', - 'getDerivedStateFromProps', - 'getDerivedStateFromProps', - 'render', - 'render', - 'componentDidMount', - ]); - } else { - expect(log).toEqual([ - 'constructor', - 'getDerivedStateFromProps', - 'render', - 'componentDidMount', - ]); - } + expect(log).toEqual([ + 'constructor', + 'constructor', + 'getDerivedStateFromProps', + 'getDerivedStateFromProps', + 'render', + 'render', + 'componentDidMount', + ]); log = []; shouldComponentUpdate = true; @@ -135,24 +127,15 @@ describe('ReactStrictMode', () => { , container, ); - if (__DEV__) { - expect(log).toEqual([ - 'getDerivedStateFromProps', - 'getDerivedStateFromProps', - 'shouldComponentUpdate', - 'shouldComponentUpdate', - 'render', - 'render', - 'componentDidUpdate', - ]); - } else { - expect(log).toEqual([ - 'getDerivedStateFromProps', - 'shouldComponentUpdate', - 'render', - 'componentDidUpdate', - ]); - } + expect(log).toEqual([ + 'getDerivedStateFromProps', + 'getDerivedStateFromProps', + 'shouldComponentUpdate', + 'shouldComponentUpdate', + 'render', + 'render', + 'componentDidUpdate', + ]); log = []; shouldComponentUpdate = false; @@ -164,19 +147,12 @@ describe('ReactStrictMode', () => { container, ); - if (__DEV__) { - expect(log).toEqual([ - 'getDerivedStateFromProps', - 'getDerivedStateFromProps', - 'shouldComponentUpdate', - 'shouldComponentUpdate', - ]); - } else { - expect(log).toEqual([ - 'getDerivedStateFromProps', - 'shouldComponentUpdate', - ]); - } + expect(log).toEqual([ + 'getDerivedStateFromProps', + 'getDerivedStateFromProps', + 'shouldComponentUpdate', + 'shouldComponentUpdate', + ]); }); it('should invoke setState callbacks twice', () => {