Skip to content

Commit f56dfe9

Browse files
gaearongaearon
authored
Warn on setState() in useInsertionEffect() (#24298)
* Warn on setState() in useInsertionEffect() * Use existing DEV reset mechanism
1 parent 548b542 commit f56dfe9

File tree

5 files changed

+157
-0
lines changed

5 files changed

+157
-0
lines changed

‎packages/react-reconciler/src/ReactFiberCommitWork.new.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ import {
136136
restorePendingUpdaters,
137137
addTransitionStartCallbackToPendingTransition,
138138
addTransitionCompleteCallbackToPendingTransition,
139+
setIsRunningInsertionEffect,
139140
} from './ReactFiberWorkLoop.new';
140141
import {
141142
NoFlags as NoHookEffect,
@@ -536,7 +537,17 @@ function commitHookEffectListUnmount(
536537
}
537538
}
538539

540+
if (__DEV__) {
541+
if ((flags & HookInsertion) !== NoHookEffect) {
542+
setIsRunningInsertionEffect(true);
543+
}
544+
}
539545
safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy);
546+
if (__DEV__) {
547+
if ((flags & HookInsertion) !== NoHookEffect) {
548+
setIsRunningInsertionEffect(false);
549+
}
550+
}
540551

541552
if (enableSchedulingProfiler) {
542553
if ((flags & HookPassive) !== NoHookEffect) {
@@ -570,7 +581,17 @@ function commitHookEffectListMount(flags: HookFlags, finishedWork: Fiber) {
570581

571582
// Mount
572583
const create = effect.create;
584+
if (__DEV__) {
585+
if ((flags & HookInsertion) !== NoHookEffect) {
586+
setIsRunningInsertionEffect(true);
587+
}
588+
}
573589
effect.destroy = create();
590+
if (__DEV__) {
591+
if ((flags & HookInsertion) !== NoHookEffect) {
592+
setIsRunningInsertionEffect(false);
593+
}
594+
}
574595

575596
if (enableSchedulingProfiler) {
576597
if ((flags & HookPassive) !== NoHookEffect) {

‎packages/react-reconciler/src/ReactFiberCommitWork.old.js

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,7 @@ import {
136136
restorePendingUpdaters,
137137
addTransitionStartCallbackToPendingTransition,
138138
addTransitionCompleteCallbackToPendingTransition,
139+
setIsRunningInsertionEffect,
139140
} from './ReactFiberWorkLoop.old';
140141
import {
141142
NoFlags as NoHookEffect,
@@ -536,7 +537,17 @@ function commitHookEffectListUnmount(
536537
}
537538
}
538539

540+
if (__DEV__) {
541+
if ((flags & HookInsertion) !== NoHookEffect) {
542+
setIsRunningInsertionEffect(true);
543+
}
544+
}
539545
safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy);
546+
if (__DEV__) {
547+
if ((flags & HookInsertion) !== NoHookEffect) {
548+
setIsRunningInsertionEffect(false);
549+
}
550+
}
540551

541552
if (enableSchedulingProfiler) {
542553
if ((flags & HookPassive) !== NoHookEffect) {
@@ -570,7 +581,17 @@ function commitHookEffectListMount(flags: HookFlags, finishedWork: Fiber) {
570581

571582
// Mount
572583
const create = effect.create;
584+
if (__DEV__) {
585+
if ((flags & HookInsertion) !== NoHookEffect) {
586+
setIsRunningInsertionEffect(true);
587+
}
588+
}
573589
effect.destroy = create();
590+
if (__DEV__) {
591+
if ((flags & HookInsertion) !== NoHookEffect) {
592+
setIsRunningInsertionEffect(false);
593+
}
594+
}
574595

575596
if (enableSchedulingProfiler) {
576597
if ((flags & HookPassive) !== NoHookEffect) {

‎packages/react-reconciler/src/ReactFiberWorkLoop.new.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,8 @@ let rootWithPassiveNestedUpdates: FiberRoot | null = null;
409409
let currentEventTime: number = NoTimestamp;
410410
let currentEventTransitionLane: Lanes = NoLanes;
411411

412+
let isRunningInsertionEffect = false;
413+
412414
export function getWorkInProgressRoot(): FiberRoot | null {
413415
return workInProgressRoot;
414416
}
@@ -520,6 +522,12 @@ export function scheduleUpdateOnFiber(
520522
): FiberRoot | null {
521523
checkForNestedUpdates();
522524

525+
if (__DEV__) {
526+
if (isRunningInsertionEffect) {
527+
console.error('useInsertionEffect must not schedule updates.');
528+
}
529+
}
530+
523531
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
524532
if (root === null) {
525533
return null;
@@ -2551,6 +2559,9 @@ export function captureCommitPhaseError(
25512559
nearestMountedAncestor: Fiber | null,
25522560
error: mixed,
25532561
) {
2562+
if (__DEV__) {
2563+
setIsRunningInsertionEffect(false);
2564+
}
25542565
if (sourceFiber.tag === HostRoot) {
25552566
// Error was thrown at the root. There is no parent, so the root
25562567
// itself should capture it.
@@ -3162,3 +3173,9 @@ function warnIfSuspenseResolutionNotWrappedWithActDEV(root: FiberRoot): void {
31623173
}
31633174
}
31643175
}
3176+
3177+
export function setIsRunningInsertionEffect(isRunning: boolean): void {
3178+
if (__DEV__) {
3179+
isRunningInsertionEffect = isRunning;
3180+
}
3181+
}

‎packages/react-reconciler/src/ReactFiberWorkLoop.old.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,8 @@ let rootWithPassiveNestedUpdates: FiberRoot | null = null;
409409
let currentEventTime: number = NoTimestamp;
410410
let currentEventTransitionLane: Lanes = NoLanes;
411411

412+
let isRunningInsertionEffect = false;
413+
412414
export function getWorkInProgressRoot(): FiberRoot | null {
413415
return workInProgressRoot;
414416
}
@@ -520,6 +522,12 @@ export function scheduleUpdateOnFiber(
520522
): FiberRoot | null {
521523
checkForNestedUpdates();
522524

525+
if (__DEV__) {
526+
if (isRunningInsertionEffect) {
527+
console.error('useInsertionEffect must not schedule updates.');
528+
}
529+
}
530+
523531
const root = markUpdateLaneFromFiberToRoot(fiber, lane);
524532
if (root === null) {
525533
return null;
@@ -2551,6 +2559,9 @@ export function captureCommitPhaseError(
25512559
nearestMountedAncestor: Fiber | null,
25522560
error: mixed,
25532561
) {
2562+
if (__DEV__) {
2563+
setIsRunningInsertionEffect(false);
2564+
}
25542565
if (sourceFiber.tag === HostRoot) {
25552566
// Error was thrown at the root. There is no parent, so the root
25562567
// itself should capture it.
@@ -3162,3 +3173,9 @@ function warnIfSuspenseResolutionNotWrappedWithActDEV(root: FiberRoot): void {
31623173
}
31633174
}
31643175
}
3176+
3177+
export function setIsRunningInsertionEffect(isRunning: boolean): void {
3178+
if (__DEV__) {
3179+
isRunningInsertionEffect = isRunning;
3180+
}
3181+
}

‎packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3090,6 +3090,87 @@ describe('ReactHooksWithNoopRenderer', () => {
30903090
}),
30913091
).toThrow('is not a function');
30923092
});
3093+
3094+
it('warns when setState is called from insertion effect setup', () => {
3095+
function App(props) {
3096+
const [, setX] = useState(0);
3097+
useInsertionEffect(() => {
3098+
setX(1);
3099+
if (props.throw) {
3100+
throw Error('No');
3101+
}
3102+
}, [props.throw]);
3103+
return null;
3104+
}
3105+
3106+
const root = ReactNoop.createRoot();
3107+
expect(() =>
3108+
act(() => {
3109+
root.render(<App />);
3110+
}),
3111+
).toErrorDev(['Warning: useInsertionEffect must not schedule updates.']);
3112+
3113+
expect(() => {
3114+
act(() => {
3115+
root.render(<App throw={true} />);
3116+
});
3117+
}).toThrow('No');
3118+
3119+
// Should not warn for regular effects after throw.
3120+
function NotInsertion() {
3121+
const [, setX] = useState(0);
3122+
useEffect(() => {
3123+
setX(1);
3124+
}, []);
3125+
return null;
3126+
}
3127+
act(() => {
3128+
root.render(<NotInsertion />);
3129+
});
3130+
});
3131+
3132+
it('warns when setState is called from insertion effect cleanup', () => {
3133+
function App(props) {
3134+
const [, setX] = useState(0);
3135+
useInsertionEffect(() => {
3136+
if (props.throw) {
3137+
throw Error('No');
3138+
}
3139+
return () => {
3140+
setX(1);
3141+
};
3142+
}, [props.throw, props.foo]);
3143+
return null;
3144+
}
3145+
3146+
const root = ReactNoop.createRoot();
3147+
act(() => {
3148+
root.render(<App foo="hello" />);
3149+
});
3150+
expect(() => {
3151+
act(() => {
3152+
root.render(<App foo="goodbye" />);
3153+
});
3154+
}).toErrorDev(['Warning: useInsertionEffect must not schedule updates.']);
3155+
3156+
expect(() => {
3157+
act(() => {
3158+
root.render(<App throw={true} />);
3159+
});
3160+
}).toThrow('No');
3161+
3162+
// Should not warn for regular effects after throw.
3163+
function NotInsertion() {
3164+
const [, setX] = useState(0);
3165+
useEffect(() => {
3166+
setX(1);
3167+
}, []);
3168+
return null;
3169+
}
3170+
act(() => {
3171+
root.render(<NotInsertion />);
3172+
});
3173+
});
30933174
});
30943175

30953176
describe('useLayoutEffect', () => {

0 commit comments

Comments
 (0)