Fix: Multiple hydration errors in same render (facebook#23273)

I made a minor mistake in the original onRecoverableError PR that only surfaces if there are hydration errors in two different Suspense boundaries in the same render. This fixes it and adds a unit test.
zhengjitf · Apr 15, 2022 · acb8826 · acb8826
1 parent f804fbf
commit acb8826
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 6 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
@@ -2191,4 +2191,68 @@ describe('ReactDOMFizzServer', () => {
     // UI looks normal
     expect(container.textContent).toEqual('AB');
   });
+
+  // @gate experimental
+  it('logs multiple hydration errors in the same render', async () => {
+    let isClient = false;
+
+    function subscribe() {
+      return () => {};
+    }
+    function getClientSnapshot() {
+      return 'Yay!';
+    }
+    function getServerSnapshot() {
+      if (isClient) {
+        throw new Error('Hydration error');
+      }
+      return 'Yay!';
+    }
+
+    function Child({label}) {
+      // This will throw during client hydration. Only reason to use
+      // useSyncExternalStore in this test is because getServerSnapshot has the
+      // ability to observe whether we're hydrating.
+      useSyncExternalStore(subscribe, getClientSnapshot, getServerSnapshot);
+      Scheduler.unstable_yieldValue(label);
+      return label;
+    }
+
+    function App() {
+      return (
+        <>
+          <Suspense fallback="Loading...">
+            <Child label="A" />
+          </Suspense>
+          <Suspense fallback="Loading...">
+            <Child label="B" />
+          </Suspense>
+        </>
+      );
+    }
+
+    await act(async () => {
+      const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
+      pipe(writable);
+    });
+    expect(Scheduler).toHaveYielded(['A', 'B']);
+
+    // Hydrate the tree. Child will throw during hydration, but not when it
+    // falls back to client rendering.
+    isClient = true;
+    ReactDOM.hydrateRoot(container, <App />, {
+      onRecoverableError(error) {
+        Scheduler.unstable_yieldValue(
+          'Logged recoverable error: ' + error.message,
+        );
+      },
+    });
+
+    expect(Scheduler).toFlushAndYield([
+      'A',
+      'B',
+      'Logged recoverable error: Hydration error',
+      'Logged recoverable error: Hydration error',
+    ]);
+  });
 });
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -920,11 +920,11 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
 }
 
 export function queueRecoverableErrors(errors: Array<mixed>) {
-  if (workInProgressRootConcurrentErrors === null) {
+  if (workInProgressRootRecoverableErrors === null) {
     workInProgressRootRecoverableErrors = errors;
   } else {
-    workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply(
-      workInProgressRootConcurrentErrors,
+    workInProgressRootRecoverableErrors.push.apply(
+      workInProgressRootRecoverableErrors,
       errors,
     );
   }

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -920,11 +920,11 @@ function recoverFromConcurrentError(root, errorRetryLanes) {
 }
 
 export function queueRecoverableErrors(errors: Array<mixed>) {
-  if (workInProgressRootConcurrentErrors === null) {
+  if (workInProgressRootRecoverableErrors === null) {
     workInProgressRootRecoverableErrors = errors;
   } else {
-    workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply(
-      workInProgressRootConcurrentErrors,
+    workInProgressRootRecoverableErrors.push.apply(
+      workInProgressRootRecoverableErrors,
       errors,
     );
   }