refactor didSuspend to didSuspendOrError

gnoff · gnoff · commit ebce981d8540 · 2022-04-20T10:00:41.000-07:00
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
diff --git 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.
@@ -702,4 +716,169 @@ describe('ReactDOMFizzServerHydrationWarning', () => {
       </div>,
     );
   });
+
+  // @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 <p>A</p>;
+      }
+
+      const App = ({text}) => {
+        return (
+          <div>
+            <Suspense fallback={<h1>Loading...</h1>}>
+              <ComponentThatSuspends />
+              <h2 name={text}>{text}</h2>
+            </Suspense>
+          </div>
+        );
+      };
+
+      return [App, resolve];
+    };
+
+    const [ServerApp, serverResolve] = makeApp();
+    await act(async () => {
+      const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
+        <ServerApp text="initial" />,
+      );
+      pipe(writable);
+    });
+    await act(() => {
+      serverResolve();
+    });
+
+    expect(getVisibleChildren(container)).toEqual(
+      <div>
+        <p>A</p>
+        <h2 name="initial">initial</h2>
+      </div>,
+    );
+
+    // 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, <ClientApp text="replaced" />, {
+      onRecoverableError(error) {
+        Scheduler.unstable_yieldValue(
+          'Logged recoverable error: ' + error.message,
+        );
+      },
+    });
+    Scheduler.unstable_flushAll();
+
+    expect(getVisibleChildren(container)).toEqual(
+      <div>
+        <p>A</p>
+        <h2 name="initial">initial</h2>
+      </div>,
+    );
+
+    // 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(
+      <div>
+        <p>A</p>
+        <h2 name="replaced">replaced</h2>
+      </div>,
+    );
+
+    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 (
+        <div>
+          <Suspense fallback={<h1>Loading...</h1>}>
+            <h2>{text}</h2>
+            <h2>{text}</h2>
+            <h2>{text}</h2>
+          </Suspense>
+        </div>
+      );
+    };
+
+    try {
+      await act(async () => {
+        const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
+          <App text="initial" />,
+        );
+        pipe(writable);
+      });
+
+      expect(getVisibleChildren(container)).toEqual(
+        <div>
+          <h2>initial</h2>
+          <h2>initial</h2>
+          <h2>initial</h2>
+        </div>,
+      );
+
+      ReactDOMClient.hydrateRoot(container, <App text="replaced" />, {
+        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(
+        <div>
+          <h2>replaced</h2>
+          <h2>replaced</h2>
+          <h2>replaced</h2>
+        </div>,
+      );
+
+      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
@@ -84,7 +84,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<mixed> | null = null;
@@ -99,9 +102,9 @@ function warnIfHydrating() {
   }
 }
 
-export function markDidSuspendWhileHydratingDEV() {
+export function markDidThrowWhileHydratingDEV() {
   if (__DEV__) {
-    didSuspend = true;
+    didSuspendOrError = true;
   }
 }
 
@@ -117,7 +120,7 @@ function enterHydrationState(fiber: Fiber): boolean {
   hydrationParentFiber = fiber;
   isHydrating = true;
   hydrationErrors = null;
-  didSuspend = false;
+  didSuspendOrError = false;
   return true;
 }
 
@@ -135,7 +138,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance(
   hydrationParentFiber = fiber;
   isHydrating = true;
   hydrationErrors = null;
-  didSuspend = false;
+  didSuspendOrError = false;
   if (treeContext !== null) {
     restoreSuspendedTreeContext(fiber, treeContext);
   }
@@ -200,7 +203,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.
@@ -451,7 +454,7 @@ function prepareToHydrateHostInstance(
   }
 
   const instance: Instance = fiber.stateNode;
-  const shouldWarnIfMismatchDev = !didSuspend;
+  const shouldWarnIfMismatchDev = !didSuspendOrError;
   const updatePayload = hydrateInstance(
     instance,
     fiber.type,
@@ -481,7 +484,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,
@@ -660,7 +663,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
@@ -84,7 +84,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<mixed> | null = null;
@@ -99,9 +102,9 @@ function warnIfHydrating() {
   }
 }
 
-export function markDidSuspendWhileHydratingDEV() {
+export function markDidThrowWhileHydratingDEV() {
   if (__DEV__) {
-    didSuspend = true;
+    didSuspendOrError = true;
   }
 }
 
@@ -117,7 +120,7 @@ function enterHydrationState(fiber: Fiber): boolean {
   hydrationParentFiber = fiber;
   isHydrating = true;
   hydrationErrors = null;
-  didSuspend = false;
+  didSuspendOrError = false;
   return true;
 }
 
@@ -135,7 +138,7 @@ function reenterHydrationStateFromDehydratedSuspenseInstance(
   hydrationParentFiber = fiber;
   isHydrating = true;
   hydrationErrors = null;
-  didSuspend = false;
+  didSuspendOrError = false;
   if (treeContext !== null) {
     restoreSuspendedTreeContext(fiber, treeContext);
   }
@@ -200,7 +203,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.
@@ -451,7 +454,7 @@ function prepareToHydrateHostInstance(
   }
 
   const instance: Instance = fiber.stateNode;
-  const shouldWarnIfMismatchDev = !didSuspend;
+  const shouldWarnIfMismatchDev = !didSuspendOrError;
   const updatePayload = hydrateInstance(
     instance,
     fiber.type,
@@ -481,7 +484,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,
@@ -660,7 +663,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
@@ -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
