From f468816ef1000b2fe0086feabc0115b86f299cad Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 25 Feb 2022 18:10:10 -0500 Subject: [PATCH] Fix false positive hydration warnings (#23364) * Failing test for react#23331 * Don't warn on hydration mismatch if suspended When something suspends during hydration, we continue rendering the siblings to warm up the cache and fire off any lazy network requests. However, if there are any mismatches while rendering the siblings, it's likely a false positive caused by the earlier suspended component. So we should suppress any hydration warnings until the tree no longer suspends. Fixes #23332 Co-authored-by: Marcel Laverdet --- .../src/__tests__/ReactDOMFizzServer-test.js | 66 +++++++++++++++++++ .../react-dom/src/client/ReactDOMComponent.js | 31 +++++---- .../src/client/ReactDOMHostConfig.js | 19 +++++- .../src/ReactFiberHydrationContext.new.js | 27 +++++++- .../src/ReactFiberHydrationContext.old.js | 27 +++++++- .../src/ReactFiberThrow.new.js | 3 + .../src/ReactFiberThrow.old.js | 3 + 7 files changed, 161 insertions(+), 15 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js index 01397d0000..0b14e567c7 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js @@ -301,6 +301,72 @@ describe('ReactDOMFizzServer', () => { ); }); + // @gate experimental + it('#23331: does not warn about hydration mismatches if something suspended in an earlier sibling', async () => { + const makeApp = () => { + let resolve; + const imports = new Promise(r => { + resolve = () => r({default: () => async}); + }); + const Lazy = React.lazy(() => imports); + + const App = () => ( +
+ Loading...}> + + after + +
+ ); + + return [App, resolve]; + }; + + // Server-side + const [App, resolve] = makeApp(); + await act(async () => { + const {pipe} = ReactDOMFizzServer.renderToPipeableStream(); + pipe(writable); + }); + expect(getVisibleChildren(container)).toEqual( +
+ Loading... +
, + ); + await act(async () => { + resolve(); + }); + expect(getVisibleChildren(container)).toEqual( +
+ async + after +
, + ); + + // Client-side + const [HydrateApp, hydrateResolve] = makeApp(); + await act(async () => { + ReactDOM.hydrateRoot(container, ); + }); + + expect(getVisibleChildren(container)).toEqual( +
+ async + after +
, + ); + + await act(async () => { + hydrateResolve(); + }); + expect(getVisibleChildren(container)).toEqual( +
+ async + after +
, + ); + }); + // @gate experimental it('should support nonce scripts', async () => { CSPnonce = 'R4nd0m'; diff --git a/packages/react-dom/src/client/ReactDOMComponent.js b/packages/react-dom/src/client/ReactDOMComponent.js index 767b200f29..1552629da1 100644 --- a/packages/react-dom/src/client/ReactDOMComponent.js +++ b/packages/react-dom/src/client/ReactDOMComponent.js @@ -230,6 +230,7 @@ export function checkForUnmatchedText( serverText: string, clientText: string | number, isConcurrentMode: boolean, + shouldWarnDev: boolean, ) { const normalizedClientText = normalizeMarkupForTextOrAttribute(clientText); const normalizedServerText = normalizeMarkupForTextOrAttribute(serverText); @@ -237,14 +238,16 @@ export function checkForUnmatchedText( return; } - if (__DEV__) { - if (!didWarnInvalidHydration) { - didWarnInvalidHydration = true; - console.error( - 'Text content did not match. Server: "%s" Client: "%s"', - normalizedServerText, - normalizedClientText, - ); + if (shouldWarnDev) { + if (__DEV__) { + if (!didWarnInvalidHydration) { + didWarnInvalidHydration = true; + console.error( + 'Text content did not match. Server: "%s" Client: "%s"', + normalizedServerText, + normalizedClientText, + ); + } } } @@ -866,6 +869,7 @@ export function diffHydratedProperties( parentNamespace: string, rootContainerElement: Element | Document, isConcurrentMode: boolean, + shouldWarnDev: boolean, ): null | Array { let isCustomComponentTag; let extraAttributeNames: Set; @@ -985,6 +989,7 @@ export function diffHydratedProperties( domElement.textContent, nextProp, isConcurrentMode, + shouldWarnDev, ); } updatePayload = [CHILDREN, nextProp]; @@ -996,6 +1001,7 @@ export function diffHydratedProperties( domElement.textContent, nextProp, isConcurrentMode, + shouldWarnDev, ); } updatePayload = [CHILDREN, '' + nextProp]; @@ -1011,6 +1017,7 @@ export function diffHydratedProperties( } } } else if ( + shouldWarnDev && __DEV__ && // Convince Flow we've calculated it (it's DEV-only in this method.) typeof isCustomComponentTag === 'boolean' @@ -1142,10 +1149,12 @@ export function diffHydratedProperties( } if (__DEV__) { - // $FlowFixMe - Should be inferred as not undefined. - if (extraAttributeNames.size > 0 && !suppressHydrationWarning) { + if (shouldWarnDev) { // $FlowFixMe - Should be inferred as not undefined. - warnForExtraAttributes(extraAttributeNames); + if (extraAttributeNames.size > 0 && !suppressHydrationWarning) { + // $FlowFixMe - Should be inferred as not undefined. + warnForExtraAttributes(extraAttributeNames); + } } } diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js index 4863a16eda..97eb494ecb 100644 --- a/packages/react-dom/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom/src/client/ReactDOMHostConfig.js @@ -786,6 +786,7 @@ export function hydrateInstance( rootContainerInstance: Container, hostContext: HostContext, internalInstanceHandle: Object, + shouldWarnDev: boolean, ): null | Array { precacheFiberNode(internalInstanceHandle, instance); // TODO: Possibly defer this until the commit phase where all the events @@ -811,6 +812,7 @@ export function hydrateInstance( parentNamespace, rootContainerInstance, isConcurrentMode, + shouldWarnDev, ); } @@ -818,6 +820,7 @@ export function hydrateTextInstance( textInstance: TextInstance, text: string, internalInstanceHandle: Object, + shouldWarnDev: boolean, ): boolean { precacheFiberNode(internalInstanceHandle, textInstance); @@ -924,7 +927,13 @@ export function didNotMatchHydratedContainerTextInstance( text: string, isConcurrentMode: boolean, ) { - checkForUnmatchedText(textInstance.nodeValue, text, isConcurrentMode); + const shouldWarnDev = true; + checkForUnmatchedText( + textInstance.nodeValue, + text, + isConcurrentMode, + shouldWarnDev, + ); } export function didNotMatchHydratedTextInstance( @@ -936,7 +945,13 @@ export function didNotMatchHydratedTextInstance( isConcurrentMode: boolean, ) { if (parentProps[SUPPRESS_HYDRATION_WARNING] !== true) { - checkForUnmatchedText(textInstance.nodeValue, text, isConcurrentMode); + const shouldWarnDev = true; + checkForUnmatchedText( + textInstance.nodeValue, + text, + isConcurrentMode, + shouldWarnDev, + ); } } diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js index 6905f58e2a..6a7a175d6c 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js @@ -84,6 +84,7 @@ import {queueRecoverableErrors} from './ReactFiberWorkLoop.new'; let hydrationParentFiber: null | Fiber = null; let nextHydratableInstance: null | HydratableInstance = null; let isHydrating: boolean = false; +let didSuspend: boolean = false; // Hydration errors that were thrown inside this boundary let hydrationErrors: Array | null = null; @@ -98,6 +99,12 @@ function warnIfHydrating() { } } +export function markDidSuspendWhileHydratingDEV() { + if (__DEV__) { + didSuspend = true; + } +} + function enterHydrationState(fiber: Fiber): boolean { if (!supportsHydration) { return false; @@ -110,6 +117,7 @@ function enterHydrationState(fiber: Fiber): boolean { hydrationParentFiber = fiber; isHydrating = true; hydrationErrors = null; + didSuspend = false; return true; } @@ -127,6 +135,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance( hydrationParentFiber = fiber; isHydrating = true; hydrationErrors = null; + didSuspend = false; if (treeContext !== null) { restoreSuspendedTreeContext(fiber, treeContext); } @@ -185,6 +194,13 @@ function deleteHydratableInstance( function warnNonhydratedInstance(returnFiber: Fiber, fiber: Fiber) { if (__DEV__) { + if (didSuspend) { + // 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. + return; + } + switch (returnFiber.tag) { case HostRoot: { const parentContainer = returnFiber.stateNode.containerInfo; @@ -418,6 +434,7 @@ function prepareToHydrateHostInstance( } const instance: Instance = fiber.stateNode; + const shouldWarnIfMismatchDev = !didSuspend; const updatePayload = hydrateInstance( instance, fiber.type, @@ -425,6 +442,7 @@ function prepareToHydrateHostInstance( rootContainerInstance, hostContext, fiber, + shouldWarnIfMismatchDev, ); // TODO: Type this specific to this type of component. fiber.updateQueue = (updatePayload: any); @@ -446,7 +464,13 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean { const textInstance: TextInstance = fiber.stateNode; const textContent: string = fiber.memoizedProps; - const shouldUpdate = hydrateTextInstance(textInstance, textContent, fiber); + const shouldWarnIfMismatchDev = !didSuspend; + const shouldUpdate = hydrateTextInstance( + textInstance, + textContent, + fiber, + shouldWarnIfMismatchDev, + ); if (shouldUpdate) { // We assume that prepareToHydrateHostTextInstance is called in a context where the // hydration parent is the parent host component of this host text. @@ -616,6 +640,7 @@ function resetHydrationState(): void { hydrationParentFiber = null; nextHydratableInstance = null; isHydrating = false; + didSuspend = false; } export function upgradeHydrationErrorsToRecoverable(): void { diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js index b9e88d3a21..8c33492494 100644 --- a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js +++ b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js @@ -84,6 +84,7 @@ import {queueRecoverableErrors} from './ReactFiberWorkLoop.old'; let hydrationParentFiber: null | Fiber = null; let nextHydratableInstance: null | HydratableInstance = null; let isHydrating: boolean = false; +let didSuspend: boolean = false; // Hydration errors that were thrown inside this boundary let hydrationErrors: Array | null = null; @@ -98,6 +99,12 @@ function warnIfHydrating() { } } +export function markDidSuspendWhileHydratingDEV() { + if (__DEV__) { + didSuspend = true; + } +} + function enterHydrationState(fiber: Fiber): boolean { if (!supportsHydration) { return false; @@ -110,6 +117,7 @@ function enterHydrationState(fiber: Fiber): boolean { hydrationParentFiber = fiber; isHydrating = true; hydrationErrors = null; + didSuspend = false; return true; } @@ -127,6 +135,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance( hydrationParentFiber = fiber; isHydrating = true; hydrationErrors = null; + didSuspend = false; if (treeContext !== null) { restoreSuspendedTreeContext(fiber, treeContext); } @@ -185,6 +194,13 @@ function deleteHydratableInstance( function warnNonhydratedInstance(returnFiber: Fiber, fiber: Fiber) { if (__DEV__) { + if (didSuspend) { + // 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. + return; + } + switch (returnFiber.tag) { case HostRoot: { const parentContainer = returnFiber.stateNode.containerInfo; @@ -418,6 +434,7 @@ function prepareToHydrateHostInstance( } const instance: Instance = fiber.stateNode; + const shouldWarnIfMismatchDev = !didSuspend; const updatePayload = hydrateInstance( instance, fiber.type, @@ -425,6 +442,7 @@ function prepareToHydrateHostInstance( rootContainerInstance, hostContext, fiber, + shouldWarnIfMismatchDev, ); // TODO: Type this specific to this type of component. fiber.updateQueue = (updatePayload: any); @@ -446,7 +464,13 @@ function prepareToHydrateHostTextInstance(fiber: Fiber): boolean { const textInstance: TextInstance = fiber.stateNode; const textContent: string = fiber.memoizedProps; - const shouldUpdate = hydrateTextInstance(textInstance, textContent, fiber); + const shouldWarnIfMismatchDev = !didSuspend; + const shouldUpdate = hydrateTextInstance( + textInstance, + textContent, + fiber, + shouldWarnIfMismatchDev, + ); if (shouldUpdate) { // We assume that prepareToHydrateHostTextInstance is called in a context where the // hydration parent is the parent host component of this host text. @@ -616,6 +640,7 @@ function resetHydrationState(): void { hydrationParentFiber = null; nextHydratableInstance = null; isHydrating = false; + didSuspend = false; } export function upgradeHydrationErrorsToRecoverable(): void { diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index cc5e3215c7..815cb25ef0 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -83,6 +83,7 @@ import { } from './ReactFiberLane.new'; import { getIsHydrating, + markDidSuspendWhileHydratingDEV, queueHydrationError, } from './ReactFiberHydrationContext.new'; @@ -513,6 +514,8 @@ 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 727c613d76..ec89f5ab0c 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -83,6 +83,7 @@ import { } from './ReactFiberLane.old'; import { getIsHydrating, + markDidSuspendWhileHydratingDEV, queueHydrationError, } from './ReactFiberHydrationContext.old'; @@ -513,6 +514,8 @@ 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.