Skip to content

Commit

Permalink
Warn on setState() in useInsertionEffect() (#24298)
Browse files Browse the repository at this point in the history 
* Warn on setState() in useInsertionEffect()

* Use existing DEV reset mechanism
  • Loading branch information
@gaearon @rickhanlonii
gaearon authored and rickhanlonii committed Apr 14, 2022
1 parent 6d1a143 commit 08b7afe
Show file tree
Hide file tree
Showing 5 changed files with 157 additions and 0 deletions.
21 changes: 21 additions & 0 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
View file
Expand Up @@ -136,6 +136,7 @@ import {
restorePendingUpdaters,
addTransitionStartCallbackToPendingTransition,
addTransitionCompleteCallbackToPendingTransition,
setIsRunningInsertionEffect,
} from './ReactFiberWorkLoop.new';
import {
NoFlags as NoHookEffect,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
21 changes: 21 additions & 0 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
View file
Expand Up @@ -136,6 +136,7 @@ import {
restorePendingUpdaters,
addTransitionStartCallbackToPendingTransition,
addTransitionCompleteCallbackToPendingTransition,
setIsRunningInsertionEffect,
} from './ReactFiberWorkLoop.old';
import {
NoFlags as NoHookEffect,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down
17 changes: 17 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
View file
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -3162,3 +3173,9 @@ function warnIfSuspenseResolutionNotWrappedWithActDEV(root: FiberRoot): void {
}
}
}

export function setIsRunningInsertionEffect(isRunning: boolean): void {
if (__DEV__) {
isRunningInsertionEffect = isRunning;
}
}
17 changes: 17 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.old.js
View file
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -3162,3 +3173,9 @@ function warnIfSuspenseResolutionNotWrappedWithActDEV(root: FiberRoot): void {
}
}
}

export function setIsRunningInsertionEffect(isRunning: boolean): void {
if (__DEV__) {
isRunningInsertionEffect = isRunning;
}
}
81 changes: 81 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
View file
Expand Up @@ -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(<App />);
}),
).toErrorDev(['Warning: useInsertionEffect must not schedule updates.']);

expect(() => {
act(() => {
root.render(<App throw={true} />);
});
}).toThrow('No');

// Should not warn for regular effects after throw.
function NotInsertion() {
const [, setX] = useState(0);
useEffect(() => {
setX(1);
}, []);
return null;
}
act(() => {
root.render(<NotInsertion />);
});
});

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(<App foo="hello" />);
});
expect(() => {
act(() => {
root.render(<App foo="goodbye" />);
});
}).toErrorDev(['Warning: useInsertionEffect must not schedule updates.']);

expect(() => {
act(() => {
root.render(<App throw={true} />);
});
}).toThrow('No');

// Should not warn for regular effects after throw.
function NotInsertion() {
const [, setX] = useState(0);
useEffect(() => {
setX(1);
}, []);
return null;
}
act(() => {
root.render(<NotInsertion />);
});
});
});

describe('useLayoutEffect', () => {
Expand Down

0 comments on commit 08b7afe

Please sign in to comment.