Skip to content

Commit 43bfd09

Browse files
committed
Add an infinite loop guard around sync work loop
We already have an infinite loop guard that detects unguarded updates, like if you repeatedly call `setState` in an effect — the nth+1 update will trigger an error. However, a limitation is that it depends on the error causing the component to be unmounted. This is usually the case but there are ways to break this expectation with undefined behavior, like if you call `setState` during the render phase on an different component (which is a bad pattern that is always accompanied by a warning). Aside from product bugs, if there were a bug in React, it might also cause something like this. Whether the bug happens because of app code or internal React code, we should do our best to avoid infinite loops because they are so hard to debug; even if it's a fatal bug and the UI can't recover, we at least need to free up the main thread so that it's more likely for the error to be reported. So this adds a second infinite loop guard around the synchronous work loop. This won't cover all possible loops, such as ones triggered during a concurrent render, but it should cover the most egregious ones.
1 parent d02afcc commit 43bfd09

File tree

3 files changed

+41
-7
lines changed

3 files changed

+41
-7
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1638,8 +1638,9 @@ describe('ReactUpdates', () => {
16381638
const root = ReactDOMClient.createRoot(container);
16391639

16401640
expect(() => {
1641-
// TODO: Add more specific error message once this is fixed
1642-
expect(() => ReactDOM.flushSync(() => root.render(<App />))).toThrow();
1641+
expect(() => ReactDOM.flushSync(() => root.render(<App />))).toThrow(
1642+
'Maximum update depth exceeded',
1643+
);
16431644
}).toErrorDev(
16441645
'Warning: Cannot update a component (`App`) while rendering a different component (`Child`)',
16451646
);

packages/react-reconciler/src/ReactFiberRootScheduler.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,10 +168,39 @@ function flushSyncWorkAcrossRoots_impl(onlyLegacy: boolean) {
168168

169169
// There may or may not be synchronous work scheduled. Let's check.
170170
let didPerformSomeWork;
171+
let nestedUpdatePasses = 0;
171172
let errors: Array<mixed> | null = null;
172173
isFlushingWork = true;
173174
do {
174175
didPerformSomeWork = false;
176+
177+
// This outer loop re-runs if performing sync work on a root spawns
178+
// additional sync work. If it happens too many times, it's very likely
179+
// caused by some sort of infinite update loop. We already have a loop guard
180+
// in place that will trigger an error on the n+1th update, but it's
181+
// possible for that error to get swallowed if the setState is called from
182+
// an unexpected place, like during the render phase. So as an added
183+
// precaution, we also use a guard here.
184+
//
185+
// This limit is slightly larger than the one that throws inside setState,
186+
// because that one is preferable because it includes a componens stack.
187+
if (++nestedUpdatePasses > 60) {
188+
// This is a fatal error, so we'll unschedule all the roots.
189+
firstScheduledRoot = lastScheduledRoot = null;
190+
const infiniteUpdateError = new Error(
191+
'Maximum update depth exceeded. This can happen when a component ' +
192+
'repeatedly calls setState inside componentWillUpdate or ' +
193+
'componentDidUpdate. React limits the number of nested updates to ' +
194+
'prevent infinite loops.',
195+
);
196+
if (errors === null) {
197+
errors = [infiniteUpdateError];
198+
} else {
199+
errors.push(infiniteUpdateError);
200+
}
201+
break;
202+
}
203+
175204
let root = firstScheduledRoot;
176205
while (root !== null) {
177206
if (onlyLegacy && root.tag !== LegacyRoot) {

scripts/jest/matchers/toWarnDev.js

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -69,12 +69,16 @@ const createMatcherFor = (consoleMethod, matcherName) =>
6969
(message.includes('\n in ') || message.includes('\n at '));
7070

7171
const consoleSpy = (format, ...args) => {
72-
// Ignore uncaught errors reported by jsdom
73-
// and React addendums because they're too noisy.
7472
if (
75-
!logAllErrors &&
76-
consoleMethod === 'error' &&
77-
shouldIgnoreConsoleError(format, args)
73+
// Ignore uncaught errors reported by jsdom
74+
// and React addendums because they're too noisy.
75+
(!logAllErrors &&
76+
consoleMethod === 'error' &&
77+
shouldIgnoreConsoleError(format, args)) ||
78+
// Ignore error objects passed to console.error, which we sometimes
79+
// use as a fallback behavior, like when reportError
80+
// isn't available.
81+
typeof format !== 'string'
7882
) {
7983
return;
8084
}

0 commit comments

Comments
 (0)