Warn about async infinite useEffect loop

Author: gaearon

packages/react-dom/src/__tests__/ReactUpdates-test.js

@@ -12,13 +12,15 @@
 let React;
 let ReactDOM;
 let ReactTestUtils;
+let Scheduler;
 
 describe('ReactUpdates', () => {
   beforeEach(() => {
     jest.resetModules();
     React = require('react');
     ReactDOM = require('react-dom');
     ReactTestUtils = require('react-dom/test-utils');
+    Scheduler = require('scheduler');
   });
 
   it('should batch state when updating state twice', () => {
@@ -1524,4 +1526,99 @@ describe('ReactUpdates', () => {
       });
     });
   });
+
+  if (__DEV__) {
+    it('warns about a deferred infinite update loop with useEffect', async () => {
+      function NonTerminating() {
+        const [step, setStep] = React.useState(0);
+        React.useEffect(() => {
+          setStep(x => x + 1);
+          Scheduler.yieldValue(step);
+        });
+        return step;
+      }
+
+      function App() {
+        return <NonTerminating />;
+      }
+
+      let error = null;
+      let stack = null;
+      let originalConsoleError = console.error;
+      console.error = (e, s1, s2) => {
+        error = e;
+        stack = s1 + s2;
+      };
+      try {
+        const container = document.createElement('div');
+        ReactDOM.render(<App />, container);
+        while (error === null) {
+          Scheduler.unstable_flushNumberOfYields(1);
+          await Promise.resolve();
+        }
+        expect(error).toContain('Warning: Maximum update depth exceeded.');
+        expect(stack).toContain('in NonTerminating');
+      } finally {
+        console.error = originalConsoleError;
+      }
+    });
+
+    it('can have nested updates if they do not cross the limit', async () => {
+      let _setStep;
+      const LIMIT = 50;
+
+      function Terminating() {
+        const [step, setStep] = React.useState(0);
+        _setStep = setStep;
+        React.useEffect(() => {
+          if (step < LIMIT) {
+            setStep(x => x + 1);
+            Scheduler.yieldValue(step);
+          }
+        });
+        return step;
+      }
+
+      const container = document.createElement('div');
+      ReactDOM.render(<Terminating />, container);
+
+      // Verify we can flush them asynchronously without warning
+      for (let i = 0; i < LIMIT * 2; i++) {
+        Scheduler.unstable_flushNumberOfYields(1);
+        await Promise.resolve();
+      }
+      expect(container.textContent).toBe('50');
+
+      // Verify restarting from 0 doesn't cross the limit
+      expect(() => {
+        _setStep(0);
+      }).toWarnDev(
+        'An update to Terminating inside a test was not wrapped in act',
+      );
+      expect(container.textContent).toBe('0');
+      for (let i = 0; i < LIMIT * 2; i++) {
+        Scheduler.unstable_flushNumberOfYields(1);
+        await Promise.resolve();
+      }
+      expect(container.textContent).toBe('50');
+    });
+
+    it('can have many updates inside useEffect without triggering a warning', () => {
+      function Terminating() {
+        const [step, setStep] = React.useState(0);
+        React.useEffect(() => {
+          for (let i = 0; i < 1000; i++) {
+            setStep(x => x + 1);
+          }
+          Scheduler.yieldValue('Done');
+        }, []);
+        return step;
+      }
+
+      const container = document.createElement('div');
+      ReactDOM.render(<Terminating />, container);
+      expect(Scheduler).toFlushAndYield(['Done']);
+      expect(container.textContent).toBe('1000');
+    });
+  }
 });

packages/react-reconciler/src/ReactFiberScheduler.old.js

@@ -67,6 +67,7 @@ import {
 } from 'shared/ReactFeatureFlags';
 import getComponentName from 'shared/getComponentName';
 import invariant from 'shared/invariant';
+import warning from 'shared/warning';
 import warningWithoutStack from 'shared/warningWithoutStack';
 
 import ReactFiberInstrumentation from './ReactFiberInstrumentation';
@@ -547,7 +548,9 @@ function commitPassiveEffects(root: FiberRoot, firstEffect: Fiber): void {
       let didError = false;
       let error;
       if (__DEV__) {
+        isInPassiveEffectDEV = true;
         invokeGuardedCallback(null, commitPassiveHookEffects, null, effect);
+        isInPassiveEffectDEV = false;
         if (hasCaughtError()) {
           didError = true;
           error = clearCaughtError();
@@ -581,6 +584,14 @@ function commitPassiveEffects(root: FiberRoot, firstEffect: Fiber): void {
   if (!isBatchingUpdates && !isRendering) {
     performSyncWork();
   }
+
+  if (__DEV__) {
+    if (rootWithPendingPassiveEffects === root) {
+      nestedPassiveEffectCountDEV++;
+    } else {
+      nestedPassiveEffectCountDEV = 0;
+    }
+  }
 }
 
 function isAlreadyFailedLegacyErrorBoundary(instance: mixed): boolean {
@@ -1897,6 +1908,21 @@ function scheduleWork(fiber: Fiber, expirationTime: ExpirationTime) {
         'the number of nested updates to prevent infinite loops.',
     );
   }
+  if (__DEV__) {
+    if (
+      isInPassiveEffectDEV &&
+      nestedPassiveEffectCountDEV > NESTED_PASSIVE_UPDATE_LIMIT
+    ) {
+      nestedPassiveEffectCountDEV = 0;
+      warning(
+        false,
+        'Maximum update depth exceeded. This can happen when a ' +
+          'component calls setState inside useEffect, but ' +
+          "useEffect either doesn't have a dependency array, or " +
+          'one of the dependencies changes on every render.',
+      );
+    }
+  }
 }
 
 function deferredUpdates<A>(fn: () => A): A {
@@ -1962,6 +1988,15 @@ const NESTED_UPDATE_LIMIT = 50;
 let nestedUpdateCount: number = 0;
 let lastCommittedRootDuringThisBatch: FiberRoot | null = null;
 
+// Similar, but for useEffect infinite loops. These are DEV-only.
+const NESTED_PASSIVE_UPDATE_LIMIT = 50;
+let nestedPassiveEffectCountDEV;
+let isInPassiveEffectDEV;
+if (__DEV__) {
+  nestedPassiveEffectCountDEV = 0;
+  isInPassiveEffectDEV = false;
+}
+
 function recomputeCurrentRendererTime() {
   const currentTimeMs = now() - originalStartTimeMs;
   currentRendererTime = msToExpirationTime(currentTimeMs);
@@ -2326,6 +2361,12 @@ function finishRendering() {
   nestedUpdateCount = 0;
   lastCommittedRootDuringThisBatch = null;
 
+  if (__DEV__) {
+    if (rootWithPendingPassiveEffects === null) {
+      nestedPassiveEffectCountDEV = 0;
+    }
+  }
+
   if (completedBatches !== null) {
     const batches = completedBatches;
     completedBatches = null;