diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js index 533ce0743a9a0..a3a575470e579 100644 --- a/packages/react-dom/src/__tests__/ReactUpdates-test.js +++ b/packages/react-dom/src/__tests__/ReactUpdates-test.js @@ -12,6 +12,7 @@ let React; let ReactDOM; let ReactTestUtils; +let Scheduler; describe('ReactUpdates', () => { beforeEach(() => { @@ -19,6 +20,7 @@ describe('ReactUpdates', () => { 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 ; + } + + 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(, 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(, 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(, container); + expect(Scheduler).toFlushAndYield(['Done']); + expect(container.textContent).toBe('1000'); + }); + } }); diff --git a/packages/react-reconciler/src/ReactFiberScheduler.old.js b/packages/react-reconciler/src/ReactFiberScheduler.old.js index e24e2ad714803..4019d43971283 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.old.js +++ b/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(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;