Throw on hydration mismatch and force client rendering if boundary ha…

…sn't suspended within concurrent root (#22629) * Throw on hydration mismatch * remove debugger * update error message * update error message part2... * fix test? * test? :( * tests 4real * remove useRefAccessWarning gating * split markSuspenseBoundary and getNearestBoundary * also assert html is correct * replace-fork * also remove client render flag on suspend * replace-fork * fix mismerge????
facebook · Nov 9, 2021 · 0ddd69d · 0ddd69d
1 parent c3f34e4
commit 0ddd69d
Show file tree

Hide file tree

Showing 7 changed files with 444 additions and 346 deletions.
diff --git a/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js b/packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
@@ -1677,45 +1677,37 @@ describe('ReactDOMFizzServer', () => {
 
   // @gate experimental
   it('calls getServerSnapshot instead of getSnapshot', async () => {
-    const ref = React.createRef();
-
     function getServerSnapshot() {
       return 'server';
     }
-
     function getClientSnapshot() {
       return 'client';
     }
-
     function subscribe() {
       return () => {};
     }
-
     function Child({text}) {
       Scheduler.unstable_yieldValue(text);
       return text;
     }
-
     function App() {
       const value = useSyncExternalStore(
         subscribe,
         getClientSnapshot,
         getServerSnapshot,
       );
       return (
-        <div ref={ref}>
+        <div>
           <Child text={value} />
         </div>
       );
     }
-
     const loggedErrors = [];
     await act(async () => {
       const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
         <Suspense fallback="Loading...">
           <App />
         </Suspense>,
-
         {
           onError(x) {
             loggedErrors.push(x);
@@ -1726,56 +1718,43 @@ describe('ReactDOMFizzServer', () => {
     });
     expect(Scheduler).toHaveYielded(['server']);
 
-    const serverRenderedDiv = container.getElementsByTagName('div')[0];
-
     ReactDOM.hydrateRoot(container, <App />);
 
-    // The first paint uses the server snapshot
-    expect(Scheduler).toFlushUntilNextPaint(['server']);
-    expect(getVisibleChildren(container)).toEqual(<div>server</div>);
-    // Hydration succeeded
-    expect(ref.current).toEqual(serverRenderedDiv);
-
-    // Asynchronously we detect that the store has changed on the client,
-    // and patch up the inconsistency
-    expect(Scheduler).toFlushUntilNextPaint(['client']);
+    expect(() => {
+      // The first paint switches to client rendering due to mismatch
+      expect(Scheduler).toFlushUntilNextPaint(['client']);
+    }).toErrorDev(
+      'Warning: An error occurred during hydration. The server HTML was replaced with client content',
+      {withoutStack: true},
+    );
     expect(getVisibleChildren(container)).toEqual(<div>client</div>);
-    expect(ref.current).toEqual(serverRenderedDiv);
   });
 
   // The selector implementation uses the lazy ref initialization pattern
-  // @gate !(enableUseRefAccessWarning && __DEV__)
   // @gate experimental
   it('calls getServerSnapshot instead of getSnapshot (with selector and isEqual)', async () => {
     // Same as previous test, but with a selector that returns a complex object
     // that is memoized with a custom `isEqual` function.
     const ref = React.createRef();
-
     function getServerSnapshot() {
       return {env: 'server', other: 'unrelated'};
     }
-
     function getClientSnapshot() {
       return {env: 'client', other: 'unrelated'};
     }
-
     function selector({env}) {
       return {env};
     }
-
     function isEqual(a, b) {
       return a.env === b.env;
     }
-
     function subscribe() {
       return () => {};
     }
-
     function Child({text}) {
       Scheduler.unstable_yieldValue(text);
       return text;
     }
-
     function App() {
       const {env} = useSyncExternalStoreWithSelector(
         subscribe,
@@ -1790,14 +1769,12 @@ describe('ReactDOMFizzServer', () => {
         </div>
       );
     }
-
     const loggedErrors = [];
     await act(async () => {
       const {pipe} = ReactDOMFizzServer.renderToPipeableStream(
         <Suspense fallback="Loading...">
           <App />
         </Suspense>,
-
         {
           onError(x) {
             loggedErrors.push(x);
@@ -1808,21 +1785,17 @@ describe('ReactDOMFizzServer', () => {
     });
     expect(Scheduler).toHaveYielded(['server']);
 
-    const serverRenderedDiv = container.getElementsByTagName('div')[0];
-
     ReactDOM.hydrateRoot(container, <App />);
 
-    // The first paint uses the server snapshot
-    expect(Scheduler).toFlushUntilNextPaint(['server']);
-    expect(getVisibleChildren(container)).toEqual(<div>server</div>);
-    // Hydration succeeded
-    expect(ref.current).toEqual(serverRenderedDiv);
-
-    // Asynchronously we detect that the store has changed on the client,
-    // and patch up the inconsistency
-    expect(Scheduler).toFlushUntilNextPaint(['client']);
+    // The first paint uses the client due to mismatch forcing client render
+    expect(() => {
+      // The first paint switches to client rendering due to mismatch
+      expect(Scheduler).toFlushUntilNextPaint(['client']);
+    }).toErrorDev(
+      'Warning: An error occurred during hydration. The server HTML was replaced with client content',
+      {withoutStack: true},
+    );
     expect(getVisibleChildren(container)).toEqual(<div>client</div>);
-    expect(ref.current).toEqual(serverRenderedDiv);
   });
 
   // @gate experimental

diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
@@ -197,12 +197,7 @@ describe('ReactDOMServerPartialHydration', () => {
     // hydrating anyway.
     suspend = true;
     ReactDOM.hydrateRoot(container, <App />);
-    expect(() => {
-      Scheduler.unstable_flushAll();
-    }).toErrorDev(
-      // TODO: This error should not be logged in this case. It's a false positive.
-      'Did not expect server HTML to contain the text node "Hello" in <div>.',
-    );
+    Scheduler.unstable_flushAll();
     jest.runAllTimers();
 
     // Expect the server-generated HTML to stay intact.
@@ -218,6 +213,101 @@ describe('ReactDOMServerPartialHydration', () => {
     expect(container.textContent).toBe('HelloHello');
   });
 
+  it('falls back to client rendering boundary on mismatch', async () => {
+    let client = false;
+    let suspend = false;
+    let resolve;
+    const promise = new Promise(resolvePromise => {
+      resolve = () => {
+        suspend = false;
+        resolvePromise();
+      };
+    });
+    function Child() {
+      if (suspend) {
+        Scheduler.unstable_yieldValue('Suspend');
+        throw promise;
+      } else {
+        Scheduler.unstable_yieldValue('Hello');
+        return 'Hello';
+      }
+    }
+    function Component({shouldMismatch}) {
+      Scheduler.unstable_yieldValue('Component');
+      if (shouldMismatch && client) {
+        return <article>Mismatch</article>;
+      }
+      return <div>Component</div>;
+    }
+    function App() {
+      return (
+        <Suspense fallback="Loading...">
+          <Child />
+          <Component />
+          <Component />
+          <Component />
+          <Component shouldMismatch={true} />
+        </Suspense>
+      );
+    }
+    const finalHTML = ReactDOMServer.renderToString(<App />);
+    const container = document.createElement('div');
+    container.innerHTML = finalHTML;
+    expect(Scheduler).toHaveYielded([
+      'Hello',
+      'Component',
+      'Component',
+      'Component',
+      'Component',
+    ]);
+
+    expect(container.innerHTML).toBe(
+      '<!--$-->Hello<div>Component</div><div>Component</div><div>Component</div><div>Component</div><!--/$-->',
+    );
+
+    suspend = true;
+    client = true;
+
+    ReactDOM.hydrateRoot(container, <App />);
+    expect(Scheduler).toFlushAndYield([
+      'Suspend',
+      'Component',
+      'Component',
+      'Component',
+      'Component',
+    ]);
+    jest.runAllTimers();
+
+    // Unchanged
+    expect(container.innerHTML).toBe(
+      '<!--$-->Hello<div>Component</div><div>Component</div><div>Component</div><div>Component</div><!--/$-->',
+    );
+
+    suspend = false;
+    resolve();
+    await promise;
+
+    expect(Scheduler).toFlushAndYield([
+      // first pass, mismatches at end
+      'Hello',
+      'Component',
+      'Component',
+      'Component',
+      'Component',
+      // second pass as client render
+      'Hello',
+      'Component',
+      'Component',
+      'Component',
+      'Component',
+    ]);
+
+    // Client rendered - suspense comment nodes removed
+    expect(container.innerHTML).toBe(
+      'Hello<div>Component</div><div>Component</div><div>Component</div><article>Mismatch</article>',
+    );
+  });
+
   it('calls the hydration callbacks after hydration or deletion', async () => {
     let suspend = false;
     let resolve;

diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.new.js b/packages/react-reconciler/src/ReactFiberHydrationContext.new.js
@@ -8,6 +8,7 @@
  */
 
 import type {Fiber} from './ReactInternalTypes';
+import {NoMode, ConcurrentMode} from './ReactTypeOfMode';
 import type {
   Instance,
   TextInstance,
@@ -323,12 +324,21 @@ function tryHydrate(fiber, nextInstance) {
   }
 }
 
+function throwOnHydrationMismatchIfConcurrentMode(fiber) {
+  if ((fiber.mode & ConcurrentMode) !== NoMode) {
+    throw new Error(
+      'An error occurred during hydration. The server HTML was replaced with client content',
+    );
+  }
+}
+
 function tryToClaimNextHydratableInstance(fiber: Fiber): void {
   if (!isHydrating) {
     return;
   }
   let nextInstance = nextHydratableInstance;
   if (!nextInstance) {
+    throwOnHydrationMismatchIfConcurrentMode(fiber);
     // Nothing to hydrate. Make it an insertion.
     insertNonHydratedInstance((hydrationParentFiber: any), fiber);
     isHydrating = false;
@@ -337,6 +347,7 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void {
   }
   const firstAttemptedInstance = nextInstance;
   if (!tryHydrate(fiber, nextInstance)) {
+    throwOnHydrationMismatchIfConcurrentMode(fiber);
     // If we can't hydrate this instance let's try the next one.
     // We use this as a heuristic. It's based on intuition and not data so it
     // might be flawed or unnecessary.

diff --git a/packages/react-reconciler/src/ReactFiberHydrationContext.old.js b/packages/react-reconciler/src/ReactFiberHydrationContext.old.js
@@ -8,6 +8,7 @@
  */
 
 import type {Fiber} from './ReactInternalTypes';
+import {NoMode, ConcurrentMode} from './ReactTypeOfMode';
 import type {
   Instance,
   TextInstance,
@@ -323,12 +324,21 @@ function tryHydrate(fiber, nextInstance) {
   }
 }
 
+function throwOnHydrationMismatchIfConcurrentMode(fiber) {
+  if ((fiber.mode & ConcurrentMode) !== NoMode) {
+    throw new Error(
+      'An error occurred during hydration. The server HTML was replaced with client content',
+    );
+  }
+}
+
 function tryToClaimNextHydratableInstance(fiber: Fiber): void {
   if (!isHydrating) {
     return;
   }
   let nextInstance = nextHydratableInstance;
   if (!nextInstance) {
+    throwOnHydrationMismatchIfConcurrentMode(fiber);
     // Nothing to hydrate. Make it an insertion.
     insertNonHydratedInstance((hydrationParentFiber: any), fiber);
     isHydrating = false;
@@ -337,6 +347,7 @@ function tryToClaimNextHydratableInstance(fiber: Fiber): void {
   }
   const firstAttemptedInstance = nextInstance;
   if (!tryHydrate(fiber, nextInstance)) {
+    throwOnHydrationMismatchIfConcurrentMode(fiber);
     // If we can't hydrate this instance let's try the next one.
     // We use this as a heuristic. It's based on intuition and not data so it
     // might be flawed or unnecessary.