From b58e231cc9122e559c225125896ab4ae1769a79a Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Thu, 7 Apr 2022 18:54:44 +0100 Subject: [PATCH] Warn on setState() in useInsertionEffect() --- .../src/ReactFiberCommitWork.new.js | 23 +++++++++++- .../src/ReactFiberCommitWork.old.js | 23 +++++++++++- .../src/ReactFiberWorkLoop.new.js | 14 +++++++ .../src/ReactFiberWorkLoop.old.js | 14 +++++++ .../ReactHooksWithNoopRenderer-test.js | 37 +++++++++++++++++++ 5 files changed, 107 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index fb60401095184..aad83fa42c8c2 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -136,6 +136,7 @@ import { restorePendingUpdaters, addTransitionStartCallbackToPendingTransition, addTransitionCompleteCallbackToPendingTransition, + setIsRunningInsertionEffect, } from './ReactFiberWorkLoop.new'; import { NoFlags as NoHookEffect, @@ -536,7 +537,16 @@ function commitHookEffectListUnmount( } } - safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + if (__DEV__ && (flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(true); + try { + safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + } finally { + setIsRunningInsertionEffect(false); + } + } else { + safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + } if (enableSchedulingProfiler) { if ((flags & HookPassive) !== NoHookEffect) { @@ -570,7 +580,16 @@ function commitHookEffectListMount(flags: HookFlags, finishedWork: Fiber) { // Mount const create = effect.create; - effect.destroy = create(); + if (__DEV__ && (flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(true); + try { + effect.destroy = create(); + } finally { + setIsRunningInsertionEffect(false); + } + } else { + effect.destroy = create(); + } if (enableSchedulingProfiler) { if ((flags & HookPassive) !== NoHookEffect) { diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index e962039d9ca9c..187a7776c8ada 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -136,6 +136,7 @@ import { restorePendingUpdaters, addTransitionStartCallbackToPendingTransition, addTransitionCompleteCallbackToPendingTransition, + setIsRunningInsertionEffect, } from './ReactFiberWorkLoop.old'; import { NoFlags as NoHookEffect, @@ -536,7 +537,16 @@ function commitHookEffectListUnmount( } } - safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + if (__DEV__ && (flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(true); + try { + safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + } finally { + setIsRunningInsertionEffect(false); + } + } else { + safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + } if (enableSchedulingProfiler) { if ((flags & HookPassive) !== NoHookEffect) { @@ -570,7 +580,16 @@ function commitHookEffectListMount(flags: HookFlags, finishedWork: Fiber) { // Mount const create = effect.create; - effect.destroy = create(); + if (__DEV__ && (flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(true); + try { + effect.destroy = create(); + } finally { + setIsRunningInsertionEffect(false); + } + } else { + effect.destroy = create(); + } if (enableSchedulingProfiler) { if ((flags & HookPassive) !== NoHookEffect) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 4da17c7bb6e80..c03c8a14cb004 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -406,6 +406,8 @@ let nestedPassiveUpdateCount: number = 0; let currentEventTime: number = NoTimestamp; let currentEventTransitionLane: Lanes = NoLanes; +let isRunningInsertionEffect = false; + export function getWorkInProgressRoot(): FiberRoot | null { return workInProgressRoot; } @@ -517,6 +519,12 @@ export function scheduleUpdateOnFiber( ): FiberRoot | null { checkForNestedUpdates(); + if (__DEV__) { + if (isRunningInsertionEffect) { + console.error('useInsertionEffect must not schedule updates.'); + } + } + const root = markUpdateLaneFromFiberToRoot(fiber, lane); if (root === null) { return null; @@ -3132,3 +3140,9 @@ function warnIfSuspenseResolutionNotWrappedWithActDEV(root: FiberRoot): void { } } } + +export function setIsRunningInsertionEffect(isRunning: boolean): void { + if (__DEV__) { + isRunningInsertionEffect = isRunning; + } +} diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 2c940a71001db..926818d38b320 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -406,6 +406,8 @@ let nestedPassiveUpdateCount: number = 0; let currentEventTime: number = NoTimestamp; let currentEventTransitionLane: Lanes = NoLanes; +let isRunningInsertionEffect = false; + export function getWorkInProgressRoot(): FiberRoot | null { return workInProgressRoot; } @@ -517,6 +519,12 @@ export function scheduleUpdateOnFiber( ): FiberRoot | null { checkForNestedUpdates(); + if (__DEV__) { + if (isRunningInsertionEffect) { + console.error('useInsertionEffect must not schedule updates.'); + } + } + const root = markUpdateLaneFromFiberToRoot(fiber, lane); if (root === null) { return null; @@ -3132,3 +3140,9 @@ function warnIfSuspenseResolutionNotWrappedWithActDEV(root: FiberRoot): void { } } } + +export function setIsRunningInsertionEffect(isRunning: boolean): void { + if (__DEV__) { + isRunningInsertionEffect = isRunning; + } +} diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 811be09639158..dae042d6c6ed3 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -3090,6 +3090,43 @@ describe('ReactHooksWithNoopRenderer', () => { }), ).toThrow('is not a function'); }); + + it('warns when setState is called from insertion effect setup', () => { + function App(props) { + const [, setX] = useState(0); + useInsertionEffect(() => { + setX(1); + }, []); + return null; + } + + const root = ReactNoop.createRoot(); + expect(() => + act(() => { + root.render(); + }), + ).toErrorDev(['Warning: useInsertionEffect must not schedule updates.']); + }); + + it('warns when setState is called from insertion effect cleanup', () => { + function App(props) { + const [, setX] = useState(0); + useInsertionEffect(() => { + return () => setX(1); + }, [props.foo]); + return null; + } + + const root = ReactNoop.createRoot(); + act(() => { + root.render(); + }); + expect(() => { + act(() => { + root.render(); + }); + }).toErrorDev(['Warning: useInsertionEffect must not schedule updates.']); + }); }); describe('useLayoutEffect', () => {