From 03e6b8ba2fb1c2820d2a5286701ff2b40590b438 Mon Sep 17 00:00:00 2001
From: Andrew Clark <git@andrewclark.io>
Date: Mon, 25 May 2020 18:16:53 -0700
Subject: [PATCH] Make LegacyHidden match semantics of old fork (#18998)

Facebook currently relies on being able to hydrate hidden HTML. So
skipping those trees is a regression.

We don't have a proper solution for this in the new API yet. So I'm
reverting it to match the old behavior.

Now the server renderer will treat LegacyHidden the same as a fragment,
with no other special behavior. We can only get away with this because
we assume that every instance of LegacyHidden is accompanied by a host
component wrapper. In the hidden mode, the host component is given a
`hidden` attribute, which ensures that the initial HTML is not visible.
To support the use of LegacyHidden as a true fragment, without an extra
DOM node, we will have to hide the initial HTML in some other way.
---
 ...DOMServerPartialHydration-test.internal.js | 71 +++++++++++++------
 .../src/server/ReactPartialRenderer.js        | 20 +++---
 .../src/ReactFiberBeginWork.new.js            |  9 +--
 scripts/jest/TestFlags.js                     |  3 +
 4 files changed, 63 insertions(+), 40 deletions(-)

diff --git a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
index 01114efa5c..6080a26964 100644
--- a/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
+++ b/packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js
@@ -87,6 +87,38 @@ describe('ReactDOMServerPartialHydration', () => {
     SuspenseList = React.SuspenseList;
   });
 
+  // Note: This is based on a similar component we use in www. We can delete
+  // once the unstable_LegacyHidden API exists in both forks, and once the
+  // extra div wrapper is no longer neccessary.
+  function LegacyHiddenDiv({children, mode}) {
+    let wrappedChildren;
+    if (gate(flags => flags.new)) {
+      // The new reconciler does not support `<div hidden={true} />`. The
+      // equivalent behavior was moved to a special type, unstable_LegacyHidden.
+      // Eventually, we will replace this with an official API.
+      wrappedChildren = (
+        <React.unstable_LegacyHidden
+          mode={mode === 'hidden' ? 'unstable-defer-without-hiding' : mode}>
+          {children}
+        </React.unstable_LegacyHidden>
+      );
+    } else {
+      // The old reconciler fork does not support the new type. Use the old
+      // `<div hidden={true} />` API. Once we remove this branch, we can also
+      // remove the extra DOM node wrapper around the children.
+      wrappedChildren = children;
+    }
+
+    return (
+      <div
+        hidden={
+          mode === 'hidden' ? 'unstable-do-not-use-legacy-hidden' : undefined
+        }>
+        {wrappedChildren}
+      </div>
+    );
+  }
+
   // @gate experimental
   it('hydrates a parent even if a child Suspense boundary is blocked', async () => {
     let suspend = false;
@@ -2810,18 +2842,21 @@ describe('ReactDOMServerPartialHydration', () => {
     expect(ref.current).not.toBe(null);
   });
 
+  // This test fails, in both forks. Without a boundary, the deferred tree won't
+  // re-enter hydration mode. It doesn't come up in practice because there's
+  // always a parent Suspense boundary. But it's still a bug. Leaving for a
+  // follow up.
+  //
+  // @gate FIXME
   // @gate experimental
-  // @gate new
-  it('renders a hidden LegacyHidden component', async () => {
-    const LegacyHidden = React.unstable_LegacyHidden;
-
+  it('hydrates a hidden subtree outside of a Suspense boundary', async () => {
     const ref = React.createRef();
 
     function App() {
       return (
-        <LegacyHidden mode="hidden">
+        <LegacyHiddenDiv mode="hidden">
           <span ref={ref}>Hidden child</span>
-        </LegacyHidden>
+        </LegacyHiddenDiv>
       );
     }
 
@@ -2831,27 +2866,25 @@ describe('ReactDOMServerPartialHydration', () => {
     container.innerHTML = finalHTML;
 
     const span = container.getElementsByTagName('span')[0];
-    expect(span).toBe(undefined);
+    expect(span.innerHTML).toBe('Hidden child');
 
     const root = ReactDOM.createRoot(container, {hydrate: true});
     root.render(<App />);
     Scheduler.unstable_flushAll();
-    expect(ref.current.innerHTML).toBe('Hidden child');
+    expect(ref.current).toBe(span);
+    expect(span.innerHTML).toBe('Hidden child');
   });
 
   // @gate experimental
-  // @gate new
   it('renders a hidden LegacyHidden component inside a Suspense boundary', async () => {
-    const LegacyHidden = React.unstable_LegacyHidden;
-
     const ref = React.createRef();
 
     function App() {
       return (
         <Suspense fallback="Loading...">
-          <LegacyHidden mode="hidden">
+          <LegacyHiddenDiv mode="hidden">
             <span ref={ref}>Hidden child</span>
-          </LegacyHidden>
+          </LegacyHiddenDiv>
         </Suspense>
       );
     }
@@ -2862,26 +2895,24 @@ describe('ReactDOMServerPartialHydration', () => {
     container.innerHTML = finalHTML;
 
     const span = container.getElementsByTagName('span')[0];
-    expect(span).toBe(undefined);
+    expect(span.innerHTML).toBe('Hidden child');
 
     const root = ReactDOM.createRoot(container, {hydrate: true});
     root.render(<App />);
     Scheduler.unstable_flushAll();
-    expect(ref.current.innerHTML).toBe('Hidden child');
+    expect(ref.current).toBe(span);
+    expect(span.innerHTML).toBe('Hidden child');
   });
 
   // @gate experimental
-  // @gate new
   it('renders a visible LegacyHidden component', async () => {
-    const LegacyHidden = React.unstable_LegacyHidden;
-
     const ref = React.createRef();
 
     function App() {
       return (
-        <LegacyHidden mode="visible">
+        <LegacyHiddenDiv mode="visible">
           <span ref={ref}>Hidden child</span>
-        </LegacyHidden>
+        </LegacyHiddenDiv>
       );
     }
 
diff --git a/packages/react-dom/src/server/ReactPartialRenderer.js b/packages/react-dom/src/server/ReactPartialRenderer.js
index b2ef1778e3..6c700a1ac5 100644
--- a/packages/react-dom/src/server/ReactPartialRenderer.js
+++ b/packages/react-dom/src/server/ReactPartialRenderer.js
@@ -1020,18 +1020,14 @@ class ReactDOMServerRenderer {
       }
 
       switch (elementType) {
-        case REACT_LEGACY_HIDDEN_TYPE: {
-          if (!enableSuspenseServerRenderer) {
-            break;
-          }
-          if (((nextChild: any): ReactElement).props.mode === 'hidden') {
-            // In hidden mode, render nothing.
-            return '';
-          }
-          // Otherwise the tree is visible, so act like a fragment.
-        }
-        // Intentional fall through
-        // eslint-disable-next-line no-fallthrough
+        // TODO: LegacyHidden acts the same as a fragment. This only works
+        // because we currently assume that every instance of LegacyHidden is
+        // accompanied by a host component wrapper. In the hidden mode, the host
+        // component is given a `hidden` attribute, which ensures that the
+        // initial HTML is not visible. To support the use of LegacyHidden as a
+        // true fragment, without an extra DOM node, we would have to hide the
+        // initial HTML in some other way.
+        case REACT_LEGACY_HIDDEN_TYPE:
         case REACT_DEBUG_TRACING_MODE_TYPE:
         case REACT_STRICT_MODE_TYPE:
         case REACT_PROFILER_TYPE:
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
index a9666629b8..21c0835a33 100644
--- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
@@ -167,7 +167,6 @@ import {
   reenterHydrationStateFromDehydratedSuspenseInstance,
   resetHydrationState,
   tryToClaimNextHydratableInstance,
-  getIsHydrating,
   warnIfHydrating,
 } from './ReactFiberHydrationContext.new';
 import {
@@ -584,13 +583,7 @@ function updateOffscreenComponent(
       };
       workInProgress.memoizedState = nextState;
       pushRenderLanes(workInProgress, renderLanes);
-    } else if (
-      !includesSomeLane(renderLanes, (OffscreenLane: Lane)) ||
-      // Server renderer does not render hidden subtrees, so if we're hydrating
-      // we should always bail out and schedule a subsequent render pass, to
-      // force a client render. Even if we're already at Offscreen priority.
-      (current === null && getIsHydrating())
-    ) {
+    } else if (!includesSomeLane(renderLanes, (OffscreenLane: Lane))) {
       let nextBaseLanes;
       if (prevState !== null) {
         const prevBaseLanes = prevState.baseLanes;
diff --git a/scripts/jest/TestFlags.js b/scripts/jest/TestFlags.js
index f363e63488..bb8417c7a0 100644
--- a/scripts/jest/TestFlags.js
+++ b/scripts/jest/TestFlags.js
@@ -41,6 +41,9 @@ const environmentFlags = {
   experimental: __EXPERIMENTAL__,
   // Similarly, should stable imply "classic"?
   stable: !__EXPERIMENTAL__,
+
+  // Use this for tests that are known to be broken.
+  FIXME: false,
 };
 
 function getTestFlags() {