From b4a1a4980c98c6d8a7ced428a1adc9e278fec430 Mon Sep 17 00:00:00 2001
From: Andrew Clark <git@andrewclark.io>
Date: Wed, 13 May 2020 20:01:10 -0700
Subject: [PATCH] Disable <div hidden /> API in old fork, too (#18917)

The motivation for doing this is to make it impossible for additional
uses of pre-rendering to sneak into www without going through the
LegacyHidden abstraction. Since this feature was already disabled in
the new fork, this brings the two closer to parity.

The LegacyHidden abstraction itself still needs to opt into
pre-rendering somehow, so rather than totally disabling the feature, I
updated the `hidden` prop check to be obnoxiously specific. Before, you
could set it to any truthy value; now, you must set it to the string
"unstable-do-not-use-legacy-hidden".

The node will still be hidden in the DOM, since any truthy value will
cause the browser to apply a style of `display: none`.

I will have to update the LegacyHidden component in www to use the
obnoxious string prop. This doesn't block merge, though, since the
behavior is gated by a dynamic flag. I will update the component before
I enable the flag.
---
 .../src/__tests__/ReactUpdates-test.js          |  8 ++++++--
 .../react-dom/src/client/ReactDOMHostConfig.js  | 10 +++++++++-
 .../src/ReactFiberBeginWork.new.js              | 17 -----------------
 .../src/ReactFiberBeginWork.old.js              | 16 ----------------
 .../src/__tests__/ReactFresh-test.js            |  8 ++++++--
 .../__tests__/ReactDOMTracing-test.internal.js  |  8 ++++++--
 packages/shared/ReactFeatureFlags.js            |  2 +-
 .../shared/forks/ReactFeatureFlags.native-fb.js |  2 +-
 .../forks/ReactFeatureFlags.native-oss.js       |  2 +-
 .../forks/ReactFeatureFlags.test-renderer.js    |  2 +-
 .../ReactFeatureFlags.test-renderer.www.js      |  2 +-
 .../shared/forks/ReactFeatureFlags.testing.js   |  2 +-
 .../forks/ReactFeatureFlags.testing.www.js      |  2 +-
 .../forks/ReactFeatureFlags.www-dynamic.js      | 10 +++-------
 packages/shared/forks/ReactFeatureFlags.www.js  |  2 +-
 15 files changed, 38 insertions(+), 55 deletions(-)

diff --git a/packages/react-dom/src/__tests__/ReactUpdates-test.js b/packages/react-dom/src/__tests__/ReactUpdates-test.js
index 9c2b69b6533af..b82fa9de01142 100644
--- a/packages/react-dom/src/__tests__/ReactUpdates-test.js
+++ b/packages/react-dom/src/__tests__/ReactUpdates-test.js
@@ -29,7 +29,9 @@ describe('ReactUpdates', () => {
   function LegacyHiddenDiv({hidden, children, ...props}) {
     if (gate(flags => flags.new)) {
       return (
-        <div hidden={hidden} {...props}>
+        <div
+          hidden={hidden ? 'unstable-do-not-use-legacy-hidden' : false}
+          {...props}>
           <React.unstable_LegacyHidden mode={hidden ? 'hidden' : 'visible'}>
             {children}
           </React.unstable_LegacyHidden>
@@ -37,7 +39,9 @@ describe('ReactUpdates', () => {
       );
     } else {
       return (
-        <div hidden={hidden} {...props}>
+        <div
+          hidden={hidden ? 'unstable-do-not-use-legacy-hidden' : false}
+          {...props}>
           {children}
         </div>
       );
diff --git a/packages/react-dom/src/client/ReactDOMHostConfig.js b/packages/react-dom/src/client/ReactDOMHostConfig.js
index ee8995931785b..4e4755d830e3a 100644
--- a/packages/react-dom/src/client/ReactDOMHostConfig.js
+++ b/packages/react-dom/src/client/ReactDOMHostConfig.js
@@ -78,6 +78,7 @@ import {
   enableModernEventSystem,
   enableCreateEventHandleAPI,
   enableScopeAPI,
+  disableHiddenPropDeprioritization,
 } from 'shared/ReactFeatureFlags';
 import {HostComponent, HostText} from 'react-reconciler/src/ReactWorkTags';
 import {TOP_BEFORE_BLUR, TOP_AFTER_BLUR} from '../events/DOMTopLevelEventTypes';
@@ -372,7 +373,14 @@ export function shouldSetTextContent(type: string, props: Props): boolean {
 }
 
 export function shouldDeprioritizeSubtree(type: string, props: Props): boolean {
-  return !!props.hidden;
+  if (disableHiddenPropDeprioritization) {
+    // This is obnoxiously specific so that nobody uses it, but we can still opt
+    // in via an infra-level userspace abstraction.
+    return props.hidden === 'unstable-do-not-use-legacy-hidden';
+  } else {
+    // Legacy behavior. Any truthy value works.
+    return !!props.hidden;
+  }
 }
 
 export function createTextInstance(
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
index 553cfe4df5d63..2058bf403a4f5 100644
--- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
@@ -75,7 +75,6 @@ import {
   warnAboutDefaultPropsOnFunctionComponents,
   enableScopeAPI,
   enableBlocksAPI,
-  warnAboutDOMHiddenAttribute,
 } from 'shared/ReactFeatureFlags';
 import invariant from 'shared/invariant';
 import shallowEqual from 'shared/shallowEqual';
@@ -1124,22 +1123,6 @@ function updateHostComponent(
   }
 
   markRef(current, workInProgress);
-
-  if (__DEV__) {
-    if (
-      warnAboutDOMHiddenAttribute &&
-      (workInProgress.mode & ConcurrentMode) !== NoMode &&
-      nextProps.hasOwnProperty('hidden')
-    ) {
-      // This warning will not be user visible. Only exists so React Core team
-      // can find existing callers and migrate them to the new API.
-      console.error(
-        'Detected use of DOM `hidden` attribute. Should migrate to new API. ' +
-          '(owner: React Core)',
-      );
-    }
-  }
-
   reconcileChildren(current, workInProgress, nextChildren, renderLanes);
   return workInProgress.child;
 }
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js
index c61fc86f090d4..0a3a4e5a47317 100644
--- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js
+++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js
@@ -69,7 +69,6 @@ import {
   warnAboutDefaultPropsOnFunctionComponents,
   enableScopeAPI,
   enableBlocksAPI,
-  warnAboutDOMHiddenAttribute,
 } from 'shared/ReactFeatureFlags';
 import invariant from 'shared/invariant';
 import shallowEqual from 'shared/shallowEqual';
@@ -1100,21 +1099,6 @@ function updateHostComponent(current, workInProgress, renderExpirationTime) {
 
   markRef(current, workInProgress);
 
-  if (__DEV__) {
-    if (
-      warnAboutDOMHiddenAttribute &&
-      (workInProgress.mode & ConcurrentMode) !== NoMode &&
-      nextProps.hasOwnProperty('hidden')
-    ) {
-      // This warning will not be user visible. Only exists so React Core team
-      // can find existing callers and migrate them to the new API.
-      console.error(
-        'Detected use of DOM `hidden` attribute. Should migrate to new API. ' +
-          '(owner: React Core)',
-      );
-    }
-  }
-
   // Check the host config to see if the children are offscreen/hidden.
   if (
     workInProgress.mode & ConcurrentMode &&
diff --git a/packages/react-refresh/src/__tests__/ReactFresh-test.js b/packages/react-refresh/src/__tests__/ReactFresh-test.js
index 0e7702eb22892..11972b0f9d5e2 100644
--- a/packages/react-refresh/src/__tests__/ReactFresh-test.js
+++ b/packages/react-refresh/src/__tests__/ReactFresh-test.js
@@ -79,7 +79,9 @@ describe('ReactFresh', () => {
   function LegacyHiddenDiv({hidden, children, ...props}) {
     if (gate(flags => flags.new)) {
       return (
-        <div hidden={hidden} {...props}>
+        <div
+          hidden={hidden ? 'unstable-do-not-use-legacy-hidden' : false}
+          {...props}>
           <React.unstable_LegacyHidden mode={hidden ? 'hidden' : 'visible'}>
             {children}
           </React.unstable_LegacyHidden>
@@ -87,7 +89,9 @@ describe('ReactFresh', () => {
       );
     } else {
       return (
-        <div hidden={hidden} {...props}>
+        <div
+          hidden={hidden ? 'unstable-do-not-use-legacy-hidden' : false}
+          {...props}>
           {children}
         </div>
       );
diff --git a/packages/react/src/__tests__/ReactDOMTracing-test.internal.js b/packages/react/src/__tests__/ReactDOMTracing-test.internal.js
index a789953b85502..8f5478ee5b8fc 100644
--- a/packages/react/src/__tests__/ReactDOMTracing-test.internal.js
+++ b/packages/react/src/__tests__/ReactDOMTracing-test.internal.js
@@ -59,7 +59,9 @@ function loadModules() {
 function LegacyHiddenDiv({hidden, children, ...props}) {
   if (gate(flags => flags.new)) {
     return (
-      <div hidden={hidden} {...props}>
+      <div
+        hidden={hidden ? 'unstable-do-not-use-legacy-hidden' : false}
+        {...props}>
         <React.unstable_LegacyHidden mode={hidden ? 'hidden' : 'visible'}>
           {children}
         </React.unstable_LegacyHidden>
@@ -67,7 +69,9 @@ function LegacyHiddenDiv({hidden, children, ...props}) {
     );
   } else {
     return (
-      <div hidden={hidden} {...props}>
+      <div
+        hidden={hidden ? 'unstable-do-not-use-legacy-hidden' : false}
+        {...props}>
         {children}
       </div>
     );
diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js
index 3afba6de967ed..d899eef5ff4af 100644
--- a/packages/shared/ReactFeatureFlags.js
+++ b/packages/shared/ReactFeatureFlags.js
@@ -140,4 +140,4 @@ export const enableLegacyFBSupport = false;
 export const deferRenderPhaseUpdateToNextBatch = true;
 
 // Flag used by www build so we can log occurrences of legacy hidden API
-export const warnAboutDOMHiddenAttribute = false;
+export const disableHiddenPropDeprioritization = true;
diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js
index 08a7466fdf5dd..07288b896fc46 100644
--- a/packages/shared/forks/ReactFeatureFlags.native-fb.js
+++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js
@@ -48,7 +48,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
 
 export const enableNewReconciler = false;
 export const deferRenderPhaseUpdateToNextBatch = true;
-export const warnAboutDOMHiddenAttribute = false;
+export const disableHiddenPropDeprioritization = true;
 
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js
index 73b03f2e042c8..53933810e3b7b 100644
--- a/packages/shared/forks/ReactFeatureFlags.native-oss.js
+++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js
@@ -47,7 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
 
 export const enableNewReconciler = false;
 export const deferRenderPhaseUpdateToNextBatch = true;
-export const warnAboutDOMHiddenAttribute = false;
+export const disableHiddenPropDeprioritization = true;
 
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js
index a8a5f26d6607c..23b2f0a75e4ed 100644
--- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js
+++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js
@@ -47,7 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
 
 export const enableNewReconciler = false;
 export const deferRenderPhaseUpdateToNextBatch = true;
-export const warnAboutDOMHiddenAttribute = false;
+export const disableHiddenPropDeprioritization = true;
 
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
index 3ac44b769bc73..12ccc8a05fc8e 100644
--- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
@@ -47,7 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
 
 export const enableNewReconciler = false;
 export const deferRenderPhaseUpdateToNextBatch = true;
-export const warnAboutDOMHiddenAttribute = false;
+export const disableHiddenPropDeprioritization = true;
 
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js
index acdc78b483765..97d5f5617ecc0 100644
--- a/packages/shared/forks/ReactFeatureFlags.testing.js
+++ b/packages/shared/forks/ReactFeatureFlags.testing.js
@@ -47,7 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
 
 export const enableNewReconciler = false;
 export const deferRenderPhaseUpdateToNextBatch = true;
-export const warnAboutDOMHiddenAttribute = false;
+export const disableHiddenPropDeprioritization = true;
 
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js
index 1da0e18cb8280..e6b65efc3fe49 100644
--- a/packages/shared/forks/ReactFeatureFlags.testing.www.js
+++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js
@@ -47,7 +47,7 @@ export const enableFilterEmptyStringAttributesDOM = false;
 
 export const enableNewReconciler = false;
 export const deferRenderPhaseUpdateToNextBatch = true;
-export const warnAboutDOMHiddenAttribute = false;
+export const disableHiddenPropDeprioritization = true;
 
 // Flow magic to verify the exports of this file match the original version.
 // eslint-disable-next-line no-unused-vars
diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js
index af65097e7c604..e0365704e2655 100644
--- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js
+++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js
@@ -21,6 +21,9 @@ export const enableModernEventSystem = __VARIANT__;
 export const enableLegacyFBSupport = __VARIANT__;
 export const enableDebugTracing = !__VARIANT__;
 
+// Temporary flag, in case we need to re-enable this feature.
+export const disableHiddenPropDeprioritization = __VARIANT__;
+
 // This only has an effect in the new reconciler. But also, the new reconciler
 // is only enabled when __VARIANT__ is true. So this is set to the opposite of
 // __VARIANT__ so that it's `false` when running against the new reconciler.
@@ -36,13 +39,6 @@ export const deferRenderPhaseUpdateToNextBatch = !__VARIANT__;
 export const debugRenderPhaseSideEffectsForStrictMode = __DEV__;
 export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
 
-// Do not add the corresponding warning to the warning filter! Only exists so we
-// can detect callers and migrate them to the new API. Should not visible to
-// anyone outside React Core team.
-//
-// Disabled in our tests, but we'll enable in www.
-export const warnAboutDOMHiddenAttribute = false;
-
 // TODO: These flags are hard-coded to the default values used in open source.
 // Update the tests so that they pass in either mode, then set these
 // to __VARIANT__.
diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js
index 4f49471f1b7a5..893ca68637411 100644
--- a/packages/shared/forks/ReactFeatureFlags.www.js
+++ b/packages/shared/forks/ReactFeatureFlags.www.js
@@ -27,7 +27,7 @@ export const {
   enableLegacyFBSupport,
   enableDebugTracing,
   deferRenderPhaseUpdateToNextBatch,
-  warnAboutDOMHiddenAttribute,
+  disableHiddenPropDeprioritization,
 } = dynamicFeatureFlags;
 
 // On WWW, __EXPERIMENTAL__ is used for a new modern build.