From a9ccb8b110b23c9213d857bfa7227e4d1b691908 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Tue, 19 Apr 2022 16:22:36 -0700 Subject: [PATCH 01/14] Add failing test case for #24384 If a components suspends during hydration we expect there to be mismatches with server rendered HTML but we were not always supressing warning messages related to these expected mismatches --- .../src/__tests__/ReactDOMFizzServer-test.js | 90 +++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index bc609efcf0d0..f71dece821ba 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -2841,4 +2841,94 @@ describe('ReactDOMFizzServer', () => { expect(window.__test_outlet).toBe(1); }); }); + + // @gate experimental + it('#24384: Suspending should halt hydration warnings while still allowing siblings to warm up', async () => { + const makeApp = () => { + let resolve, resolved; + const promise = new Promise(r => { + resolve = () => { + resolved = true; + return r(); + }; + }); + function ComponentThatSuspends() { + if (!resolved) { + throw promise; + } + return

A

; + } + + const App = ({text}) => { + return ( +
+ Loading...}> + +

{text}

+
+
+ ); + }; + + return [App, resolve]; + }; + + const [ServerApp, serverResolve] = makeApp(); + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + , + ); + pipe(writable); + }); + await act(() => { + serverResolve(); + }); + + expect(getVisibleChildren(container)).toEqual( +
+

A

+

initial

+
, + ); + + // the client app is rendered with an intentionally incorrect text. The still Suspended component causes + // hydration to fail silently (allowing for cache warming but otherwise skipping this boundary) until it + // resolves. + const [ClientApp, clientResolve] = makeApp(); + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue( + 'Logged recoverable error: ' + error.message, + ); + }, + }); + Scheduler.unstable_flushAll(); + + expect(getVisibleChildren(container)).toEqual( +
+

A

+

initial

+
, + ); + + // Now that the boundary resolves to it's children the hydration completes and discovers that there is a mismatch requiring + // client-side rendering. + await clientResolve(); + expect(() => { + expect(Scheduler).toFlushAndYield([ + 'Logged recoverable error: Text content does not match server-rendered HTML.', + 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.', + ]); + }).toErrorDev( + 'Warning: Prop `name` did not match. Server: "initial" Client: "replaced"', + ); + expect(getVisibleChildren(container)).toEqual( +
+

A

+

replaced

+
, + ); + + expect(Scheduler).toFlushAndYield([]); + }); }); From b530537361f0d80a1e66560690d7135995508534 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Tue, 19 Apr 2022 16:28:07 -0700 Subject: [PATCH 02/14] Mark hydration as suspending on every thrownException previously hydration would only be marked as supsending when a genuine error was thrown. This created an opportunity for a hydration mismatch that would warn after which later hydration mismatches would not lead to warnings. By moving the marker check earlier in the thrownException function we get the hydration context to enter the didSuspend state on both error and thrown promise cases which eliminates this gap. --- packages/react-reconciler/src/ReactFiberThrow.new.js | 6 ++++-- packages/react-reconciler/src/ReactFiberThrow.old.js | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 815cb25ef045..bf8d83e8f868 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -444,6 +444,10 @@ function throwException( } } + if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { + markDidSuspendWhileHydratingDEV(); + } + if ( value !== null && typeof value === 'object' && @@ -514,8 +518,6 @@ function throwException( } else { // This is a regular error, not a Suspense wakeable. if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { - markDidSuspendWhileHydratingDEV(); - const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); // If the error was thrown during hydration, we may be able to recover by // discarding the dehydrated content and switching to a client render. diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index ec89f5ab0cd5..6f7609a4fdeb 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -444,6 +444,10 @@ function throwException( } } + if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { + markDidSuspendWhileHydratingDEV(); + } + if ( value !== null && typeof value === 'object' && @@ -514,8 +518,6 @@ function throwException( } else { // This is a regular error, not a Suspense wakeable. if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { - markDidSuspendWhileHydratingDEV(); - const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); // If the error was thrown during hydration, we may be able to recover by // discarding the dehydrated content and switching to a client render. From dc1cd0b1dc3307401244777495058ba94c759cba Mon Sep 17 00:00:00 2001 From: Josh Story Date: Tue, 19 Apr 2022 16:32:53 -0700 Subject: [PATCH 03/14] Fix failing test related to client render fallbacks This test was actually subject to the project identified in the issue fixed in this branch. After fixing the underlying issue the assertion logic needed to change to pick the right warning which now emits after hydration successfully completes on promise resolution. I changed the container type to 'section' to make the error message slightly easier to read/understand (for me) --- .../ReactDOMServerPartialHydration-test.internal.js | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js index 4fce6008bcf5..fe9d8dd453cd 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js @@ -285,7 +285,7 @@ describe('ReactDOMServerPartialHydration', () => { } try { const finalHTML = ReactDOMServer.renderToString(); - const container = document.createElement('div'); + const container = document.createElement('section'); container.innerHTML = finalHTML; expect(Scheduler).toHaveYielded([ 'Hello', @@ -350,12 +350,14 @@ describe('ReactDOMServerPartialHydration', () => { ); if (__DEV__) { - expect(mockError.mock.calls[0]).toEqual([ + const secondToLastCall = + mockError.mock.calls[mockError.mock.calls.length - 2]; + expect(secondToLastCall).toEqual([ 'Warning: Expected server HTML to contain a matching <%s> in <%s>.%s', - 'div', - 'div', + 'article', + 'section', '\n' + - ' in div (at **)\n' + + ' in article (at **)\n' + ' in Component (at **)\n' + ' in Suspense (at **)\n' + ' in App (at **)', From b06d2e956bacab57c7a9a2a1e4d6835c219701b6 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 20 Apr 2022 08:55:30 -0700 Subject: [PATCH 04/14] Only mark didSuspend on suspense path For unknown reasons the didSuspend was being set only on the error path and nto the suspense path. The original change hoisted this to happen on both paths. This change moves the didSuspend call to the suspense path only. This appears to be a noop because if the error path marked didSuspend it would suppress later warnings but if it does not the warning functions themsevles do that suppression (after the first one which necessarily already happened) --- packages/react-reconciler/src/ReactFiberThrow.new.js | 8 ++++---- packages/react-reconciler/src/ReactFiberThrow.old.js | 4 ++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index bf8d83e8f868..f32fe5849419 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -444,10 +444,6 @@ function throwException( } } - if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { - markDidSuspendWhileHydratingDEV(); - } - if ( value !== null && typeof value === 'object' && @@ -457,6 +453,10 @@ function throwException( const wakeable: Wakeable = (value: any); resetSuspendedComponent(sourceFiber, rootRenderLanes); + if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { + markDidSuspendWhileHydratingDEV(); + } + if (__DEV__) { if (enableDebugTracing) { if (sourceFiber.mode & DebugTracingMode) { diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index 6f7609a4fdeb..9b07ede635ff 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -457,6 +457,10 @@ function throwException( const wakeable: Wakeable = (value: any); resetSuspendedComponent(sourceFiber, rootRenderLanes); + if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { + markDidSuspendWhileHydratingDEV(); + } + if (__DEV__) { if (enableDebugTracing) { if (sourceFiber.mode & DebugTracingMode) { From 5e1cd0a905f825f12df983153222a3514bc9a7f6 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 20 Apr 2022 09:20:20 -0700 Subject: [PATCH 05/14] gate test on hydration fallback flags --- packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index f71dece821ba..ad5abfad18fe 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -2842,7 +2842,7 @@ describe('ReactDOMFizzServer', () => { }); }); - // @gate experimental + // @gate experimental && enableClientRenderFallbackOnTextMismatch && enableClientRenderFallbackOnHydrationMismatch it('#24384: Suspending should halt hydration warnings while still allowing siblings to warm up', async () => { const makeApp = () => { let resolve, resolved; From 668d8c9eb61d38f2ecee5fc25cb1f99fd7974f37 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 20 Apr 2022 10:00:41 -0700 Subject: [PATCH 06/14] refactor didSuspend to didSuspendOrError the orignial behavior applied the hydration warning bailout to error paths only. originally I moved it to Suspense paths only but this commit restores it to both paths and renames the marker function as didThrow rather than didSuspend The logic here is that for either case if we get a mismatch in hydration we want to warm up components but effectively consider the hydration for this boundary halted --- ...actDOMFizzSuppressHydrationWarning-test.js | 179 ++++++++++++++++++ .../src/ReactFiberHydrationContext.new.js | 21 +- .../src/ReactFiberHydrationContext.old.js | 21 +- .../src/ReactFiberThrow.new.js | 9 +- .../src/ReactFiberThrow.old.js | 13 +- 5 files changed, 215 insertions(+), 28 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzSuppressHydrationWarning-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzSuppressHydrationWarning-test.js index ac53da312729..3c86c55f0270 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzSuppressHydrationWarning-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzSuppressHydrationWarning-test.js @@ -12,6 +12,7 @@ let JSDOM; let Stream; let Scheduler; +let Suspense; let React; let ReactDOMClient; let ReactDOMFizzServer; @@ -28,6 +29,7 @@ describe('ReactDOMFizzServerHydrationWarning', () => { JSDOM = require('jsdom').JSDOM; Scheduler = require('scheduler'); React = require('react'); + Suspense = React.Suspense; ReactDOMClient = require('react-dom/client'); if (__EXPERIMENTAL__) { ReactDOMFizzServer = require('react-dom/server'); @@ -58,6 +60,18 @@ describe('ReactDOMFizzServerHydrationWarning', () => { }); }); + function normalizeCodeLocInfo(strOrErr) { + if (strOrErr && strOrErr.replace) { + return strOrErr.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function( + m, + name, + ) { + return '\n in ' + name + ' (at **)'; + }); + } + return strOrErr; + } + async function act(callback) { await callback(); // Await one turn around the event loop. @@ -667,4 +681,169 @@ describe('ReactDOMFizzServerHydrationWarning', () => { , ); }); + + // @gate experimental && enableClientRenderFallbackOnTextMismatch && enableClientRenderFallbackOnHydrationMismatch + it('#24384: Suspending should halt hydration warnings while still allowing siblings to warm up', async () => { + const makeApp = () => { + let resolve, resolved; + const promise = new Promise(r => { + resolve = () => { + resolved = true; + return r(); + }; + }); + function ComponentThatSuspends() { + if (!resolved) { + throw promise; + } + return

A

; + } + + const App = ({text}) => { + return ( +
+ Loading...}> + +

{text}

+
+
+ ); + }; + + return [App, resolve]; + }; + + const [ServerApp, serverResolve] = makeApp(); + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + , + ); + pipe(writable); + }); + await act(() => { + serverResolve(); + }); + + expect(getVisibleChildren(container)).toEqual( +
+

A

+

initial

+
, + ); + + // the client app is rendered with an intentionally incorrect text. The still Suspended component causes + // hydration to fail silently (allowing for cache warming but otherwise skipping this boundary) until it + // resolves. + const [ClientApp, clientResolve] = makeApp(); + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue( + 'Logged recoverable error: ' + error.message, + ); + }, + }); + Scheduler.unstable_flushAll(); + + expect(getVisibleChildren(container)).toEqual( +
+

A

+

initial

+
, + ); + + // Now that the boundary resolves to it's children the hydration completes and discovers that there is a mismatch requiring + // client-side rendering. + await clientResolve(); + expect(() => { + expect(Scheduler).toFlushAndYield([ + 'Logged recoverable error: Text content does not match server-rendered HTML.', + 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.', + ]); + }).toErrorDev( + 'Warning: Prop `name` did not match. Server: "initial" Client: "replaced"', + ); + expect(getVisibleChildren(container)).toEqual( +
+

A

+

replaced

+
, + ); + + expect(Scheduler).toFlushAndYield([]); + }); + + // @gate experimental && enableClientRenderFallbackOnTextMismatch && enableClientRenderFallbackOnHydrationMismatch + it('only warns once on hydration mismatch while within a suspense boundary', async () => { + const originalConsoleError = console.error; + const mockError = jest.fn(); + console.error = (...args) => { + mockError(...args.map(normalizeCodeLocInfo)); + }; + + const App = ({text}) => { + return ( +
+ Loading...}> +

{text}

+

{text}

+

{text}

+
+
+ ); + }; + + try { + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + , + ); + pipe(writable); + }); + + expect(getVisibleChildren(container)).toEqual( +
+

initial

+

initial

+

initial

+
, + ); + + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue( + 'Logged recoverable error: ' + error.message, + ); + }, + }); + expect(Scheduler).toFlushAndYield([ + 'Logged recoverable error: Text content does not match server-rendered HTML.', + 'Logged recoverable error: Text content does not match server-rendered HTML.', + 'Logged recoverable error: Text content does not match server-rendered HTML.', + 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.', + ]); + + expect(getVisibleChildren(container)).toEqual( +
+

replaced

+

replaced

+

replaced

+
, + ); + + expect(Scheduler).toFlushAndYield([]); + expect(mockError.mock.calls.length).toBe(1); + expect(mockError.mock.calls[0]).toEqual([ + 'Warning: Text content did not match. Server: "%s" Client: "%s"%s', + 'initial', + 'replaced', + '\n' + + ' in h2 (at **)\n' + + ' in Suspense (at **)\n' + + ' in div (at **)\n' + + ' in App (at **)', + ]); + } finally { + console.error = originalConsoleError; + } + }); }); diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index 26b05d008ac5..d95ccc81ada7 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -80,7 +80,10 @@ import {queueRecoverableErrors} from './ReactFiberWorkLoop.new'; let hydrationParentFiber: null | Fiber = null; let nextHydratableInstance: null | HydratableInstance = null; let isHydrating: boolean = false; -let didSuspend: boolean = false; + +// this flag allows for warning supression when we expect there to be mismatches due to +// earlier mismatches or a suspended fiber. +let didSuspendOrError: boolean = false; // Hydration errors that were thrown inside this boundary let hydrationErrors: Array | null = null; @@ -95,9 +98,9 @@ function warnIfHydrating() { } } -export function markDidSuspendWhileHydratingDEV() { +export function markDidThrowWhileHydratingDEV() { if (__DEV__) { - didSuspend = true; + didSuspendOrError = true; } } @@ -113,7 +116,7 @@ function enterHydrationState(fiber: Fiber): boolean { hydrationParentFiber = fiber; isHydrating = true; hydrationErrors = null; - didSuspend = false; + didSuspendOrError = false; return true; } @@ -131,7 +134,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance( hydrationParentFiber = fiber; isHydrating = true; hydrationErrors = null; - didSuspend = false; + didSuspendOrError = false; if (treeContext !== null) { restoreSuspendedTreeContext(fiber, treeContext); } @@ -196,7 +199,7 @@ function deleteHydratableInstance( function warnNonhydratedInstance(returnFiber: Fiber, fiber: Fiber) { if (__DEV__) { - if (didSuspend) { + if (didSuspendOrError) { // Inside a boundary that already suspended. We're currently rendering the // siblings of a suspended node. The mismatch may be due to the missing // data, so it's probably a false positive. @@ -444,7 +447,7 @@ function prepareToHydrateHostInstance( } const instance: Instance = fiber.stateNode; - const shouldWarnIfMismatchDev = !didSuspend; + const shouldWarnIfMismatchDev = !didSuspendOrError; const updatePayload = hydrateInstance( instance, fiber.type, @@ -474,7 +477,7 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean { const textInstance: TextInstance = fiber.stateNode; const textContent: string = fiber.memoizedProps; - const shouldWarnIfMismatchDev = !didSuspend; + const shouldWarnIfMismatchDev = !didSuspendOrError; const shouldUpdate = hydrateTextInstance( textInstance, textContent, @@ -653,7 +656,7 @@ function resetHydrationState(): void { hydrationParentFiber = null; nextHydratableInstance = null; isHydrating = false; - didSuspend = false; + didSuspendOrError = false; } export function upgradeHydrationErrorsToRecoverable(): void { diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js index cf7890e3edba..e5b2a5afcc36 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js @@ -80,7 +80,10 @@ import {queueRecoverableErrors} from './ReactFiberWorkLoop.old'; let hydrationParentFiber: null | Fiber = null; let nextHydratableInstance: null | HydratableInstance = null; let isHydrating: boolean = false; -let didSuspend: boolean = false; + +// this flag allows for warning supression when we expect there to be mismatches due to +// earlier mismatches or a suspended fiber. +let didSuspendOrError: boolean = false; // Hydration errors that were thrown inside this boundary let hydrationErrors: Array | null = null; @@ -95,9 +98,9 @@ function warnIfHydrating() { } } -export function markDidSuspendWhileHydratingDEV() { +export function markDidThrowWhileHydratingDEV() { if (__DEV__) { - didSuspend = true; + didSuspendOrError = true; } } @@ -113,7 +116,7 @@ function enterHydrationState(fiber: Fiber): boolean { hydrationParentFiber = fiber; isHydrating = true; hydrationErrors = null; - didSuspend = false; + didSuspendOrError = false; return true; } @@ -131,7 +134,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance( hydrationParentFiber = fiber; isHydrating = true; hydrationErrors = null; - didSuspend = false; + didSuspendOrError = false; if (treeContext !== null) { restoreSuspendedTreeContext(fiber, treeContext); } @@ -196,7 +199,7 @@ function deleteHydratableInstance( function warnNonhydratedInstance(returnFiber: Fiber, fiber: Fiber) { if (__DEV__) { - if (didSuspend) { + if (didSuspendOrError) { // Inside a boundary that already suspended. We're currently rendering the // siblings of a suspended node. The mismatch may be due to the missing // data, so it's probably a false positive. @@ -444,7 +447,7 @@ function prepareToHydrateHostInstance( } const instance: Instance = fiber.stateNode; - const shouldWarnIfMismatchDev = !didSuspend; + const shouldWarnIfMismatchDev = !didSuspendOrError; const updatePayload = hydrateInstance( instance, fiber.type, @@ -474,7 +477,7 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean { const textInstance: TextInstance = fiber.stateNode; const textContent: string = fiber.memoizedProps; - const shouldWarnIfMismatchDev = !didSuspend; + const shouldWarnIfMismatchDev = !didSuspendOrError; const shouldUpdate = hydrateTextInstance( textInstance, textContent, @@ -653,7 +656,7 @@ function resetHydrationState(): void { hydrationParentFiber = null; nextHydratableInstance = null; isHydrating = false; - didSuspend = false; + didSuspendOrError = false; } export function upgradeHydrationErrorsToRecoverable(): void { diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index f32fe5849419..7db27ba935d0 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -83,7 +83,7 @@ import { } from './ReactFiberLane.new'; import { getIsHydrating, - markDidSuspendWhileHydratingDEV, + markDidThrowWhileHydratingDEV, queueHydrationError, } from './ReactFiberHydrationContext.new'; @@ -453,8 +453,10 @@ function throwException( const wakeable: Wakeable = (value: any); resetSuspendedComponent(sourceFiber, rootRenderLanes); - if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { - markDidSuspendWhileHydratingDEV(); + if (__DEV__) { + if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { + markDidThrowWhileHydratingDEV(); + } } if (__DEV__) { @@ -518,6 +520,7 @@ function throwException( } else { // This is a regular error, not a Suspense wakeable. if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { + markDidThrowWhileHydratingDEV(); const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); // If the error was thrown during hydration, we may be able to recover by // discarding the dehydrated content and switching to a client render. diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index 9b07ede635ff..a8e75e9ce661 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -83,7 +83,7 @@ import { } from './ReactFiberLane.old'; import { getIsHydrating, - markDidSuspendWhileHydratingDEV, + markDidThrowWhileHydratingDEV, queueHydrationError, } from './ReactFiberHydrationContext.old'; @@ -444,10 +444,6 @@ function throwException( } } - if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { - markDidSuspendWhileHydratingDEV(); - } - if ( value !== null && typeof value === 'object' && @@ -457,8 +453,10 @@ function throwException( const wakeable: Wakeable = (value: any); resetSuspendedComponent(sourceFiber, rootRenderLanes); - if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { - markDidSuspendWhileHydratingDEV(); + if (__DEV__) { + if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { + markDidThrowWhileHydratingDEV(); + } } if (__DEV__) { @@ -522,6 +520,7 @@ function throwException( } else { // This is a regular error, not a Suspense wakeable. if (getIsHydrating() && sourceFiber.mode & ConcurrentMode) { + markDidThrowWhileHydratingDEV(); const suspenseBoundary = getNearestSuspenseBoundaryToCapture(returnFiber); // If the error was thrown during hydration, we may be able to recover by // discarding the dehydrated content and switching to a client render. From 3bccb4decfacb71dd9a3f2c7a17f01eea62bd412 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 20 Apr 2022 11:07:25 -0700 Subject: [PATCH 07/14] factor tests to assert different behavior between prod and dev --- ...actDOMFizzSuppressHydrationWarning-test.js | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzSuppressHydrationWarning-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzSuppressHydrationWarning-test.js index 3c86c55f0270..d1f60567a91a 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzSuppressHydrationWarning-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzSuppressHydrationWarning-test.js @@ -831,17 +831,21 @@ describe('ReactDOMFizzServerHydrationWarning', () => { ); expect(Scheduler).toFlushAndYield([]); - expect(mockError.mock.calls.length).toBe(1); - expect(mockError.mock.calls[0]).toEqual([ - 'Warning: Text content did not match. Server: "%s" Client: "%s"%s', - 'initial', - 'replaced', - '\n' + - ' in h2 (at **)\n' + - ' in Suspense (at **)\n' + - ' in div (at **)\n' + - ' in App (at **)', - ]); + if (__DEV__) { + expect(mockError.mock.calls.length).toBe(1); + expect(mockError.mock.calls[0]).toEqual([ + 'Warning: Text content did not match. Server: "%s" Client: "%s"%s', + 'initial', + 'replaced', + '\n' + + ' in h2 (at **)\n' + + ' in Suspense (at **)\n' + + ' in div (at **)\n' + + ' in App (at **)', + ]); + } else { + expect(mockError.mock.calls.length).toBe(0); + } } finally { console.error = originalConsoleError; } From eeede500db308ffe1975b4cc584cf0b0e06f526c Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 20 Apr 2022 11:07:59 -0700 Subject: [PATCH 08/14] add DEV suffix to didSuspendOrError to better indicate this feature should only affect dev behavior --- .../src/ReactFiberHydrationContext.new.js | 16 ++++++++-------- .../src/ReactFiberHydrationContext.old.js | 16 ++++++++-------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index d95ccc81ada7..bfc14d5b4ffb 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -83,7 +83,7 @@ let isHydrating: boolean = false; // this flag allows for warning supression when we expect there to be mismatches due to // earlier mismatches or a suspended fiber. -let didSuspendOrError: boolean = false; +let didSuspendOrErrorDEV: boolean = false; // Hydration errors that were thrown inside this boundary let hydrationErrors: Array | null = null; @@ -100,7 +100,7 @@ function warnIfHydrating() { export function markDidThrowWhileHydratingDEV() { if (__DEV__) { - didSuspendOrError = true; + didSuspendOrErrorDEV = true; } } @@ -116,7 +116,7 @@ function enterHydrationState(fiber: Fiber): boolean { hydrationParentFiber = fiber; isHydrating = true; hydrationErrors = null; - didSuspendOrError = false; + didSuspendOrErrorDEV = false; return true; } @@ -134,7 +134,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance( hydrationParentFiber = fiber; isHydrating = true; hydrationErrors = null; - didSuspendOrError = false; + didSuspendOrErrorDEV = false; if (treeContext !== null) { restoreSuspendedTreeContext(fiber, treeContext); } @@ -199,7 +199,7 @@ function deleteHydratableInstance( function warnNonhydratedInstance(returnFiber: Fiber, fiber: Fiber) { if (__DEV__) { - if (didSuspendOrError) { + if (didSuspendOrErrorDEV) { // Inside a boundary that already suspended. We're currently rendering the // siblings of a suspended node. The mismatch may be due to the missing // data, so it's probably a false positive. @@ -447,7 +447,7 @@ function prepareToHydrateHostInstance( } const instance: Instance = fiber.stateNode; - const shouldWarnIfMismatchDev = !didSuspendOrError; + const shouldWarnIfMismatchDev = !didSuspendOrErrorDEV; const updatePayload = hydrateInstance( instance, fiber.type, @@ -477,7 +477,7 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean { const textInstance: TextInstance = fiber.stateNode; const textContent: string = fiber.memoizedProps; - const shouldWarnIfMismatchDev = !didSuspendOrError; + const shouldWarnIfMismatchDev = !didSuspendOrErrorDEV; const shouldUpdate = hydrateTextInstance( textInstance, textContent, @@ -656,7 +656,7 @@ function resetHydrationState(): void { hydrationParentFiber = null; nextHydratableInstance = null; isHydrating = false; - didSuspendOrError = false; + didSuspendOrErrorDEV = false; } export function upgradeHydrationErrorsToRecoverable(): void { diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js index e5b2a5afcc36..7a9c67d9f97d 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js @@ -83,7 +83,7 @@ let isHydrating: boolean = false; // this flag allows for warning supression when we expect there to be mismatches due to // earlier mismatches or a suspended fiber. -let didSuspendOrError: boolean = false; +let didSuspendOrErrorDEV: boolean = false; // Hydration errors that were thrown inside this boundary let hydrationErrors: Array | null = null; @@ -100,7 +100,7 @@ function warnIfHydrating() { export function markDidThrowWhileHydratingDEV() { if (__DEV__) { - didSuspendOrError = true; + didSuspendOrErrorDEV = true; } } @@ -116,7 +116,7 @@ function enterHydrationState(fiber: Fiber): boolean { hydrationParentFiber = fiber; isHydrating = true; hydrationErrors = null; - didSuspendOrError = false; + didSuspendOrErrorDEV = false; return true; } @@ -134,7 +134,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance( hydrationParentFiber = fiber; isHydrating = true; hydrationErrors = null; - didSuspendOrError = false; + didSuspendOrErrorDEV = false; if (treeContext !== null) { restoreSuspendedTreeContext(fiber, treeContext); } @@ -199,7 +199,7 @@ function deleteHydratableInstance( function warnNonhydratedInstance(returnFiber: Fiber, fiber: Fiber) { if (__DEV__) { - if (didSuspendOrError) { + if (didSuspendOrErrorDEV) { // Inside a boundary that already suspended. We're currently rendering the // siblings of a suspended node. The mismatch may be due to the missing // data, so it's probably a false positive. @@ -447,7 +447,7 @@ function prepareToHydrateHostInstance( } const instance: Instance = fiber.stateNode; - const shouldWarnIfMismatchDev = !didSuspendOrError; + const shouldWarnIfMismatchDev = !didSuspendOrErrorDEV; const updatePayload = hydrateInstance( instance, fiber.type, @@ -477,7 +477,7 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean { const textInstance: TextInstance = fiber.stateNode; const textContent: string = fiber.memoizedProps; - const shouldWarnIfMismatchDev = !didSuspendOrError; + const shouldWarnIfMismatchDev = !didSuspendOrErrorDEV; const shouldUpdate = hydrateTextInstance( textInstance, textContent, @@ -656,7 +656,7 @@ function resetHydrationState(): void { hydrationParentFiber = null; nextHydratableInstance = null; isHydrating = false; - didSuspendOrError = false; + didSuspendOrErrorDEV = false; } export function upgradeHydrationErrorsToRecoverable(): void { From c8275826d14897551c016876185f806a84deb336 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 20 Apr 2022 11:22:30 -0700 Subject: [PATCH 09/14] move tests back to ReactDOMFizzServer-test --- .../src/__tests__/ReactDOMFizzServer-test.js | 79 ++++++++ ...actDOMFizzSuppressHydrationWarning-test.js | 183 ------------------ 2 files changed, 79 insertions(+), 183 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index ad5abfad18fe..8877d2a25d9d 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -2931,4 +2931,83 @@ describe('ReactDOMFizzServer', () => { expect(Scheduler).toFlushAndYield([]); }); + + // @gate experimental && enableClientRenderFallbackOnTextMismatch && enableClientRenderFallbackOnHydrationMismatch + it('only warns once on hydration mismatch while within a suspense boundary', async () => { + const originalConsoleError = console.error; + const mockError = jest.fn(); + console.error = (...args) => { + mockError(...args.map(normalizeCodeLocInfo)); + }; + + const App = ({text}) => { + return ( +
+ Loading...}> +

{text}

+

{text}

+

{text}

+
+
+ ); + }; + + try { + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream( + , + ); + pipe(writable); + }); + + expect(getVisibleChildren(container)).toEqual( +
+

initial

+

initial

+

initial

+
, + ); + + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue( + 'Logged recoverable error: ' + error.message, + ); + }, + }); + expect(Scheduler).toFlushAndYield([ + 'Logged recoverable error: Text content does not match server-rendered HTML.', + 'Logged recoverable error: Text content does not match server-rendered HTML.', + 'Logged recoverable error: Text content does not match server-rendered HTML.', + 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.', + ]); + + expect(getVisibleChildren(container)).toEqual( +
+

replaced

+

replaced

+

replaced

+
, + ); + + expect(Scheduler).toFlushAndYield([]); + if (__DEV__) { + expect(mockError.mock.calls.length).toBe(1); + expect(mockError.mock.calls[0]).toEqual([ + 'Warning: Text content did not match. Server: "%s" Client: "%s"%s', + 'initial', + 'replaced', + '\n' + + ' in h2 (at **)\n' + + ' in Suspense (at **)\n' + + ' in div (at **)\n' + + ' in App (at **)', + ]); + } else { + expect(mockError.mock.calls.length).toBe(0); + } + } finally { + console.error = originalConsoleError; + } + }); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzSuppressHydrationWarning-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzSuppressHydrationWarning-test.js index d1f60567a91a..ac53da312729 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzSuppressHydrationWarning-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzSuppressHydrationWarning-test.js @@ -12,7 +12,6 @@ let JSDOM; let Stream; let Scheduler; -let Suspense; let React; let ReactDOMClient; let ReactDOMFizzServer; @@ -29,7 +28,6 @@ describe('ReactDOMFizzServerHydrationWarning', () => { JSDOM = require('jsdom').JSDOM; Scheduler = require('scheduler'); React = require('react'); - Suspense = React.Suspense; ReactDOMClient = require('react-dom/client'); if (__EXPERIMENTAL__) { ReactDOMFizzServer = require('react-dom/server'); @@ -60,18 +58,6 @@ describe('ReactDOMFizzServerHydrationWarning', () => { }); }); - function normalizeCodeLocInfo(strOrErr) { - if (strOrErr && strOrErr.replace) { - return strOrErr.replace(/\n +(?:at|in) ([\S]+)[^\n]*/g, function( - m, - name, - ) { - return '\n in ' + name + ' (at **)'; - }); - } - return strOrErr; - } - async function act(callback) { await callback(); // Await one turn around the event loop. @@ -681,173 +667,4 @@ describe('ReactDOMFizzServerHydrationWarning', () => { , ); }); - - // @gate experimental && enableClientRenderFallbackOnTextMismatch && enableClientRenderFallbackOnHydrationMismatch - it('#24384: Suspending should halt hydration warnings while still allowing siblings to warm up', async () => { - const makeApp = () => { - let resolve, resolved; - const promise = new Promise(r => { - resolve = () => { - resolved = true; - return r(); - }; - }); - function ComponentThatSuspends() { - if (!resolved) { - throw promise; - } - return

A

; - } - - const App = ({text}) => { - return ( -
- Loading...}> - -

{text}

-
-
- ); - }; - - return [App, resolve]; - }; - - const [ServerApp, serverResolve] = makeApp(); - await act(async () => { - const {pipe} = ReactDOMFizzServer.renderToPipeableStream( - , - ); - pipe(writable); - }); - await act(() => { - serverResolve(); - }); - - expect(getVisibleChildren(container)).toEqual( -
-

A

-

initial

-
, - ); - - // the client app is rendered with an intentionally incorrect text. The still Suspended component causes - // hydration to fail silently (allowing for cache warming but otherwise skipping this boundary) until it - // resolves. - const [ClientApp, clientResolve] = makeApp(); - ReactDOMClient.hydrateRoot(container, , { - onRecoverableError(error) { - Scheduler.unstable_yieldValue( - 'Logged recoverable error: ' + error.message, - ); - }, - }); - Scheduler.unstable_flushAll(); - - expect(getVisibleChildren(container)).toEqual( -
-

A

-

initial

-
, - ); - - // Now that the boundary resolves to it's children the hydration completes and discovers that there is a mismatch requiring - // client-side rendering. - await clientResolve(); - expect(() => { - expect(Scheduler).toFlushAndYield([ - 'Logged recoverable error: Text content does not match server-rendered HTML.', - 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.', - ]); - }).toErrorDev( - 'Warning: Prop `name` did not match. Server: "initial" Client: "replaced"', - ); - expect(getVisibleChildren(container)).toEqual( -
-

A

-

replaced

-
, - ); - - expect(Scheduler).toFlushAndYield([]); - }); - - // @gate experimental && enableClientRenderFallbackOnTextMismatch && enableClientRenderFallbackOnHydrationMismatch - it('only warns once on hydration mismatch while within a suspense boundary', async () => { - const originalConsoleError = console.error; - const mockError = jest.fn(); - console.error = (...args) => { - mockError(...args.map(normalizeCodeLocInfo)); - }; - - const App = ({text}) => { - return ( -
- Loading...}> -

{text}

-

{text}

-

{text}

-
-
- ); - }; - - try { - await act(async () => { - const {pipe} = ReactDOMFizzServer.renderToPipeableStream( - , - ); - pipe(writable); - }); - - expect(getVisibleChildren(container)).toEqual( -
-

initial

-

initial

-

initial

-
, - ); - - ReactDOMClient.hydrateRoot(container, , { - onRecoverableError(error) { - Scheduler.unstable_yieldValue( - 'Logged recoverable error: ' + error.message, - ); - }, - }); - expect(Scheduler).toFlushAndYield([ - 'Logged recoverable error: Text content does not match server-rendered HTML.', - 'Logged recoverable error: Text content does not match server-rendered HTML.', - 'Logged recoverable error: Text content does not match server-rendered HTML.', - 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.', - ]); - - expect(getVisibleChildren(container)).toEqual( -
-

replaced

-

replaced

-

replaced

-
, - ); - - expect(Scheduler).toFlushAndYield([]); - if (__DEV__) { - expect(mockError.mock.calls.length).toBe(1); - expect(mockError.mock.calls[0]).toEqual([ - 'Warning: Text content did not match. Server: "%s" Client: "%s"%s', - 'initial', - 'replaced', - '\n' + - ' in h2 (at **)\n' + - ' in Suspense (at **)\n' + - ' in div (at **)\n' + - ' in App (at **)', - ]); - } else { - expect(mockError.mock.calls.length).toBe(0); - } - } finally { - console.error = originalConsoleError; - } - }); }); From 945352ec238b9a2c710debd68564ff672f8d7bb6 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 20 Apr 2022 11:25:38 -0700 Subject: [PATCH 10/14] fix comment casing --- packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js | 2 +- .../react-reconciler/src/ReactFiberHydrationContext.new.js | 4 ++-- .../react-reconciler/src/ReactFiberHydrationContext.old.js | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 8877d2a25d9d..7a68bb5e3e3e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -2891,7 +2891,7 @@ describe('ReactDOMFizzServer', () => { , ); - // the client app is rendered with an intentionally incorrect text. The still Suspended component causes + // The client app is rendered with an intentionally incorrect text. The still Suspended component causes // hydration to fail silently (allowing for cache warming but otherwise skipping this boundary) until it // resolves. const [ClientApp, clientResolve] = makeApp(); diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index bfc14d5b4ffb..e0e592d32790 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -81,8 +81,8 @@ let hydrationParentFiber: null | Fiber = null; let nextHydratableInstance: null | HydratableInstance = null; let isHydrating: boolean = false; -// this flag allows for warning supression when we expect there to be mismatches due to -// earlier mismatches or a suspended fiber. +// This flag allows for warning supression when we expect there to be mismatches +// due to earlier mismatches or a suspended fiber. let didSuspendOrErrorDEV: boolean = false; // Hydration errors that were thrown inside this boundary diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js index 7a9c67d9f97d..a4fd5c93e22e 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js @@ -81,8 +81,8 @@ let hydrationParentFiber: null | Fiber = null; let nextHydratableInstance: null | HydratableInstance = null; let isHydrating: boolean = false; -// this flag allows for warning supression when we expect there to be mismatches due to -// earlier mismatches or a suspended fiber. +// This flag allows for warning supression when we expect there to be mismatches +// due to earlier mismatches or a suspended fiber. let didSuspendOrErrorDEV: boolean = false; // Hydration errors that were thrown inside this boundary From 07a2eeef34748df26af0e3b0e5473112a0f51304 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 20 Apr 2022 11:32:16 -0700 Subject: [PATCH 11/14] drop extra flag gates in tests --- packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 7a68bb5e3e3e..d88bf46cd124 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -2842,7 +2842,7 @@ describe('ReactDOMFizzServer', () => { }); }); - // @gate experimental && enableClientRenderFallbackOnTextMismatch && enableClientRenderFallbackOnHydrationMismatch + // @gate experimental && enableClientRenderFallbackOnTextMismatch it('#24384: Suspending should halt hydration warnings while still allowing siblings to warm up', async () => { const makeApp = () => { let resolve, resolved; @@ -2932,7 +2932,7 @@ describe('ReactDOMFizzServer', () => { expect(Scheduler).toFlushAndYield([]); }); - // @gate experimental && enableClientRenderFallbackOnTextMismatch && enableClientRenderFallbackOnHydrationMismatch + // @gate experimental && enableClientRenderFallbackOnTextMismatch it('only warns once on hydration mismatch while within a suspense boundary', async () => { const originalConsoleError = console.error; const mockError = jest.fn(); From 8839f1b8387970f0ac575732d90cd0c5451a7c6d Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 20 Apr 2022 14:31:38 -0700 Subject: [PATCH 12/14] add test for user error case --- .../src/__tests__/ReactDOMFizzServer-test.js | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index d88bf46cd124..eff9ad5fbae8 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -3010,4 +3010,80 @@ describe('ReactDOMFizzServer', () => { console.error = originalConsoleError; } }); + + // @gate experimental && enableClientRenderFallbackOnTextMismatch + it('supresses hydration warnings when an error occurs within a Suspense boundary', async () => { + let isClient = false; + let shouldThrow = true; + + function ThrowUntilOnClient({children}) { + if (isClient && shouldThrow) { + throw new Error('uh oh'); + } + return children; + } + + function StopThrowingOnClient() { + if (isClient) { + shouldThrow = false; + } + return null; + } + + const App = () => { + return ( +
+ Loading...}> + +

one

+
+ +

two

+

three

+
+
+ ); + }; + + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(); + pipe(writable); + }); + + expect(getVisibleChildren(container)).toEqual( +
+

one

+

two

+

three

+
, + ); + + isClient = true; + + ReactDOMClient.hydrateRoot(container, , { + onRecoverableError(error) { + Scheduler.unstable_yieldValue( + 'Logged recoverable error: ' + error.message, + ); + }, + }); + await act(async () => { + expect(Scheduler).toFlushAndYieldThrough([ + 'Logged recoverable error: uh oh', + 'Logged recoverable error: Hydration failed because the initial UI does not match what was rendered on the server.', + 'Logged recoverable error: Hydration failed because the initial UI does not match what was rendered on the server.', + 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.', + ]); + }); + + expect(getVisibleChildren(container)).toEqual( +
+

one

+

two

+

three

+
, + ); + + expect(Scheduler).toFlushAndYield([]); + }); }); From 3fd1eb6356eaaa274b3f2d72e229638670d112d6 Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 20 Apr 2022 14:33:09 -0700 Subject: [PATCH 13/14] remove unnecessary gate --- packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index eff9ad5fbae8..aec49569a21e 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -3011,7 +3011,7 @@ describe('ReactDOMFizzServer', () => { } }); - // @gate experimental && enableClientRenderFallbackOnTextMismatch + // @gate experimental it('supresses hydration warnings when an error occurs within a Suspense boundary', async () => { let isClient = false; let shouldThrow = true; From be1cefa009afd113239d289aa44ca86b3011876e Mon Sep 17 00:00:00 2001 From: Josh Story Date: Wed, 20 Apr 2022 14:39:33 -0700 Subject: [PATCH 14/14] Make test better it not has an intentional client mismatch that would error if there wasn't supression brought about by the earlier error. when it client rendres it has the updated value not found in the server response but we do not see a hydration warning because it was superseded by the thrown error in that render --- .../src/__tests__/ReactDOMFizzServer-test.js | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index aec49569a21e..bf5388597571 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -3037,9 +3037,9 @@ describe('ReactDOMFizzServer', () => {

one

-

two

-

three

+

{isClient ? 'five' : 'three'}

+ ); @@ -3067,20 +3067,18 @@ describe('ReactDOMFizzServer', () => { ); }, }); - await act(async () => { - expect(Scheduler).toFlushAndYieldThrough([ - 'Logged recoverable error: uh oh', - 'Logged recoverable error: Hydration failed because the initial UI does not match what was rendered on the server.', - 'Logged recoverable error: Hydration failed because the initial UI does not match what was rendered on the server.', - 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.', - ]); - }); + expect(Scheduler).toFlushAndYield([ + 'Logged recoverable error: uh oh', + 'Logged recoverable error: Hydration failed because the initial UI does not match what was rendered on the server.', + 'Logged recoverable error: Hydration failed because the initial UI does not match what was rendered on the server.', + 'Logged recoverable error: There was an error while hydrating this Suspense boundary. Switched to client rendering.', + ]); expect(getVisibleChildren(container)).toEqual(

one

two

-

three

+

five

, );