Warn about async infinite useEffect loop (#15180)
* Warn about async infinite useEffect loop * Make tests sync
Dan Abramov committed
Mar 22, 2019 at 20:04 UTC
5c2b2c0852c715abda7296bd6e7a2e941ca66969
2 files changed
+135
packages/react-dom/src/__tests__/ReactUpdates-test.js
+94
@@ -12,6 +12,7 @@
12
let React;
13
let ReactDOM;
14
let ReactTestUtils;
15
+let Scheduler;
16
17
describe('ReactUpdates', () => {
18
beforeEach(() => {
@@ -19,6 +20,7 @@ describe('ReactUpdates', () => {
20
React = require('react');
21
ReactDOM = require('react-dom');
22
ReactTestUtils = require('react-dom/test-utils');
23
+ Scheduler = require('scheduler');
24
});
25
26
it('should batch state when updating state twice', () => {
@@ -1524,4 +1526,96 @@ describe('ReactUpdates', () => {
1526
});
1527
});
1528
});
1529
+
1530
+ if (__DEV__) {
1531
+ it('warns about a deferred infinite update loop with useEffect', () => {
1532
+ function NonTerminating() {
1533
+ const [step, setStep] = React.useState(0);
1534
+ React.useEffect(() => {
1535
+ setStep(x => x + 1);
1536
+ Scheduler.yieldValue(step);
1537
+ });
1538
+ return step;
1539
+ }
1540
+
1541
+ function App() {
1542
+ return <NonTerminating />;
1543
+ }
1544
+
1545
+ let error = null;
1546
+ let stack = null;
1547
+ let originalConsoleError = console.error;
1548
+ console.error = (e, s) => {
1549
+ error = e;
1550
+ stack = s;
1551
+ };
1552
+ try {
1553
+ const container = document.createElement('div');
1554
+ ReactDOM.render(<App />, container);
1555
+ while (error === null) {
1556
+ Scheduler.unstable_flushNumberOfYields(1);
1557
+ }
1558
+ expect(error).toContain('Warning: Maximum update depth exceeded.');
1559
+ expect(stack).toContain('in NonTerminating');
1560
+ } finally {
1561
+ console.error = originalConsoleError;
1562
+ }
1563
+ });
1564
+
1565
+ it('can have nested updates if they do not cross the limit', () => {
1566
+ let _setStep;
1567
+ const LIMIT = 50;
1568
+
1569
+ function Terminating() {
1570
+ const [step, setStep] = React.useState(0);
1571
+ _setStep = setStep;
1572
+ React.useEffect(() => {
1573
+ if (step < LIMIT) {
1574
+ setStep(x => x + 1);
1575
+ Scheduler.yieldValue(step);
1576
+ }
1577
+ });
1578
+ return step;
1579
+ }
1580
+
1581
+ const container = document.createElement('div');
1582
+ ReactDOM.render(<Terminating />, container);
1583
+
1584
+ // Verify we can flush them asynchronously without warning
1585
+ for (let i = 0; i < LIMIT * 2; i++) {
1586
+ Scheduler.unstable_flushNumberOfYields(1);
1587
+ }
1588
+ expect(container.textContent).toBe('50');
1589
+
1590
+ // Verify restarting from 0 doesn't cross the limit
1591
+ expect(() => {
1592
+ _setStep(0);
1593
+ }).toWarnDev(
1594
+ 'An update to Terminating inside a test was not wrapped in act',
1595
+ );
1596
+ expect(container.textContent).toBe('0');
1597
+ for (let i = 0; i < LIMIT * 2; i++) {
1598
+ Scheduler.unstable_flushNumberOfYields(1);
1599
+ }
1600
+ expect(container.textContent).toBe('50');
1601
+ });
1602
+
1603
+ it('can have many updates inside useEffect without triggering a warning', () => {
1604
+ function Terminating() {
1605
+ const [step, setStep] = React.useState(0);
1606
+ React.useEffect(() => {
1607
+ for (let i = 0; i < 1000; i++) {
1608
+ setStep(x => x + 1);
1609
+ }
1610
+ Scheduler.yieldValue('Done');
1611
+ }, []);
1612
+ return step;
1613
+ }
1614
+
1615
+ const container = document.createElement('div');
1616
+ ReactDOM.render(<Terminating />, container);
1617
+ expect(Scheduler).toFlushAndYield(['Done']);
1618
+ expect(container.textContent).toBe('1000');
1619
+ });
1620
+ }
1621
});
packages/react-reconciler/src/ReactFiberScheduler.old.js
+41
@@ -67,6 +67,7 @@ import {
67
} from 'shared/ReactFeatureFlags';
68
import getComponentName from 'shared/getComponentName';
69
import invariant from 'shared/invariant';
70
+import warning from 'shared/warning';
71
import warningWithoutStack from 'shared/warningWithoutStack';
72
73
import ReactFiberInstrumentation from './ReactFiberInstrumentation';
@@ -547,7 +548,9 @@ function commitPassiveEffects(root: FiberRoot, firstEffect: Fiber): void {
548
let didError = false;
549
let error;
550
if (__DEV__) {
551
+ isInPassiveEffectDEV = true;
552
invokeGuardedCallback(null, commitPassiveHookEffects, null, effect);
553
+ isInPassiveEffectDEV = false;
554
if (hasCaughtError()) {
555
didError = true;
556
error = clearCaughtError();
@@ -581,6 +584,14 @@ function commitPassiveEffects(root: FiberRoot, firstEffect: Fiber): void {
584
if (!isBatchingUpdates && !isRendering) {
585
performSyncWork();
586
}
587
+
588
+ if (__DEV__) {
589
+ if (rootWithPendingPassiveEffects === root) {
590
+ nestedPassiveEffectCountDEV++;
591
+ } else {
592
+ nestedPassiveEffectCountDEV = 0;
593
+ }
594
+ }
595
}
596
597
function isAlreadyFailedLegacyErrorBoundary(instance: mixed): boolean {
@@ -1897,6 +1908,21 @@ function scheduleWork(fiber: Fiber, expirationTime: ExpirationTime) {
1908
'the number of nested updates to prevent infinite loops.',
1909
);
1910
}
1911
+ if (__DEV__) {
1912
+ if (
1913
+ isInPassiveEffectDEV &&
1914
+ nestedPassiveEffectCountDEV > NESTED_PASSIVE_UPDATE_LIMIT
1915
+ ) {
1916
+ nestedPassiveEffectCountDEV = 0;
1917
+ warning(
1918
+ false,
1919
+ 'Maximum update depth exceeded. This can happen when a ' +
1920
+ 'component calls setState inside useEffect, but ' +
1921
+ "useEffect either doesn't have a dependency array, or " +
1922
+ 'one of the dependencies changes on every render.',
1923
+ );
1924
+ }
1925
+ }
1926
}
1927
1928
function deferredUpdates<A>(fn: () => A): A {
@@ -1962,6 +1988,15 @@ const NESTED_UPDATE_LIMIT = 50;
1988
let nestedUpdateCount: number = 0;
1989
let lastCommittedRootDuringThisBatch: FiberRoot | null = null;
1990
1991
+// Similar, but for useEffect infinite loops. These are DEV-only.
1992
+const NESTED_PASSIVE_UPDATE_LIMIT = 50;
1993
+let nestedPassiveEffectCountDEV;
1994
+let isInPassiveEffectDEV;
1995
+if (__DEV__) {
1996
+ nestedPassiveEffectCountDEV = 0;
1997
+ isInPassiveEffectDEV = false;
1998
+}
1999
+
2000
function recomputeCurrentRendererTime() {
2001
const currentTimeMs = now() - originalStartTimeMs;
2002
currentRendererTime = msToExpirationTime(currentTimeMs);
@@ -2343,6 +2378,12 @@ function finishRendering() {
2378
nestedUpdateCount = 0;
2379
lastCommittedRootDuringThisBatch = null;
2380
2381
+ if (__DEV__) {
2382
+ if (rootWithPendingPassiveEffects === null) {
2383
+ nestedPassiveEffectCountDEV = 0;
2384
+ }
2385
+ }
2386
+
2387
if (completedBatches !== null) {
2388
const batches = completedBatches;
2389
completedBatches = null;