diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js
index 67abeb6dd0ad6..9b1478cf3967a 100644
--- a/packages/react-dom/src/__tests__/ReactUpdates-test.js
+++ b/packages/react-dom/src/__tests__/ReactUpdates-test.js
@@ -1620,6 +1620,64 @@ describe('ReactUpdates', () => {
});
});
+ it("does not infinite loop if there's a synchronous render phase update on another component", () => {
+ let setState;
+ function App() {
+ const [, _setState] = React.useState(0);
+ setState = _setState;
+ return ;
+ }
+
+ function Child(step) {
+ // This will cause an infinite update loop, and a warning in dev.
+ setState(n => n + 1);
+ return null;
+ }
+
+ const container = document.createElement('div');
+ const root = ReactDOMClient.createRoot(container);
+
+ expect(() => {
+ expect(() => ReactDOM.flushSync(() => root.render())).toThrow(
+ 'Maximum update depth exceeded',
+ );
+ }).toErrorDev(
+ 'Warning: Cannot update a component (`App`) while rendering a different component (`Child`)',
+ );
+ });
+
+ it("does not infinite loop if there's an async render phase update on another component", async () => {
+ let setState;
+ function App() {
+ const [, _setState] = React.useState(0);
+ setState = _setState;
+ return ;
+ }
+
+ function Child(step) {
+ // This will cause an infinite update loop, and a warning in dev.
+ setState(n => n + 1);
+ return null;
+ }
+
+ const container = document.createElement('div');
+ const root = ReactDOMClient.createRoot(container);
+
+ await expect(async () => {
+ let error;
+ try {
+ await act(() => {
+ React.startTransition(() => root.render());
+ });
+ } catch (e) {
+ error = e;
+ }
+ expect(error.message).toMatch('Maximum update depth exceeded');
+ }).toErrorDev(
+ 'Warning: Cannot update a component (`App`) while rendering a different component (`Child`)',
+ );
+ });
+
// TODO: Replace this branch with @gate pragmas
if (__DEV__) {
it('warns about a deferred infinite update loop with useEffect', async () => {
diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js
index 8e0c65b8552df..6b4036f71df03 100644
--- a/packages/react-reconciler/src/ReactFiberRootScheduler.js
+++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js
@@ -139,6 +139,18 @@ export function ensureRootIsScheduled(root: FiberRoot): void {
}
}
+function unscheduleAllRoots() {
+ // This is only done in a fatal error situation, as a last resort to prevent
+ // an infinite render loop.
+ let root = firstScheduledRoot;
+ while (root !== null) {
+ const next = root.next;
+ root.next = null;
+ root = next;
+ }
+ firstScheduledRoot = lastScheduledRoot = null;
+}
+
export function flushSyncWorkOnAllRoots() {
// This is allowed to be called synchronously, but the caller should check
// the execution context first.
@@ -169,10 +181,47 @@ function flushSyncWorkAcrossRoots_impl(onlyLegacy: boolean) {
// There may or may not be synchronous work scheduled. Let's check.
let didPerformSomeWork;
+ let nestedUpdatePasses = 0;
let errors: Array | null = null;
isFlushingWork = true;
do {
didPerformSomeWork = false;
+
+ // This outer loop re-runs if performing sync work on a root spawns
+ // additional sync work. If it happens too many times, it's very likely
+ // caused by some sort of infinite update loop. We already have a loop guard
+ // in place that will trigger an error on the n+1th update, but it's
+ // possible for that error to get swallowed if the setState is called from
+ // an unexpected place, like during the render phase. So as an added
+ // precaution, we also use a guard here.
+ //
+ // Ideally, there should be no known way to trigger this synchronous loop.
+ // It's really just here as a safety net.
+ //
+ // This limit is slightly larger than the one that throws inside setState,
+ // because that one is preferable because it includes a componens stack.
+ if (++nestedUpdatePasses > 60) {
+ // This is a fatal error, so we'll unschedule all the roots.
+ unscheduleAllRoots();
+ // TODO: Change this error message to something different to distinguish
+ // it from the one that is thrown from setState. Those are less fatal
+ // because they usually will result in the bad component being unmounted,
+ // and an error boundary being triggered, rather than us having to
+ // forcibly stop the entire scheduler.
+ const infiniteUpdateError = new Error(
+ 'Maximum update depth exceeded. This can happen when a component ' +
+ 'repeatedly calls setState inside componentWillUpdate or ' +
+ 'componentDidUpdate. React limits the number of nested updates to ' +
+ 'prevent infinite loops.',
+ );
+ if (errors === null) {
+ errors = [infiniteUpdateError];
+ } else {
+ errors.push(infiniteUpdateError);
+ }
+ break;
+ }
+
let root = firstScheduledRoot;
while (root !== null) {
if (onlyLegacy && root.tag !== LegacyRoot) {
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js
index c558fbd21dca6..f486bf2648dc3 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js
@@ -141,9 +141,9 @@ import {
includesExpiredLane,
getNextLanes,
getLanesToRetrySynchronouslyOnError,
- markRootUpdated,
- markRootSuspended as markRootSuspended_dontCallThisOneDirectly,
- markRootPinged,
+ markRootSuspended as _markRootSuspended,
+ markRootUpdated as _markRootUpdated,
+ markRootPinged as _markRootPinged,
markRootEntangled,
markRootFinished,
addFiberToLanesMap,
@@ -370,6 +370,13 @@ let workInProgressRootConcurrentErrors: Array> | null =
let workInProgressRootRecoverableErrors: Array> | null =
null;
+// Tracks when an update occurs during the render phase.
+let workInProgressRootDidIncludeRecursiveRenderUpdate: boolean = false;
+// Thacks when an update occurs during the commit phase. It's a separate
+// variable from the one for renders because the commit phase may run
+// concurrently to a render phase.
+let didIncludeCommitPhaseUpdate: boolean = false;
+
// The most recent time we either committed a fallback, or when a fallback was
// filled in with the resolved UI. This lets us throttle the appearance of new
// content as it streams in, to minimize jank.
@@ -1114,6 +1121,7 @@ function finishConcurrentRender(
root,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
+ workInProgressRootDidIncludeRecursiveRenderUpdate,
);
} else {
if (
@@ -1148,6 +1156,7 @@ function finishConcurrentRender(
finishedWork,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
+ workInProgressRootDidIncludeRecursiveRenderUpdate,
lanes,
),
msUntilTimeout,
@@ -1160,6 +1169,7 @@ function finishConcurrentRender(
finishedWork,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
+ workInProgressRootDidIncludeRecursiveRenderUpdate,
lanes,
);
}
@@ -1170,6 +1180,7 @@ function commitRootWhenReady(
finishedWork: Fiber,
recoverableErrors: Array> | null,
transitions: Array | null,
+ didIncludeRenderPhaseUpdate: boolean,
lanes: Lanes,
) {
// TODO: Combine retry throttling with Suspensey commits. Right now they run
@@ -1196,7 +1207,13 @@ function commitRootWhenReady(
// us that it's ready. This will be canceled if we start work on the
// root again.
root.cancelPendingCommit = schedulePendingCommit(
- commitRoot.bind(null, root, recoverableErrors, transitions),
+ commitRoot.bind(
+ null,
+ root,
+ recoverableErrors,
+ transitions,
+ didIncludeRenderPhaseUpdate,
+ ),
);
markRootSuspended(root, lanes);
return;
@@ -1204,7 +1221,7 @@ function commitRootWhenReady(
}
// Otherwise, commit immediately.
- commitRoot(root, recoverableErrors, transitions);
+ commitRoot(root, recoverableErrors, transitions, didIncludeRenderPhaseUpdate);
}
function isRenderConsistentWithExternalStores(finishedWork: Fiber): boolean {
@@ -1260,17 +1277,51 @@ function isRenderConsistentWithExternalStores(finishedWork: Fiber): boolean {
return true;
}
+// The extra indirections around markRootUpdated and markRootSuspended is
+// needed to avoid a circular dependency between this module and
+// ReactFiberLane. There's probably a better way to split up these modules and
+// avoid this problem. Perhaps all the root-marking functions should move into
+// the work loop.
+
+function markRootUpdated(root: FiberRoot, updatedLanes: Lanes) {
+ _markRootUpdated(root, updatedLanes);
+
+ // Check for recursive updates
+ if (executionContext & RenderContext) {
+ workInProgressRootDidIncludeRecursiveRenderUpdate = true;
+ } else if (executionContext & CommitContext) {
+ didIncludeCommitPhaseUpdate = true;
+ }
+
+ throwIfInfiniteUpdateLoopDetected();
+}
+
+function markRootPinged(root: FiberRoot, pingedLanes: Lanes) {
+ _markRootPinged(root, pingedLanes);
+
+ // Check for recursive pings. Pings are conceptually different from updates in
+ // other contexts but we call it an "update" in this context because
+ // repeatedly pinging a suspended render can cause a recursive render loop.
+ // The relevant property is that it can result in a new render attempt
+ // being scheduled.
+ if (executionContext & RenderContext) {
+ workInProgressRootDidIncludeRecursiveRenderUpdate = true;
+ } else if (executionContext & CommitContext) {
+ didIncludeCommitPhaseUpdate = true;
+ }
+
+ throwIfInfiniteUpdateLoopDetected();
+}
+
function markRootSuspended(root: FiberRoot, suspendedLanes: Lanes) {
// When suspending, we should always exclude lanes that were pinged or (more
// rarely, since we try to avoid it) updated during the render phase.
- // TODO: Lol maybe there's a better way to factor this besides this
- // obnoxiously named function :)
suspendedLanes = removeLanes(suspendedLanes, workInProgressRootPingedLanes);
suspendedLanes = removeLanes(
suspendedLanes,
workInProgressRootInterleavedUpdatedLanes,
);
- markRootSuspended_dontCallThisOneDirectly(root, suspendedLanes);
+ _markRootSuspended(root, suspendedLanes);
}
// This is the entry point for synchronous tasks that don't go
@@ -1341,6 +1392,7 @@ export function performSyncWorkOnRoot(root: FiberRoot): null {
root,
workInProgressRootRecoverableErrors,
workInProgressTransitions,
+ workInProgressRootDidIncludeRecursiveRenderUpdate,
);
// Before exiting, make sure there's a callback scheduled for the next
@@ -1555,6 +1607,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber {
workInProgressRootPingedLanes = NoLanes;
workInProgressRootConcurrentErrors = null;
workInProgressRootRecoverableErrors = null;
+ workInProgressRootDidIncludeRecursiveRenderUpdate = false;
finishQueueingConcurrentUpdates();
@@ -2577,6 +2630,7 @@ function commitRoot(
root: FiberRoot,
recoverableErrors: null | Array>,
transitions: Array | null,
+ didIncludeRenderPhaseUpdate: boolean,
) {
// TODO: This no longer makes any sense. We already wrap the mutation and
// layout phases. Should be able to remove.
@@ -2590,6 +2644,7 @@ function commitRoot(
root,
recoverableErrors,
transitions,
+ didIncludeRenderPhaseUpdate,
previousUpdateLanePriority,
);
} finally {
@@ -2604,6 +2659,7 @@ function commitRootImpl(
root: FiberRoot,
recoverableErrors: null | Array>,
transitions: Array | null,
+ didIncludeRenderPhaseUpdate: boolean,
renderPriorityLevel: EventPriority,
) {
do {
@@ -2683,6 +2739,9 @@ function commitRootImpl(
markRootFinished(root, remainingLanes);
+ // Reset this before firing side effects so we can detect recursive updates.
+ didIncludeCommitPhaseUpdate = false;
+
if (root === workInProgressRoot) {
// We can reset these now that they are finished.
workInProgressRoot = null;
@@ -2929,7 +2988,19 @@ function commitRootImpl(
// Read this again, since a passive effect might have updated it
remainingLanes = root.pendingLanes;
- if (includesSyncLane(remainingLanes)) {
+ if (
+ // Check if there was a recursive update spawned by this render, in either
+ // the render phase or the commit phase. We track these explicitly because
+ // we can't infer from the remaining lanes alone.
+ didIncludeCommitPhaseUpdate ||
+ didIncludeRenderPhaseUpdate ||
+ // As an additional precaution, we also check if there's any remaining sync
+ // work. Theoretically this should be unreachable but if there's a mistake
+ // in React it helps to be overly defensive given how hard it is to debug
+ // those scenarios otherwise. This won't catch recursive async updates,
+ // though, which is why we check the flags above first.
+ includesSyncLane(remainingLanes)
+ ) {
if (enableProfilerTimer && enableProfilerNestedUpdatePhase) {
markNestedUpdateScheduled();
}
@@ -3471,6 +3542,17 @@ export function throwIfInfiniteUpdateLoopDetected() {
rootWithNestedUpdates = null;
rootWithPassiveNestedUpdates = null;
+ if (executionContext & RenderContext && workInProgressRoot !== null) {
+ // We're in the render phase. Disable the concurrent error recovery
+ // mechanism to ensure that the error we're about to throw gets handled.
+ // We need it to trigger the nearest error boundary so that the infinite
+ // update loop is broken.
+ workInProgressRoot.errorRecoveryDisabledLanes = mergeLanes(
+ workInProgressRoot.errorRecoveryDisabledLanes,
+ workInProgressRootRenderLanes,
+ );
+ }
+
throw new Error(
'Maximum update depth exceeded. This can happen when a component ' +
'repeatedly calls setState inside componentWillUpdate or ' +
diff --git a/scripts/jest/matchers/toWarnDev.js b/scripts/jest/matchers/toWarnDev.js
index 88ae8661e9964..992cedcf6e262 100644
--- a/scripts/jest/matchers/toWarnDev.js
+++ b/scripts/jest/matchers/toWarnDev.js
@@ -69,12 +69,16 @@ const createMatcherFor = (consoleMethod, matcherName) =>
(message.includes('\n in ') || message.includes('\n at '));
const consoleSpy = (format, ...args) => {
- // Ignore uncaught errors reported by jsdom
- // and React addendums because they're too noisy.
if (
- !logAllErrors &&
- consoleMethod === 'error' &&
- shouldIgnoreConsoleError(format, args)
+ // Ignore uncaught errors reported by jsdom
+ // and React addendums because they're too noisy.
+ (!logAllErrors &&
+ consoleMethod === 'error' &&
+ shouldIgnoreConsoleError(format, args)) ||
+ // Ignore error objects passed to console.error, which we sometimes
+ // use as a fallback behavior, like when reportError
+ // isn't available.
+ typeof format !== 'string'
) {
return;
}