Fix: Treat incomplete tree as an error during recovery (#36911)
In `recoverFromConcurrentError`, there's logic to determine if the latest render attempt resulted in another error or if it completed successfully, by checking whether the exit status matches RootErrored. The logic was incomplete because RootSuspendedAtTheShell is also considered an errored state in this context. This can cause an incomplete tree (i.e. one that unwinds without entering the complete phase) to be mistaken for a complete one, leading to confusing errors. An incomplete tree can never be committed because it's not guaranteed to represent a coherent state. An example of how this can manifest as a bug: in #33580, an error caught by an error boundary triggers a synchronous recovery render. During that render a parent component suspends on `use(thenable)` with no Suspense boundary above it, so the tree unwinds to the shell instead of completing. Because the incomplete tree is mistaken for a recovered one and committed, the parent fiber becomes current with a truncated hook list — only the hooks it rendered before suspending. On the next render it calls the rest of its hooks and throws "Rendered more hooks than during the previous render." Fixes #33580. Co-authored-by: Arunanshu Biswas <48434243+arunanshub@users.noreply.github.com> Co-authored-by: Arunanshu Biswas <48434243+arunanshub@users.noreply.github.com>
Andrew Clark committed
Jun 30, 2026 at 15:38 UTC
c3555f0ca2648380ccd3d6af23479610e72f6bf1
3 files changed
+241
-1
packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js
+180
@@ -101,6 +101,56 @@ describe('ReactDOMFizzShellHydration', () => {
101
}
102
}
103
104
+ async function hydrateRootAndCollectErrors(reactNode) {
105
+ const errors = [];
106
+ await clientAct(async () => {
107
+ ReactDOMClient.hydrateRoot(container, reactNode, {
108
+ onCaughtError(error) {
109
+ Scheduler.log('onCaughtError: ' + error.message);
110
+ errors.push('caught: ' + error.message);
111
+ },
112
+ onUncaughtError(error) {
113
+ Scheduler.log('onUncaughtError: ' + error.message);
114
+ errors.push('uncaught: ' + error.message);
115
+ },
116
+ onRecoverableError(error) {
117
+ Scheduler.log('onRecoverableError: ' + error.message);
118
+ errors.push('recoverable: ' + error.message);
119
+ },
120
+ });
121
+ });
122
+ return errors;
123
+ }
124
+
125
+ function createErrorBoundaryAndBomb() {
126
+ class ErrorBoundary extends React.Component {
127
+ constructor(props) {
128
+ super(props);
129
+ this.state = {error: null};
130
+ }
131
+
132
+ static getDerivedStateFromError(error) {
133
+ return {error};
134
+ }
135
+
136
+ componentDidCatch() {}
137
+
138
+ render() {
139
+ if (this.state.error) {
140
+ return 'Something went wrong: ' + this.state.error.message;
141
+ }
142
+
143
+ return this.props.children;
144
+ }
145
+ }
146
+
147
+ function Bomb() {
148
+ throw new Error('boom');
149
+ }
150
+
151
+ return {ErrorBoundary, Bomb};
152
+ }
153
+
154
function resolveText(text) {
155
const record = textCache.get(text);
156
if (record === undefined) {
@@ -655,4 +705,134 @@ describe('ReactDOMFizzShellHydration', () => {
705
expect(container.innerHTML).toBe('Client');
706
},
707
);
708
+
709
+ it(
710
+ 'does not corrupt hooks during hydration when conditional use suspends ' +
711
+ 'after a cascading update (#33580)',
712
+ async () => {
713
+ const {ErrorBoundary, Bomb} = createErrorBoundaryAndBomb();
714
+
715
+ function Updater({setPromise}) {
716
+ const [state, setState] = React.useState(false);
717
+
718
+ React.useEffect(() => {
719
+ setState(true);
720
+ startTransition(() => {
721
+ setPromise(Promise.resolve('resolved'));
722
+ });
723
+ }, [state]);
724
+
725
+ return null;
726
+ }
727
+
728
+ function Page() {
729
+ const [promise, setPromise] = React.useState(null);
730
+ const value = promise ? React.use(promise) : promise;
731
+
732
+ React.useMemo(() => {}, []);
733
+
734
+ return (
735
+ <>
736
+ <Updater setPromise={setPromise} />
737
+ <React.Suspense fallback="Loading...">
738
+ <ErrorBoundary>
739
+ <Bomb />
740
+ </ErrorBoundary>
741
+ </React.Suspense>
742
+ {value !== null ? value : 'hello world'}
743
+ </>
744
+ );
745
+ }
746
+
747
+ function App() {
748
+ return <Page />;
749
+ }
750
+
751
+ await serverAct(async () => {
752
+ const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />, {
753
+ onError(error) {
754
+ Scheduler.log('onError: ' + error.message);
755
+ },
756
+ });
757
+ pipe(writable);
758
+ });
759
+ assertLog(['onError: boom']);
760
+
761
+ const errors = await hydrateRootAndCollectErrors(<App />);
762
+ assertLog(['onCaughtError: boom']);
763
+
764
+ expect(
765
+ errors.find(error => error.includes('Rendered more hooks')),
766
+ ).toBeUndefined();
767
+ expect(container.textContent).toBe('Something went wrong: boomresolved');
768
+ },
769
+ );
770
+
771
+ it('preserves hooks when suspension happens before the first tracked hook', async () => {
772
+ const {ErrorBoundary, Bomb} = createErrorBoundaryAndBomb();
773
+ let setReady;
774
+
775
+ function Updater({setPromise}) {
776
+ React.useEffect(() => {
777
+ setReady(true);
778
+ startTransition(() => {
779
+ setPromise(Promise.resolve('resolved'));
780
+ });
781
+ }, []);
782
+
783
+ return null;
784
+ }
785
+
786
+ function Page({promise}) {
787
+ const value = promise ? React.use(promise) : promise;
788
+
789
+ const [ready, _setReady] = React.useState(false);
790
+ setReady = _setReady;
791
+
792
+ React.useMemo(() => {}, []);
793
+
794
+ return (
795
+ <>
796
+ <React.Suspense fallback="Loading...">
797
+ <ErrorBoundary>
798
+ <Bomb />
799
+ </ErrorBoundary>
800
+ </React.Suspense>
801
+ <span>{ready ? 'ready' : 'not-ready'}</span>
802
+ <span>{value !== null ? value : 'hello world'}</span>
803
+ </>
804
+ );
805
+ }
806
+
807
+ function App() {
808
+ const [promise, setPromise] = React.useState(null);
809
+
810
+ return (
811
+ <>
812
+ <Updater setPromise={setPromise} />
813
+ <Page promise={promise} />
814
+ </>
815
+ );
816
+ }
817
+
818
+ await serverAct(async () => {
819
+ const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />, {
820
+ onError(error) {
821
+ Scheduler.log('onError: ' + error.message);
822
+ },
823
+ });
824
+ pipe(writable);
825
+ });
826
+ assertLog(['onError: boom']);
827
+
828
+ const errors = await hydrateRootAndCollectErrors(<App />);
829
+ assertLog(['onCaughtError: boom']);
830
+
831
+ expect(
832
+ errors.find(error => error.includes('Rendered more hooks')),
833
+ ).toBeUndefined();
834
+ expect(container.textContent).toBe(
835
+ 'Something went wrong: boomreadyresolved',
836
+ );
837
+ });
838
});
packages/react-reconciler/src/ReactFiberWorkLoop.js
+6
-1
@@ -1341,7 +1341,12 @@ function recoverFromConcurrentError(
1341
}
1342
1343
const exitStatus = renderRootSync(root, errorRetryLanes, false);
1344
- if (exitStatus !== RootErrored) {
1344
+ // A status of RootSuspendedAtTheShell means the retry unwound to the root
1345
+ // without completing (e.g. something suspended in the shell), so the tree is
1346
+ // incomplete and must not be treated as recovered — committing it would
1347
+ // corrupt the current tree. Fall through and return the status as-is so the
1348
+ // root stays suspended.
1349
+ if (exitStatus !== RootErrored && exitStatus !== RootSuspendedAtTheShell) {
1350
// Successfully finished rendering on retry
1351
1352
if (workInProgressRootDidAttachPingListener && !wasRootDehydrated) {
packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
+55
@@ -732,6 +732,61 @@ describe('ReactHooksWithNoopRenderer', () => {
732
});
733
});
734
735
+ it(
736
+ 'preserves pending updates on later hooks that were not processed ' +
737
+ 'before unwind',
738
+ async () => {
739
+ const thenable = {then() {}};
740
+
741
+ let setLabel;
742
+ function Foo({suspend}) {
743
+ return (
744
+ <Suspense fallback="Loading...">
745
+ <Bar suspend={suspend} />
746
+ </Suspense>
747
+ );
748
+ }
749
+
750
+ function Bar({suspend}) {
751
+ const [counter, setCounter] = useState(0);
752
+
753
+ if (suspend) {
754
+ setCounter(c => c + 1);
755
+ Scheduler.log('Suspend!');
756
+ throw thenable;
757
+ }
758
+
759
+ const [label, _setLabel] = useState('A');
760
+ setLabel = _setLabel;
761
+
762
+ return <Text text={`${label}:${counter}`} />;
763
+ }
764
+
765
+ const root = ReactNoop.createRoot();
766
+ root.render(<Foo suspend={false} />);
767
+
768
+ await waitForAll(['A:0']);
769
+ expect(root).toMatchRenderedOutput(<span prop="A:0" />);
770
+
771
+ await act(async () => {
772
+ React.startTransition(() => {
773
+ root.render(<Foo suspend={true} />);
774
+ setLabel('B');
775
+ });
776
+
777
+ await waitForAll(['Suspend!']);
778
+ expect(root).toMatchRenderedOutput(<span prop="A:0" />);
779
+
780
+ React.startTransition(() => {
781
+ root.render(<Foo suspend={false} />);
782
+ });
783
+
784
+ await waitForAll(['B:0']);
785
+ expect(root).toMatchRenderedOutput(<span prop="B:0" />);
786
+ });
787
+ },
788
+ );
789
+
790
it('regression: render phase updates cause lower pri work to be dropped', async () => {
791
let setRow;
792
function ScrollView() {