From b190c870ffeb72cc5fcf893c407c9595c3ed1d68 Mon Sep 17 00:00:00 2001 From: Arunanshu Biswas Date: Tue, 31 Mar 2026 23:27:09 +0530 Subject: [PATCH] fix: complete hook list after render phase updates have been cleaned up --- .../ReactDOMFizzShellHydration-test.js | 180 ++++++++++++++++++ .../react-reconciler/src/ReactFiberHooks.js | 47 +++++ .../ReactHooksWithNoopRenderer-test.js | 55 ++++++ 3 files changed, 282 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js index fb2f33a4bade..22479ee0ec29 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js @@ -101,6 +101,56 @@ describe('ReactDOMFizzShellHydration', () => { } } + async function hydrateRootAndCollectErrors(reactNode) { + const errors = []; + await clientAct(async () => { + ReactDOMClient.hydrateRoot(container, reactNode, { + onCaughtError(error) { + Scheduler.log('onCaughtError: ' + error.message); + errors.push('caught: ' + error.message); + }, + onUncaughtError(error) { + Scheduler.log('onUncaughtError: ' + error.message); + errors.push('uncaught: ' + error.message); + }, + onRecoverableError(error) { + Scheduler.log('onRecoverableError: ' + error.message); + errors.push('recoverable: ' + error.message); + }, + }); + }); + return errors; + } + + function createErrorBoundaryAndBomb() { + class ErrorBoundary extends React.Component { + constructor(props) { + super(props); + this.state = {error: null}; + } + + static getDerivedStateFromError(error) { + return {error}; + } + + componentDidCatch() {} + + render() { + if (this.state.error) { + return 'Something went wrong: ' + this.state.error.message; + } + + return this.props.children; + } + } + + function Bomb() { + throw new Error('boom'); + } + + return {ErrorBoundary, Bomb}; + } + function resolveText(text) { const record = textCache.get(text); if (record === undefined) { @@ -655,4 +705,134 @@ describe('ReactDOMFizzShellHydration', () => { expect(container.innerHTML).toBe('Client'); }, ); + + it( + 'does not corrupt hooks during hydration when conditional use suspends ' + + 'after a cascading update (#33580)', + async () => { + const {ErrorBoundary, Bomb} = createErrorBoundaryAndBomb(); + + function Updater({setPromise}) { + const [state, setState] = React.useState(false); + + React.useEffect(() => { + setState(true); + startTransition(() => { + setPromise(Promise.resolve('resolved')); + }); + }, [state]); + + return null; + } + + function Page() { + const [promise, setPromise] = React.useState(null); + const value = promise ? React.use(promise) : promise; + + React.useMemo(() => {}, []); + + return ( + <> + + + + + + + {value !== null ? value : 'hello world'} + + ); + } + + function App() { + return ; + } + + await serverAct(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(, { + onError(error) { + Scheduler.log('onError: ' + error.message); + }, + }); + pipe(writable); + }); + assertLog(['onError: boom']); + + const errors = await hydrateRootAndCollectErrors(); + assertLog(['onCaughtError: boom']); + + expect( + errors.find(error => error.includes('Rendered more hooks')), + ).toBeUndefined(); + expect(container.textContent).toBe('Something went wrong: boomresolved'); + }, + ); + + it('preserves hooks when suspension happens before the first tracked hook', async () => { + const {ErrorBoundary, Bomb} = createErrorBoundaryAndBomb(); + let setReady; + + function Updater({setPromise}) { + React.useEffect(() => { + setReady(true); + startTransition(() => { + setPromise(Promise.resolve('resolved')); + }); + }, []); + + return null; + } + + function Page({promise}) { + const value = promise ? React.use(promise) : promise; + + const [ready, _setReady] = React.useState(false); + setReady = _setReady; + + React.useMemo(() => {}, []); + + return ( + <> + + + + + + {ready ? 'ready' : 'not-ready'} + {value !== null ? value : 'hello world'} + + ); + } + + function App() { + const [promise, setPromise] = React.useState(null); + + return ( + <> + + + + ); + } + + await serverAct(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(, { + onError(error) { + Scheduler.log('onError: ' + error.message); + }, + }); + pipe(writable); + }); + assertLog(['onError: boom']); + + const errors = await hydrateRootAndCollectErrors(); + assertLog(['onCaughtError: boom']); + + expect( + errors.find(error => error.includes('Rendered more hooks')), + ).toBeUndefined(); + expect(container.textContent).toBe( + 'Something went wrong: boomreadyresolved', + ); + }); }); diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 29c83c7d7263..48408a55e927 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -957,6 +957,53 @@ export function resetHooksOnUnwind(workInProgress: Fiber): void { didScheduleRenderPhaseUpdate = false; } + // If a render unwinds after processing only part of the hook list, the + // work-in-progress fiber is left with a truncated chain. If that truncated + // chain is later committed, it replaces the current fiber's complete list and + // subsequent renders can remount later hooks or throw because their entries + // are missing. + // + // Complete the hook list after render phase updates are cleaned up: + // - If some tracked hooks were processed, append the remaining current hooks. + // - If we suspended before the first tracked hook, clone the entire current + // list so the next render still reuses the existing hooks. + const current = workInProgress.alternate; + if (current !== null) { + let nextCurrentHook: Hook | null; + let tail: Hook | null; + + if (workInProgressHook !== null) { + nextCurrentHook = currentHook !== null ? currentHook.next : null; + tail = workInProgressHook; + } else if (workInProgress.memoizedState === null) { + nextCurrentHook = current.memoizedState; + tail = null; + } else { + nextCurrentHook = null; + tail = null; + } + + while (nextCurrentHook !== null) { + const clone: Hook = { + memoizedState: nextCurrentHook.memoizedState, + + baseState: nextCurrentHook.baseState, + baseQueue: nextCurrentHook.baseQueue, + queue: nextCurrentHook.queue, + + next: null, + }; + + if (tail === null) { + workInProgress.memoizedState = tail = clone; + } else { + tail = tail.next = clone; + } + + nextCurrentHook = nextCurrentHook.next; + } + } + renderLanes = NoLanes; currentlyRenderingFiber = (null: any); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index c45db0096517..f48a78de4afb 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -732,6 +732,61 @@ describe('ReactHooksWithNoopRenderer', () => { }); }); + it( + 'preserves pending updates on later hooks that were not processed ' + + 'before unwind', + async () => { + const thenable = {then() {}}; + + let setLabel; + function Foo({suspend}) { + return ( + + + + ); + } + + function Bar({suspend}) { + const [counter, setCounter] = useState(0); + + if (suspend) { + setCounter(c => c + 1); + Scheduler.log('Suspend!'); + throw thenable; + } + + const [label, _setLabel] = useState('A'); + setLabel = _setLabel; + + return ; + } + + const root = ReactNoop.createRoot(); + root.render(); + + await waitForAll(['A:0']); + expect(root).toMatchRenderedOutput(); + + await act(async () => { + React.startTransition(() => { + root.render(); + setLabel('B'); + }); + + await waitForAll(['Suspend!']); + expect(root).toMatchRenderedOutput(); + + React.startTransition(() => { + root.render(); + }); + + await waitForAll(['B:0']); + expect(root).toMatchRenderedOutput(); + }); + }, + ); + it('regression: render phase updates cause lower pri work to be dropped', async () => { let setRow; function ScrollView() {