From edcdfc7a85c9d816fca083ff4d11a69c7edb2729 Mon Sep 17 00:00:00 2001 From: dan Date: Thu, 7 Apr 2022 20:12:49 +0100 Subject: [PATCH] Warn on setState() in useInsertionEffect() (#24298) * Warn on setState() in useInsertionEffect() * Use existing DEV reset mechanism --- .../src/ReactFiberCommitWork.new.js | 21 +++++ .../src/ReactFiberCommitWork.old.js | 21 +++++ .../src/ReactFiberWorkLoop.new.js | 17 ++++ .../src/ReactFiberWorkLoop.old.js | 17 ++++ .../ReactHooksWithNoopRenderer-test.js | 81 +++++++++++++++++++ 5 files changed, 157 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index fb60401095184..d1bbdcdde156d 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,17 @@ function commitHookEffectListUnmount( } } + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(true); + } + } safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(false); + } + } if (enableSchedulingProfiler) { if ((flags & HookPassive) !== NoHookEffect) { @@ -570,7 +581,17 @@ function commitHookEffectListMount(flags: HookFlags, finishedWork: Fiber) { // Mount const create = effect.create; + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(true); + } + } effect.destroy = create(); + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(false); + } + } 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..bff8b2781310b 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,17 @@ function commitHookEffectListUnmount( } } + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(true); + } + } safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(false); + } + } if (enableSchedulingProfiler) { if ((flags & HookPassive) !== NoHookEffect) { @@ -570,7 +581,17 @@ function commitHookEffectListMount(flags: HookFlags, finishedWork: Fiber) { // Mount const create = effect.create; + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(true); + } + } effect.destroy = create(); + if (__DEV__) { + if ((flags & HookInsertion) !== NoHookEffect) { + setIsRunningInsertionEffect(false); + } + } 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 3f4071164df57..75d9c76b55d18 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -409,6 +409,8 @@ let rootWithPassiveNestedUpdates: FiberRoot | null = null; let currentEventTime: number = NoTimestamp; let currentEventTransitionLane: Lanes = NoLanes; +let isRunningInsertionEffect = false; + export function getWorkInProgressRoot(): FiberRoot | null { return workInProgressRoot; } @@ -520,6 +522,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; @@ -2551,6 +2559,9 @@ export function captureCommitPhaseError( nearestMountedAncestor: Fiber | null, error: mixed, ) { + if (__DEV__) { + setIsRunningInsertionEffect(false); + } if (sourceFiber.tag === HostRoot) { // Error was thrown at the root. There is no parent, so the root // itself should capture it. @@ -3162,3 +3173,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 604a585406ae6..17bfc935bc90f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -409,6 +409,8 @@ let rootWithPassiveNestedUpdates: FiberRoot | null = null; let currentEventTime: number = NoTimestamp; let currentEventTransitionLane: Lanes = NoLanes; +let isRunningInsertionEffect = false; + export function getWorkInProgressRoot(): FiberRoot | null { return workInProgressRoot; } @@ -520,6 +522,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; @@ -2551,6 +2559,9 @@ export function captureCommitPhaseError( nearestMountedAncestor: Fiber | null, error: mixed, ) { + if (__DEV__) { + setIsRunningInsertionEffect(false); + } if (sourceFiber.tag === HostRoot) { // Error was thrown at the root. There is no parent, so the root // itself should capture it. @@ -3162,3 +3173,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..bacf9da7029a7 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -3090,6 +3090,87 @@ 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); + if (props.throw) { + throw Error('No'); + } + }, [props.throw]); + return null; + } + + const root = ReactNoop.createRoot(); + expect(() => + act(() => { + root.render(); + }), + ).toErrorDev(['Warning: useInsertionEffect must not schedule updates.']); + + expect(() => { + act(() => { + root.render(); + }); + }).toThrow('No'); + + // Should not warn for regular effects after throw. + function NotInsertion() { + const [, setX] = useState(0); + useEffect(() => { + setX(1); + }, []); + return null; + } + act(() => { + root.render(); + }); + }); + + it('warns when setState is called from insertion effect cleanup', () => { + function App(props) { + const [, setX] = useState(0); + useInsertionEffect(() => { + if (props.throw) { + throw Error('No'); + } + return () => { + setX(1); + }; + }, [props.throw, props.foo]); + return null; + } + + const root = ReactNoop.createRoot(); + act(() => { + root.render(); + }); + expect(() => { + act(() => { + root.render(); + }); + }).toErrorDev(['Warning: useInsertionEffect must not schedule updates.']); + + expect(() => { + act(() => { + root.render(); + }); + }).toThrow('No'); + + // Should not warn for regular effects after throw. + function NotInsertion() { + const [, setX] = useState(0); + useEffect(() => { + setX(1); + }, []); + return null; + } + act(() => { + root.render(); + }); + }); }); describe('useLayoutEffect', () => {