Fix false positive hydration warnings (#23364)

acdlite · laverdet · web-flow · commit f468816ef100 · 2022-02-25T18:10:10.000-05:00
* 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 <marcel@laverdet.com>
diff --git 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: () => <span id="async">async</span>});
+      });
+      const Lazy = React.lazy(() => imports);
+
+      const App = () => (
+        <div>
+          <Suspense fallback={<span>Loading...</span>}>
+            <Lazy />
+            <span id="after">after</span>
+          </Suspense>
+        </div>
+      );
+
+      return [App, resolve];
+    };
+
+    // Server-side
+    const [App, resolve] = makeApp();
+    await act(async () => {
+      const {pipe} = ReactDOMFizzServer.renderToPipeableStream(<App />);
+      pipe(writable);
+    });
+    expect(getVisibleChildren(container)).toEqual(
+      <div>
+        <span>Loading...</span>
+      </div>,
+    );
+    await act(async () => {
+      resolve();
+    });
+    expect(getVisibleChildren(container)).toEqual(
+      <div>
+        <span id="async">async</span>
+        <span id="after">after</span>
+      </div>,
+    );
+
+    // Client-side
+    const [HydrateApp, hydrateResolve] = makeApp();
+    await act(async () => {
+      ReactDOM.hydrateRoot(container, <HydrateApp />);
+    });
+
+    expect(getVisibleChildren(container)).toEqual(
+      <div>
+        <span id="async">async</span>
+        <span id="after">after</span>
+      </div>,
+    );
+
+    await act(async () => {
+      hydrateResolve();
+    });
+    expect(getVisibleChildren(container)).toEqual(
+      <div>
+        <span id="async">async</span>
+        <span id="after">after</span>
+      </div>,
+    );
+  });
+
   // @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
@@ -230,21 +230,24 @@ export function checkForUnmatchedText(
   serverText: string,
   clientText: string | number,
   isConcurrentMode: boolean,
+  shouldWarnDev: boolean,
 ) {
   const normalizedClientText = normalizeMarkupForTextOrAttribute(clientText);
   const normalizedServerText = normalizeMarkupForTextOrAttribute(serverText);
   if (normalizedServerText === normalizedClientText) {
     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<mixed> {
   let isCustomComponentTag;
   let extraAttributeNames: Set<string>;
@@ -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
@@ -786,6 +786,7 @@ export function hydrateInstance(
   rootContainerInstance: Container,
   hostContext: HostContext,
   internalInstanceHandle: Object,
+  shouldWarnDev: boolean,
 ): null | Array<mixed> {
   precacheFiberNode(internalInstanceHandle, instance);
   // TODO: Possibly defer this until the commit phase where all the events
@@ -811,13 +812,15 @@ export function hydrateInstance(
     parentNamespace,
     rootContainerInstance,
     isConcurrentMode,
+    shouldWarnDev,
   );
 }
 
 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
@@ -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<mixed> | 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,13 +434,15 @@ function prepareToHydrateHostInstance(
   }
 
   const instance: Instance = fiber.stateNode;
+  const shouldWarnIfMismatchDev = !didSuspend;
   const updatePayload = hydrateInstance(
     instance,
     fiber.type,
     fiber.memoizedProps,
     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
@@ -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<mixed> | 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,13 +434,15 @@ function prepareToHydrateHostInstance(
   }
 
   const instance: Instance = fiber.stateNode;
+  const shouldWarnIfMismatchDev = !didSuspend;
   const updatePayload = hydrateInstance(
     instance,
     fiber.type,
     fiber.memoizedProps,
     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
diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js
