Skip to content

Commit b4546cd

Browse files
authored
[enableInfiniteRenderLoopDetection] Warn about potential infinite loop, instead of interrupting (facebook#35999)
The `enableInfiniteRenderLoopDetection` feature flag is currently disabled everywhere. When attempted to roll out this at Meta, we've observed multiple false-positives, where counter-based approach would interrupt the render that would've resolved at some later iteration. This change gates the scenarios that are only discovered with the instrumentation behind `enableInfiniteRenderLoopDetection` flag to warn about potential infinite loop, instead of throwing an error and hitting an error boundary. The main reason is to see if we can a signal on which possible area of scenarios this new approach to infinite loops covers. The gist of the approach is to ensure that we are still throwing error and breaking the infinite loop, if we were doing this without `enableInfiniteRenderLoopDetection` feature flag enabled. This will log multiple errors if there is an infinite loop, but this should be fine, and it also aligns with the pattern for warnings about passive effects infinite loop. I've validated that tests in `ReactUpdates-test.js` are passing independently whether the feature flag is enabled or not.
1 parent 3f0b9e6 commit b4546cd

File tree

2 files changed

+109
-53
lines changed

2 files changed

+109
-53
lines changed

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

Lines changed: 42 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1792,8 +1792,8 @@ describe('ReactUpdates', () => {
17921792
expect(subscribers.length).toBe(limit);
17931793
});
17941794

1795-
it("does not infinite loop if there's a synchronous render phase update on another component", async () => {
1796-
if (gate(flags => !flags.enableInfiniteRenderLoopDetection)) {
1795+
it("warns about potential infinite loop if there's a synchronous render phase update on another component", async () => {
1796+
if (!__DEV__ || gate(flags => !flags.enableInfiniteRenderLoopDetection)) {
17971797
return;
17981798
}
17991799
let setState;
@@ -1809,22 +1809,29 @@ describe('ReactUpdates', () => {
18091809
return null;
18101810
}
18111811

1812-
const container = document.createElement('div');
1813-
const root = ReactDOMClient.createRoot(container);
1814-
1815-
await expect(async () => {
1816-
await act(() => ReactDOM.flushSync(() => root.render(<App />)));
1817-
}).rejects.toThrow('Maximum update depth exceeded');
1818-
assertConsoleErrorDev([
1819-
'Cannot update a component (`App`) while rendering a different component (`Child`). ' +
1820-
'To locate the bad setState() call inside `Child`, ' +
1821-
'follow the stack trace as described in https://react.dev/link/setstate-in-render\n' +
1822-
' in App (at **)',
1823-
]);
1812+
const originalConsoleError = console.error;
1813+
console.error = e => {
1814+
if (
1815+
typeof e === 'string' &&
1816+
e.startsWith(
1817+
'Maximum update depth exceeded. This could be an infinite loop.',
1818+
)
1819+
) {
1820+
Scheduler.log('stop');
1821+
}
1822+
};
1823+
try {
1824+
const container = document.createElement('div');
1825+
const root = ReactDOMClient.createRoot(container);
1826+
root.render(<App />);
1827+
await waitFor(['stop']);
1828+
} finally {
1829+
console.error = originalConsoleError;
1830+
}
18241831
});
18251832

1826-
it("does not infinite loop if there's an async render phase update on another component", async () => {
1827-
if (gate(flags => !flags.enableInfiniteRenderLoopDetection)) {
1833+
it("warns about potential infinite loop if there's an async render phase update on another component", async () => {
1834+
if (!__DEV__ || gate(flags => !flags.enableInfiniteRenderLoopDetection)) {
18281835
return;
18291836
}
18301837
let setState;
@@ -1840,21 +1847,25 @@ describe('ReactUpdates', () => {
18401847
return null;
18411848
}
18421849

1843-
const container = document.createElement('div');
1844-
const root = ReactDOMClient.createRoot(container);
1845-
1846-
await expect(async () => {
1847-
await act(() => {
1848-
React.startTransition(() => root.render(<App />));
1849-
});
1850-
}).rejects.toThrow('Maximum update depth exceeded');
1851-
1852-
assertConsoleErrorDev([
1853-
'Cannot update a component (`App`) while rendering a different component (`Child`). ' +
1854-
'To locate the bad setState() call inside `Child`, ' +
1855-
'follow the stack trace as described in https://react.dev/link/setstate-in-render\n' +
1856-
' in App (at **)',
1857-
]);
1850+
const originalConsoleError = console.error;
1851+
console.error = e => {
1852+
if (
1853+
typeof e === 'string' &&
1854+
e.startsWith(
1855+
'Maximum update depth exceeded. This could be an infinite loop.',
1856+
)
1857+
) {
1858+
Scheduler.log('stop');
1859+
}
1860+
};
1861+
try {
1862+
const container = document.createElement('div');
1863+
const root = ReactDOMClient.createRoot(container);
1864+
React.startTransition(() => root.render(<App />));
1865+
await waitFor(['stop']);
1866+
} finally {
1867+
console.error = originalConsoleError;
1868+
}
18581869
});
18591870

18601871
// TODO: Replace this branch with @gate pragmas

packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 67 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -751,6 +751,11 @@ let rootWithNestedUpdates: FiberRoot | null = null;
751751
let isFlushingPassiveEffects = false;
752752
let didScheduleUpdateDuringPassiveEffects = false;
753753

754+
const NO_NESTED_UPDATE = 0;
755+
const NESTED_UPDATE_SYNC_LANE = 1;
756+
const NESTED_UPDATE_PHASE_SPAWN = 2;
757+
let nestedUpdateKind: 0 | 1 | 2 = NO_NESTED_UPDATE;
758+
754759
const NESTED_PASSIVE_UPDATE_LIMIT = 50;
755760
let nestedPassiveUpdateCount: number = 0;
756761
let rootWithPassiveNestedUpdates: FiberRoot | null = null;
@@ -4313,15 +4318,30 @@ function flushSpawnedWork(): void {
43134318
// hydration lanes in this check, because render triggered by selective
43144319
// hydration is conceptually not an update.
43154320
if (
4321+
// Was the finished render the result of an update (not hydration)?
4322+
includesSomeLane(lanes, UpdateLanes) &&
4323+
// Did it schedule a sync update?
4324+
includesSomeLane(remainingLanes, SyncUpdateLanes)
4325+
) {
4326+
if (enableProfilerTimer && enableProfilerNestedUpdatePhase) {
4327+
markNestedUpdateScheduled();
4328+
}
4329+
4330+
// Count the number of times the root synchronously re-renders without
4331+
// finishing. If there are too many, it indicates an infinite update loop.
4332+
if (root === rootWithNestedUpdates) {
4333+
nestedUpdateCount++;
4334+
} else {
4335+
nestedUpdateCount = 0;
4336+
rootWithNestedUpdates = root;
4337+
}
4338+
nestedUpdateKind = NESTED_UPDATE_SYNC_LANE;
4339+
} else if (
43164340
// Check if there was a recursive update spawned by this render, in either
43174341
// the render phase or the commit phase. We track these explicitly because
43184342
// we can't infer from the remaining lanes alone.
4319-
(enableInfiniteRenderLoopDetection &&
4320-
(didIncludeRenderPhaseUpdate || didIncludeCommitPhaseUpdate)) ||
4321-
// Was the finished render the result of an update (not hydration)?
4322-
(includesSomeLane(lanes, UpdateLanes) &&
4323-
// Did it schedule a sync update?
4324-
includesSomeLane(remainingLanes, SyncUpdateLanes))
4343+
enableInfiniteRenderLoopDetection &&
4344+
(didIncludeRenderPhaseUpdate || didIncludeCommitPhaseUpdate)
43254345
) {
43264346
if (enableProfilerTimer && enableProfilerNestedUpdatePhase) {
43274347
markNestedUpdateScheduled();
@@ -4335,8 +4355,11 @@ function flushSpawnedWork(): void {
43354355
nestedUpdateCount = 0;
43364356
rootWithNestedUpdates = root;
43374357
}
4358+
nestedUpdateKind = NESTED_UPDATE_PHASE_SPAWN;
43384359
} else {
43394360
nestedUpdateCount = 0;
4361+
rootWithNestedUpdates = null;
4362+
nestedUpdateKind = NO_NESTED_UPDATE;
43404363
}
43414364

43424365
if (enableProfilerTimer && enableComponentPerformanceTrack) {
@@ -5152,25 +5175,47 @@ export function throwIfInfiniteUpdateLoopDetected() {
51525175
rootWithNestedUpdates = null;
51535176
rootWithPassiveNestedUpdates = null;
51545177

5178+
const updateKind = nestedUpdateKind;
5179+
nestedUpdateKind = NO_NESTED_UPDATE;
5180+
51555181
if (enableInfiniteRenderLoopDetection) {
5156-
if (executionContext & RenderContext && workInProgressRoot !== null) {
5157-
// We're in the render phase. Disable the concurrent error recovery
5158-
// mechanism to ensure that the error we're about to throw gets handled.
5159-
// We need it to trigger the nearest error boundary so that the infinite
5160-
// update loop is broken.
5161-
workInProgressRoot.errorRecoveryDisabledLanes = mergeLanes(
5162-
workInProgressRoot.errorRecoveryDisabledLanes,
5163-
workInProgressRootRenderLanes,
5164-
);
5182+
if (updateKind === NESTED_UPDATE_SYNC_LANE) {
5183+
if (executionContext & RenderContext && workInProgressRoot !== null) {
5184+
// This loop was identified only because of the instrumentation gated with enableInfiniteRenderLoopDetection, warn instead of throwing.
5185+
if (__DEV__) {
5186+
console.error(
5187+
'Maximum update depth exceeded. This could be an infinite loop. This can happen when a component ' +
5188+
'repeatedly calls setState during render phase or inside useLayoutEffect, ' +
5189+
'causing infinite render loop. React limits the number of nested updates to ' +
5190+
'prevent infinite loops.',
5191+
);
5192+
}
5193+
} else {
5194+
throw new Error(
5195+
'Maximum update depth exceeded. This can happen when a component ' +
5196+
'repeatedly calls setState inside componentWillUpdate or ' +
5197+
'componentDidUpdate. React limits the number of nested updates to ' +
5198+
'prevent infinite loops.',
5199+
);
5200+
}
5201+
} else if (updateKind === NESTED_UPDATE_PHASE_SPAWN) {
5202+
if (__DEV__) {
5203+
console.error(
5204+
'Maximum update depth exceeded. This could be an infinite loop. This can happen when a component ' +
5205+
'repeatedly calls setState during render phase or inside useLayoutEffect, ' +
5206+
'causing infinite render loop. React limits the number of nested updates to ' +
5207+
'prevent infinite loops.',
5208+
);
5209+
}
51655210
}
5211+
} else {
5212+
throw new Error(
5213+
'Maximum update depth exceeded. This can happen when a component ' +
5214+
'repeatedly calls setState inside componentWillUpdate or ' +
5215+
'componentDidUpdate. React limits the number of nested updates to ' +
5216+
'prevent infinite loops.',
5217+
);
51665218
}
5167-
5168-
throw new Error(
5169-
'Maximum update depth exceeded. This can happen when a component ' +
5170-
'repeatedly calls setState inside componentWillUpdate or ' +
5171-
'componentDidUpdate. React limits the number of nested updates to ' +
5172-
'prevent infinite loops.',
5173-
);
51745219
}
51755220

51765221
if (__DEV__) {

0 commit comments

Comments
 (0)