From f772e8cf15d3e1c00e194a5746846a86ef6d93be Mon Sep 17 00:00:00 2001 From: Himanshu Date: Sun, 24 May 2026 16:35:35 +0530 Subject: [PATCH] refactor: force infinite render loop detection to throw instead of warning and remove related feature flags --- .../src/__tests__/ReactUpdates-test.js | 218 +++--------------- .../src/ReactFiberWorkLoop.js | 57 ++--- .../ReactFlightAsyncDebugInfo-test.js | 24 +- packages/shared/ReactFeatureFlags.js | 1 - .../forks/ReactFeatureFlags.native-fb.js | 1 - .../forks/ReactFeatureFlags.native-oss.js | 1 - .../forks/ReactFeatureFlags.test-renderer.js | 1 - ...actFeatureFlags.test-renderer.native-fb.js | 1 - .../ReactFeatureFlags.test-renderer.www.js | 1 - .../forks/ReactFeatureFlags.www-dynamic.js | 1 - .../shared/forks/ReactFeatureFlags.www.js | 1 - 11 files changed, 58 insertions(+), 249 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js index 503f3f5c32cb..4f22bacc6078 100644 --- a/packages/react-dom/src/__tests__/ReactUpdates-test.js +++ b/packages/react-dom/src/__tests__/ReactUpdates-test.js @@ -1792,136 +1792,35 @@ describe('ReactUpdates', () => { expect(subscribers.length).toBe(limit); }); - it("warns about potential infinite loop if there's a synchronous render phase update on another component", async () => { - if ( - !__DEV__ || - gate( - flags => - !flags.enableInfiniteRenderLoopDetection || - flags.enableInfiniteRenderLoopDetectionForceThrow, - ) - ) { - return; - } - 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 originalConsoleError = console.error; - console.error = e => { - if ( - typeof e === 'string' && - e.startsWith( - 'Maximum update depth exceeded. This could be an infinite loop.', - ) - ) { - Scheduler.log('stop'); - } - }; - try { - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - root.render(); - await waitFor(['stop']); - } finally { - console.error = originalConsoleError; + it('throws on a deferred infinite update loop with useEffect', async () => { + function NonTerminating() { + const [step, setStep] = React.useState(0); + React.useEffect(function myEffect() { + setStep(x => x + 1); + }); + return step; } - }); - it("warns about potential infinite loop if there's an async render phase update on another component", async () => { - if ( - !__DEV__ || - gate( - flags => - !flags.enableInfiniteRenderLoopDetection || - flags.enableInfiniteRenderLoopDetectionForceThrow, - ) - ) { - return; - } - let setState; function App() { - const [, _setState] = React.useState(0); - setState = _setState; - return ; + return ; } - function Child(step) { - // This will cause an infinite update loop, and a warning in dev. - setState(n => n + 1); - return null; - } - - const originalConsoleError = console.error; - console.error = e => { - if ( - typeof e === 'string' && - e.startsWith( - 'Maximum update depth exceeded. This could be an infinite loop.', - ) - ) { - Scheduler.log('stop'); - } - }; - try { - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - React.startTransition(() => root.render()); - await waitFor(['stop']); - } finally { - console.error = originalConsoleError; - } - }); - - // TODO: Replace this branch with @gate pragmas - if (__DEV__) { - it('warns about a deferred infinite update loop with useEffect', async () => { - function NonTerminating() { - const [step, setStep] = React.useState(0); - React.useEffect(function myEffect() { - setStep(x => x + 1); - }); - return step; - } - - function App() { - return ; - } - - let error = null; - let ownerStack = null; - let debugStack = null; - const originalConsoleError = console.error; - console.error = e => { - error = e; - ownerStack = React.captureOwnerStack(); - debugStack = new Error().stack; - Scheduler.log('stop'); - }; - try { - const container = document.createElement('div'); - const root = ReactDOMClient.createRoot(container); - root.render(); - await waitFor(['stop']); - } finally { - console.error = originalConsoleError; + const container = document.createElement('div'); + const errors = []; + const root = ReactDOMClient.createRoot(container, { + onUncaughtError: (error, errorInfo) => { + errors.push(error.message); } + }); - expect(error).toContain('Maximum update depth exceeded'); - // The currently executing effect should be on the native stack - expect(debugStack).toContain('at myEffect'); - expect(ownerStack).toContain('at App'); + await act(() => { + root.render(); }); + expect(errors.length).toBe(1); + expect(errors[0]).toContain('Maximum update depth exceeded'); + }); + it('can have nested updates if they do not cross the limit', async () => { let _setStep; const LIMIT = 50; @@ -1973,7 +1872,6 @@ describe('ReactUpdates', () => { assertLog(['Done']); expect(container.textContent).toBe('1000'); }); - } it('prevents infinite update loop triggered by synchronous updates in useEffect', async () => { // Ignore flushSync warning @@ -2020,31 +1918,8 @@ describe('ReactUpdates', () => { ]); }); - it('warns instead of throwing when infinite Suspense ping loop is detected via enableInfiniteRenderLoopDetection during commit phase', async () => { - if ( - !__DEV__ || - gate( - flags => - !flags.enableInfiniteRenderLoopDetection || - flags.enableInfiniteRenderLoopDetectionForceThrow, - ) - ) { - return; - } - - // When a Suspense child throws a thenable, React registers two listeners: - // 1. ping (attachPingListener, render) → pingSuspendedRoot → markRootPinged - // 2. retry (attachSuspenseRetryListeners, commit) → resolveRetryWakeable - // - // The ping path calls throwIfInfiniteUpdateLoopDetected(true) via - // markRootPinged WITHOUT a prior getRootForUpdatedFiber(false) check. - // When this fires during CommitContext (not RenderContext), - // the isFromInfiniteRenderLoopDetectionInstrumentation=true parameter - // ensures we warn instead of throw. - // - // Without the fix (passing false), the condition - // false || (executionContext & RenderContext && ...) - // evaluates to false in CommitContext, causing a throw. + // @gate enableInfiniteRenderLoopDetection + it('throws when infinite Suspense ping loop is detected via enableInfiniteRenderLoopDetection during commit phase', async () => { let currentResolve = null; let shouldStop = false; @@ -2055,16 +1930,11 @@ describe('ReactUpdates', () => { if (shouldStop) { return; } - // Resolve the suspended thenable during commit phase (CommitContext). - // The ping callback (registered first during render) fires first, - // triggering markRootPinged → throwIfInfiniteUpdateLoopDetected(true). if (currentResolve !== null) { const resolve = currentResolve; currentResolve = null; resolve(); } - // Schedule a sync update to ensure nestedUpdateKind is - // NESTED_UPDATE_SYNC_LANE at commitRootImpl epilogue. setState(n => n + 1); }); @@ -2079,9 +1949,6 @@ describe('ReactUpdates', () => { if (shouldStop) { return null; } - // Each render throws a new thenable. React calls .then() on it twice - // (ping during render, retry during commit). We collect all callbacks - // so resolve() fires them in registration order: ping first. const callbacks = []; const thenable = { then(onFulfilled) { @@ -2102,42 +1969,21 @@ describe('ReactUpdates', () => { const root = ReactDOMClient.createRoot(container, { onUncaughtError: error => { errors.push(error.message); + shouldStop = true; }, }); - const originalConsoleError = console.error; - console.error = e => { - if ( - typeof e === 'string' && - e.startsWith( - 'Maximum update depth exceeded. This could be an infinite loop.', - ) - ) { - // Stop the loop after the first warning so act() can finish. - shouldStop = true; - } - }; - - try { - await act(() => { - root.render(); - }); - } finally { - console.error = originalConsoleError; - } + await act(() => { + root.render(); + }); - // With the fix (throwIfInfiniteUpdateLoopDetected(true) in markRootPinged): - // the loop is discovered via enableInfiniteRenderLoopDetection instrumentation - // and produces a warning. - // Without the fix (throwIfInfiniteUpdateLoopDetected(false)): - // the same check throws because executionContext is CommitContext, not - // RenderContext. expect(shouldStop).toBe(true); - expect(errors).toEqual([]); + expect(errors.length).toBeGreaterThanOrEqual(1); + expect(errors[0]).toContain('Maximum update depth exceeded. This could be an infinite loop.'); }); - // @gate enableInfiniteRenderLoopDetection && enableInfiniteRenderLoopDetectionForceThrow - it('throws when sync render-phase update loop is detected with force-throw enabled', async () => { + // @gate enableInfiniteRenderLoopDetection + it('throws when sync render-phase update loop is detected', async () => { // Render-phase setState on another component's hook produces a sync // recursive update. With ForceThrow enabled this should throw via // throwForcedInfiniteRenderLoopError instead of only warning in DEV. @@ -2200,8 +2046,8 @@ describe('ReactUpdates', () => { ); }); - // @gate enableInfiniteRenderLoopDetection && enableInfiniteRenderLoopDetectionForceThrow - it('throws when phase-spawn update loop is detected with force-throw enabled', async () => { + // @gate enableInfiniteRenderLoopDetection + it('throws when phase-spawn update loop is detected', async () => { // Wrapping the initial render in startTransition makes the render-phase // setState inherit a non-sync transition lane. After commit, the next // render is non-sync, so the loop detector classifies the recursion as diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index d45a2d18cf93..3687cdc949a4 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -51,7 +51,6 @@ import { disableLegacyContext, alwaysThrottleRetries, enableInfiniteRenderLoopDetection, - enableInfiniteRenderLoopDetectionForceThrow, disableLegacyMode, enableComponentPerformanceTrack, enableYieldingBeforePassive, @@ -5215,21 +5214,10 @@ export function throwIfInfiniteUpdateLoopDetected( isFromInfiniteRenderLoopDetectionInstrumentation || (executionContext & RenderContext) !== NoContext ) { - // This loop was identified only because of the instrumentation gated with enableInfiniteRenderLoopDetection, - // warn instead of throwing, unless enableInfiniteRenderLoopDetectionForceThrow. - if (enableInfiniteRenderLoopDetectionForceThrow) { - throwForcedInfiniteRenderLoopError( - workInProgressRoot, - workInProgressRootRenderLanes, - ); - } else if (__DEV__) { - console.error( - 'Maximum update depth exceeded. This could be an infinite loop. This can happen when a component ' + - 'repeatedly calls setState during render phase or inside useLayoutEffect, ' + - 'causing infinite render loop. React limits the number of nested updates to ' + - 'prevent infinite loops.', - ); - } + throwForcedInfiniteRenderLoopError( + workInProgressRoot, + workInProgressRootRenderLanes, + ); } else { throw new Error( 'Maximum update depth exceeded. This can happen when a component ' + @@ -5239,19 +5227,10 @@ export function throwIfInfiniteUpdateLoopDetected( ); } } else if (updateKind === NESTED_UPDATE_PHASE_SPAWN) { - if (enableInfiniteRenderLoopDetectionForceThrow) { - throwForcedInfiniteRenderLoopError( - workInProgressRoot, - workInProgressRootRenderLanes, - ); - } else if (__DEV__) { - console.error( - 'Maximum update depth exceeded. This could be an infinite loop. This can happen when a component ' + - 'repeatedly calls setState during render phase or inside useLayoutEffect, ' + - 'causing infinite render loop. React limits the number of nested updates to ' + - 'prevent infinite loops.', - ); - } + throwForcedInfiniteRenderLoopError( + workInProgressRoot, + workInProgressRootRenderLanes, + ); } } else { throw new Error( @@ -5263,18 +5242,16 @@ export function throwIfInfiniteUpdateLoopDetected( } } - if (__DEV__) { - if (nestedPassiveUpdateCount > NESTED_PASSIVE_UPDATE_LIMIT) { - nestedPassiveUpdateCount = 0; - rootWithPassiveNestedUpdates = null; + if (nestedPassiveUpdateCount > NESTED_PASSIVE_UPDATE_LIMIT) { + nestedPassiveUpdateCount = 0; + rootWithPassiveNestedUpdates = null; - console.error( - '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.', - ); - } + throw new Error( + '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.', + ); } } diff --git a/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js b/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js index 5107fc0bfaea..428b359d7027 100644 --- a/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js +++ b/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js @@ -314,7 +314,9 @@ describe('ReactFlightAsyncDebugInfo', () => { ], "start": 0, "value": { - "value": undefined, + "value": [ + undefined, + ], }, }, "env": "Server", @@ -419,7 +421,9 @@ describe('ReactFlightAsyncDebugInfo', () => { ], "start": 0, "value": { - "value": undefined, + "value": [ + undefined, + ], }, }, "env": "Server", @@ -3206,7 +3210,7 @@ describe('ReactFlightAsyncDebugInfo', () => { "awaited": { "end": 0, "env": "Server", - "name": "Object.readFile", + "name": "", "owner": { "env": "Server", "key": null, @@ -3223,16 +3227,6 @@ describe('ReactFlightAsyncDebugInfo', () => { ], ], }, - "stack": [ - [ - "Component", - "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 3151, - 7, - 3149, - 5, - ], - ], "start": 0, "value": { "status": "halted", @@ -3259,8 +3253,8 @@ describe('ReactFlightAsyncDebugInfo', () => { [ "Component", "/packages/react-server/src/__tests__/ReactFlightAsyncDebugInfo-test.js", - 3153, - 7, + 3150, + 20, 3149, 5, ], diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 5d2d6efaa7df..20a8fb5fd111 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -149,7 +149,6 @@ export const enableInfiniteRenderLoopDetection: boolean = false; * mechanism to throw instead of only warning in cases where it would * otherwise downgrade to a warning. */ -export const enableInfiniteRenderLoopDetectionForceThrow: boolean = false; export const enableFragmentRefs: boolean = true; export const enableFragmentRefsScrollIntoView: boolean = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 96d13e9ec461..e420b5443cc4 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -47,7 +47,6 @@ export const enableCreateEventHandleAPI: boolean = false; export const enableMoveBefore: boolean = true; export const enableFizzExternalRuntime: boolean = true; export const enableInfiniteRenderLoopDetection: boolean = false; -export const enableInfiniteRenderLoopDetectionForceThrow: boolean = false; export const enableLegacyCache: boolean = false; export const enableLegacyFBSupport: boolean = false; export const enableLegacyHidden: boolean = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index 3c452162ab98..d37dff7f34e9 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -31,7 +31,6 @@ export const enableCreateEventHandleAPI: boolean = false; export const enableMoveBefore: boolean = true; export const enableFizzExternalRuntime: boolean = true; export const enableInfiniteRenderLoopDetection: boolean = false; -export const enableInfiniteRenderLoopDetectionForceThrow: boolean = false; export const enableLegacyCache: boolean = false; export const enableLegacyFBSupport: boolean = false; export const enableLegacyHidden: boolean = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 309de96b4951..537448d3904a 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -52,7 +52,6 @@ export const passChildrenWhenCloningPersistedNodes: boolean = false; export const disableClientCache: boolean = true; export const enableInfiniteRenderLoopDetection: boolean = false; -export const enableInfiniteRenderLoopDetectionForceThrow: boolean = false; export const enableEagerAlternateStateNodeCleanup: boolean = true; export const enableEffectEventMutationPhase: boolean = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index af8dd955d0ba..8d97f9dd9ddd 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -26,7 +26,6 @@ export const enableCreateEventHandleAPI = false; export const enableMoveBefore = false; export const enableFizzExternalRuntime = true; export const enableInfiniteRenderLoopDetection = false; -export const enableInfiniteRenderLoopDetectionForceThrow = false; export const enableLegacyCache = false; export const enableLegacyFBSupport = false; export const enableLegacyHidden = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index e57495ed4e53..53b8b487df3b 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -54,7 +54,6 @@ export const passChildrenWhenCloningPersistedNodes: boolean = false; export const disableClientCache: boolean = true; export const enableInfiniteRenderLoopDetection: boolean = false; -export const enableInfiniteRenderLoopDetectionForceThrow: boolean = false; export const enableReactTestRendererWarning: boolean = false; export const disableLegacyMode: boolean = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index d3340f6eb940..e19b3314c19b 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -27,7 +27,6 @@ export const transitionLaneExpirationMs = 5000; export const enableSchedulingProfiler: boolean = __VARIANT__; export const enableInfiniteRenderLoopDetection: boolean = __VARIANT__; -export const enableInfiniteRenderLoopDetectionForceThrow: boolean = __VARIANT__; export const enableFastAddPropertiesInDiffing: boolean = __VARIANT__; export const enableSuspenseyImages: boolean = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 062cbaa4268a..740a3f3f845b 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -20,7 +20,6 @@ export const { disableSchedulerTimeoutInWorkLoop, enableEffectEventMutationPhase, enableInfiniteRenderLoopDetection, - enableInfiniteRenderLoopDetectionForceThrow, enableNoCloningMemoCache, enableObjectFiber, enableRetryLaneExpiration,