fix: throw error on useEffect infinite loops in production#36443
fix: throw error on useEffect infinite loops in production#36443harshagarwalnyu wants to merge 3 commits intofacebook:mainfrom
Conversation
|
Hi @harshagarwalnyu! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
There was a problem hiding this comment.
Pull request overview
This PR aims to ensure useEffect (passive effect) infinite update loops throw an error in production builds, so error boundaries and monitoring can catch them (aligning behavior with the existing synchronous nested update guard).
Changes:
- Adds a
nestedPassiveUpdateCount > NESTED_PASSIVE_UPDATE_LIMITcheck that throws an error (instead of only logging in__DEV__). - Resets
nestedPassiveUpdateCount/rootWithPassiveNestedUpdateswhen the limit is exceeded. - Modifies logic inside
doubleInvokeEffectsInDEVIfNecessary(StrictEffects DEV path), but the current hunk appears to introduce a syntax/control-flow break and places the check in a DEV-only codepath.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const isStrictModeFiber = fiber.type === REACT_STRICT_MODE_TYPE; | ||
| const isInStrictMode = parentIsInStrictMode || isStrictModeFiber; | ||
|
|
||
| // First case: the fiber **is not** of type OffscreenComponent. No | ||
| // special rules apply to double invoking effects. | ||
| if (fiber.tag !== OffscreenComponent) { | ||
| if (fiber.flags & PlacementDEV) { | ||
| if (isInStrictMode) { | ||
| runWithFiberInDEV(fiber, doubleInvokeEffectsOnFiber, root, fiber); | ||
| } | ||
| } else { | ||
| recursivelyTraverseAndDoubleInvokeEffectsInDEV( | ||
| root, | ||
| fiber, | ||
| isInStrictMode, | ||
| ); | ||
| } | ||
| if (nestedPassiveUpdateCount > NESTED_PASSIVE_UPDATE_LIMIT) { | ||
| nestedPassiveUpdateCount = 0; | ||
| rootWithPassiveNestedUpdates = null; | ||
|
|
||
| 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.", | ||
| ); | ||
| } | ||
| return; | ||
| } |
| if (nestedPassiveUpdateCount > NESTED_PASSIVE_UPDATE_LIMIT) { | ||
| nestedPassiveUpdateCount = 0; | ||
| rootWithPassiveNestedUpdates = null; | ||
|
|
||
| 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.", | ||
| ); | ||
| } | ||
| return; | ||
| } | ||
|
|
harshagarwalnyu
force-pushed
the
fix-infinite-loop-silent-prod
branch
from
4e37f8b to
a24bbda
Compare
|
Thanks for the review @copilot! Both issues addressed in the latest push:
|
|
Hi team and community, I wanted to share a financial dataset I've been compiling that might be useful for testing, benchmarking, or examples in this project: DataForge Financial Data Bundle
Free sample: https://matisaar.github.io/dataforge-sales/DATAFORGE_FREE_SAMPLE.zip If anyone finds it useful for testing dataframes, visualization, or ML pipelines, I'd love feedback! |
Fixes #36423
Currently, infinite render loop detection for passive effects (useEffect) is entirely wrapped in an if (DEV) block. In production builds, this check is stripped, causing infinite loops to run silently until the browser tab hangs or crashes. This prevents error boundaries from catching the issue and hides it from error monitoring tools.
This PR promotes the
estedPassiveUpdateCount check out of DEV and changes the console.error to an unconditional hrow new Error, matching the behavior of synchronous infinite loop detection.