From 79ed5e18fd5dba4148e3ca73da53fd6eef037209 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 17 Feb 2022 09:24:03 -0600 Subject: [PATCH] Delete vestigial RetryAfterError logic (#23312) This code was originally added to force a client render after receiving an error during hydration. Later we added the ForceClientRender to implement the same behavior, but scoped to an individual Suspense boundary instead of all the boundaries in the entire root. So it's now redudant. We had some test coverage already but I added another test specifically for the case of throwing a recoverable hydration error in the shell. --- .../src/__tests__/ReactDOMFizzServer-test.js | 77 +++++++++++++++++++ .../src/ReactFiberBeginWork.new.js | 11 --- .../src/ReactFiberBeginWork.old.js | 11 --- .../src/ReactFiberWorkLoop.new.js | 15 +--- .../src/ReactFiberWorkLoop.old.js | 15 +--- 5 files changed, 85 insertions(+), 44 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 0782b57ff4b47..1ecab933035f8 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -1864,6 +1864,83 @@ describe('ReactDOMFizzServer', () => { } }); + // @gate experimental + it( + 'errors during hydration in the shell force a client render at the ' + + 'root, and during the client render it recovers', + async () => { + let isClient = false; + + function subscribe() { + return () => {}; + } + function getClientSnapshot() { + return 'Yay!'; + } + + // At the time of writing, the only API that exposes whether it's currently + // hydrating is the `getServerSnapshot` API, so I'm using that here to + // simulate an error during hydration. + function getServerSnapshot() { + if (isClient) { + throw new Error('Hydration error'); + } + return 'Yay!'; + } + + function Child() { + const value = useSyncExternalStore( + subscribe, + getClientSnapshot, + getServerSnapshot, + ); + Scheduler.unstable_yieldValue(value); + return value; + } + + const spanRef = React.createRef(); + + function App() { + return ( + + + + ); + } + + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(); + pipe(writable); + }); + expect(Scheduler).toHaveYielded(['Yay!']); + + const span = container.getElementsByTagName('span')[0]; + + // 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(error.message); + }, + }); + + // An error logged but instead of surfacing it to the UI, we switched + // to client rendering. + expect(() => { + expect(Scheduler).toFlushAndYield(['Yay!', 'Hydration error']); + }).toErrorDev( + 'An error occurred during hydration. The server HTML was replaced', + {withoutStack: true}, + ); + expect(getVisibleChildren(container)).toEqual(Yay!); + + // The node that's inside the boundary that errored during hydration was + // not hydrated. + expect(spanRef.current).not.toBe(span); + }, + ); + // @gate experimental it( 'errors during hydration force a client render at the nearest Suspense ' + diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 521a75d11dcb7..f0d7b6c625cc6 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -225,9 +225,6 @@ import { markSkippedUpdateLanes, getWorkInProgressRoot, pushRenderLanes, - getExecutionContext, - RetryAfterError, - NoContext, } from './ReactFiberWorkLoop.new'; import {setWorkInProgressVersion} from './ReactMutableSource.new'; import { @@ -2646,14 +2643,6 @@ function updateDehydratedSuspenseComponent( // but after we've already committed once. warnIfHydrating(); - if ((getExecutionContext() & RetryAfterError) !== NoContext) { - return retrySuspenseComponentWithoutHydrating( - current, - workInProgress, - renderLanes, - ); - } - if ((workInProgress.mode & ConcurrentMode) === NoMode) { return retrySuspenseComponentWithoutHydrating( current, diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 471bb11d941da..b562291850ea5 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -225,9 +225,6 @@ import { markSkippedUpdateLanes, getWorkInProgressRoot, pushRenderLanes, - getExecutionContext, - RetryAfterError, - NoContext, } from './ReactFiberWorkLoop.old'; import {setWorkInProgressVersion} from './ReactMutableSource.old'; import { @@ -2646,14 +2643,6 @@ function updateDehydratedSuspenseComponent( // but after we've already committed once. warnIfHydrating(); - if ((getExecutionContext() & RetryAfterError) !== NoContext) { - return retrySuspenseComponentWithoutHydrating( - current, - workInProgress, - renderLanes, - ); - } - if ((workInProgress.mode & ConcurrentMode) === NoMode) { return retrySuspenseComponentWithoutHydrating( current, diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 4a1880e0cf848..12bc721c6d399 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -245,11 +245,10 @@ const { type ExecutionContext = number; -export const NoContext = /* */ 0b0000; -const BatchedContext = /* */ 0b0001; -const RenderContext = /* */ 0b0010; -const CommitContext = /* */ 0b0100; -export const RetryAfterError = /* */ 0b1000; +export const NoContext = /* */ 0b000; +const BatchedContext = /* */ 0b001; +const RenderContext = /* */ 0b010; +const CommitContext = /* */ 0b100; type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5 | 6; const RootInProgress = 0; @@ -945,9 +944,6 @@ function performConcurrentWorkOnRoot(root, didTimeout) { } function recoverFromConcurrentError(root, errorRetryLanes) { - const prevExecutionContext = executionContext; - executionContext |= RetryAfterError; - // If an error occurred during hydration, discard server response and fall // back to client side render. if (root.isDehydrated) { @@ -970,9 +966,6 @@ function recoverFromConcurrentError(root, errorRetryLanes) { } else { // The UI failed to recover. } - - executionContext = prevExecutionContext; - return exitStatus; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index b9fada5bd00b0..9e37f41f4fc82 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -245,11 +245,10 @@ const { type ExecutionContext = number; -export const NoContext = /* */ 0b0000; -const BatchedContext = /* */ 0b0001; -const RenderContext = /* */ 0b0010; -const CommitContext = /* */ 0b0100; -export const RetryAfterError = /* */ 0b1000; +export const NoContext = /* */ 0b000; +const BatchedContext = /* */ 0b001; +const RenderContext = /* */ 0b010; +const CommitContext = /* */ 0b100; type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5 | 6; const RootInProgress = 0; @@ -945,9 +944,6 @@ function performConcurrentWorkOnRoot(root, didTimeout) { } function recoverFromConcurrentError(root, errorRetryLanes) { - const prevExecutionContext = executionContext; - executionContext |= RetryAfterError; - // If an error occurred during hydration, discard server response and fall // back to client side render. if (root.isDehydrated) { @@ -970,9 +966,6 @@ function recoverFromConcurrentError(root, errorRetryLanes) { } else { // The UI failed to recover. } - - executionContext = prevExecutionContext; - return exitStatus; }