Fix: Suspend while recovering from hydration error (#28800)
Fixes a bug that happens when an error occurs during hydration, React switches to client rendering, and then the client render suspends. It works correctly if there's a Suspense boundary on the stack, but not if it happens in the shell of the app. Prior to this fix, the app would crash with an "Unknown root exit status" error. I left a TODO comment for how we might refactor this code to be less confusing in the future.
Andrew Clark committed
Apr 9, 2024 at 17:11 UTC
3f9e237a2feb74f1fca23b76d9d2e9e1713e2ba1
2 files changed
+91
-3
packages/react-dom/src/__tests__/ReactDOMFizzShellHydration-test.js
+75
@@ -548,4 +548,79 @@ describe('ReactDOMFizzShellHydration', () => {
548
]);
549
expect(container.textContent).toBe('Hello world');
550
});
551
+
552
+ it(
553
+ 'handles suspending while recovering from a hydration error (in the ' +
554
+ 'shell, no Suspense boundary)',
555
+ async () => {
556
+ const useSyncExternalStore = React.useSyncExternalStore;
557
+
558
+ let isClient = false;
559
+
560
+ let resolve;
561
+ const clientPromise = new Promise(res => {
562
+ resolve = res;
563
+ });
564
+
565
+ function App() {
566
+ const state = useSyncExternalStore(
567
+ function subscribe() {
568
+ return () => {};
569
+ },
570
+ function getSnapshot() {
571
+ return 'Client';
572
+ },
573
+ function getServerSnapshot() {
574
+ const isHydrating = isClient;
575
+ if (isHydrating) {
576
+ // This triggers an error during hydration
577
+ throw new Error('Oops!');
578
+ }
579
+ return 'Server';
580
+ },
581
+ );
582
+
583
+ if (state === 'Client') {
584
+ return React.use(clientPromise);
585
+ }
586
+
587
+ return state;
588
+ }
589
+
590
+ // Server render
591
+ await serverAct(async () => {
592
+ const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
593
+ pipe(writable);
594
+ });
595
+ assertLog([]);
596
+
597
+ expect(container.innerHTML).toBe('Server');
598
+
599
+ // During hydration, an error is thrown. React attempts to recover by
600
+ // switching to client render
601
+ isClient = true;
602
+ await clientAct(async () => {
603
+ ReactDOMClient.hydrateRoot(container, <App />, {
604
+ onRecoverableError(error) {
605
+ Scheduler.log('onRecoverableError: ' + error.message);
606
+ if (error.cause) {
607
+ Scheduler.log('Cause: ' + error.cause.message);
608
+ }
609
+ },
610
+ });
611
+ });
612
+ expect(container.innerHTML).toBe('Server'); // Still suspended
613
+ assertLog([]);
614
+
615
+ await clientAct(async () => {
616
+ resolve('Client');
617
+ });
618
+ assertLog([
619
+ 'onRecoverableError: There was an error while hydrating but React was ' +
620
+ 'able to recover by instead client rendering the entire root.',
621
+ 'Cause: Oops!',
622
+ ]);
623
+ expect(container.innerHTML).toBe('Client');
624
+ },
625
+ );
626
});
packages/react-reconciler/src/ReactFiberWorkLoop.js
+16
-3
@@ -931,19 +931,32 @@ export function performConcurrentWorkOnRoot(
931
932
// Check if something threw
933
if (exitStatus === RootErrored) {
934
- const originallyAttemptedLanes = lanes;
934
+ const lanesThatJustErrored = lanes;
935
const errorRetryLanes = getLanesToRetrySynchronouslyOnError(
936
root,
937
- originallyAttemptedLanes,
937
+ lanesThatJustErrored,
938
);
939
if (errorRetryLanes !== NoLanes) {
940
lanes = errorRetryLanes;
941
exitStatus = recoverFromConcurrentError(
942
root,
943
- originallyAttemptedLanes,
943
+ lanesThatJustErrored,
944
errorRetryLanes,
945
);
946
renderWasConcurrent = false;
947
+ // Need to check the exit status again.
948
+ if (exitStatus !== RootErrored) {
949
+ // The root did not error this time. Restart the exit algorithm
950
+ // from the beginning.
951
+ // TODO: Refactor the exit algorithm to be less confusing. Maybe
952
+ // more branches + recursion instead of a loop. I think the only
953
+ // thing that causes it to be a loop is the RootDidNotComplete
954
+ // check. If that's true, then we don't need a loop/recursion
955
+ // at all.
956
+ continue;
957
+ } else {
958
+ // The root errored yet again. Proceed to commit the tree.
959
+ }
960
}
961
}
962
if (exitStatus === RootFatalErrored) {