Warn on setState() in useInsertionEffect()

facebook · Apr 7, 2022 · b58e231 · b58e231
1 parent e8f4a66
commit b58e231
Show file tree

Hide file tree

Showing 5 changed files with 107 additions and 4 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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(<App />);
+        }),
+      ).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(<App foo="hello" />);
+      });
+      expect(() => {
+        act(() => {
+          root.render(<App foo="goodbye" />);
+        });
+      }).toErrorDev(['Warning: useInsertionEffect must not schedule updates.']);
+    });
   });
 
   describe('useLayoutEffect', () => {