fix: complete partial hook chain on unwind to prevent corruption (#33580)#35717
fix: complete partial hook chain on unwind to prevent corruption (#33580)#35717DukeDeSouth wants to merge 1 commit intofacebook:mainfrom
Conversation
…ebook#33580) When a component suspends mid-render (e.g., after `useState` but before `useMemo`), the work-in-progress fiber has an incomplete hook chain. If this fiber is later committed as part of a Suspense fallback, the incomplete chain replaces the current fiber's complete chain. Subsequent renders then fail with "Rendered more hooks than during the previous render" because the current fiber no longer has entries for hooks after the interruption point. This fixes the issue by completing the work-in-progress hook chain in `resetHooksOnUnwind`: any remaining hooks from the current fiber are cloned and appended so that the chain is always complete when committed. The bug is triggered by a specific combination of conditions: 1. Server-side rendering with hydration 2. An ErrorBoundary inside a Suspense boundary (causes ForceClientRender) 3. A cascading transition update (`startTransition` + `setState`) 4. Conditional `use(thenable)` that causes suspension 5. Additional hooks (`useMemo`) after the `use()` call Fixes facebook#33580 Co-authored-by: Cursor <cursoragent@cursor.com>
|
Hi @DukeDeSouth! 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! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
Human View
Summary
Fixes #33580 — "Rendered more hooks than during the previous render" when a component calls
use(thenable)conditionally after hydration.Root Cause
When a component suspends mid-render (e.g., after
useStatebut beforeuseMemo),resetHooksOnUnwindis called and the work-in-progress (WIP) fiber retains only the hooks that were processed before the interruption. If this WIP fiber is later committed — for example, as part of a Suspense boundary showing its fallback — the incomplete hook chain replaces the current fiber's complete chain.On the next render,
updateWorkInProgressHooktries to clone hooks from the (now-corrupted) current fiber but runs out of entries, throwing:The Fix
In
resetHooksOnUnwind, before clearingcurrentHookandworkInProgressHook, we now clone the remaining unprocessed hooks from the current fiber and append them to the WIP hook chain. This ensures the chain is always complete when committed, preserving hook integrity across Suspense fallback commits.Reproduction Conditions
The bug requires a specific combination of 6 interacting conditions:
hydrateRoot)ForceClientRenderduring hydrationsetStateinuseEffecttriggers re-renderstartTransition(() => setPromise(...))at lower priorityuse(thenable)—promise ? use(promise) : promisecauses suspension when promise becomes non-nulluse()—useMemo(or any hook) comes after the conditionaluse()callSequence of Events
Test Plan
ReactDOMFizzShellHydration-test.jsthat reproduces the exact conditionsReactDOMFizzShellHydration-test.js— 11/11 ✅ReactHooks-test.internal.js— 72/72 ✅ReactSuspenseWithNoopRenderer-test.js— 69/69 ✅ReactUse-test.js— 48/48 ✅ReactHooksWithNoopRenderer-test.js— 96/96 ✅Related
use#34068 — Previous fix attempt (closed by stale bot, author was not confident in the approach)AI View (DCCE Protocol v1.0)
Metadata
AI Contribution Summary
Verification Steps Performed
Human Review Guidance
ReactDOMFizzShellHydration-test.js,ReactHooks-test.internal.js,ReactSuspenseWithNoopRenderer-test.jsMade with M7 Cursor