From 64223fed82414ee41839e95ebc97df330b2b71ca Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 10 Feb 2022 08:40:00 -0800 Subject: [PATCH] Fix: Multiple hydration errors in same render (#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. --- .../src/__tests__/ReactDOMFizzServer-test.js | 64 +++++++++++++++++++ .../src/ReactFiberWorkLoop.new.js | 6 +- .../src/ReactFiberWorkLoop.old.js | 6 +- 3 files changed, 70 insertions(+), 6 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 2fddae2d64df2..0782b57ff4b47 100644 --- 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 ( + <> + + + + + + + + ); + } + + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(); + 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, , { + 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 index 33131e20bb632..d9a11759a6bf8 100644 --- 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) { - 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 index 17ee82384d5e6..8cb20434be660 100644 --- 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) { - if (workInProgressRootConcurrentErrors === null) { + if (workInProgressRootRecoverableErrors === null) { workInProgressRootRecoverableErrors = errors; } else { - workInProgressRootConcurrentErrors = workInProgressRootConcurrentErrors.push.apply( - workInProgressRootConcurrentErrors, + workInProgressRootRecoverableErrors.push.apply( + workInProgressRootRecoverableErrors, errors, ); }