Land behind flag

facebook · Feb 28, 2024 · b513735 · b513735
1 parent 8d31258
commit b513735
Show file tree

Hide file tree

Showing 11 changed files with 160 additions and 10 deletions.
diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js
@@ -78,6 +78,7 @@ import {
   mediaEventTypes,
   listenToNonDelegatedEvent,
 } from '../events/DOMPluginEventSystem';
+import {enableNewDOMProps} from '../../../shared/ReactFeatureFlags';
 
 let didWarnControlledToUncontrolled = false;
 let didWarnUncontrolledToControlled = false;
@@ -741,8 +742,17 @@ function setProp(
       break;
     }
     // Overloaded Boolean
-    case 'capture':
     case 'hidden':
+      if (!enableNewDOMProps) {
+        if (value && typeof value !== 'function' && typeof value !== 'symbol') {
+          domElement.setAttribute(key, '');
+        } else {
+          domElement.removeAttribute(key);
+        }
+        break;
+      }
+    // fallthrough to overloaded boolean with enableNewDOMProps
+    case 'capture':
     case 'download': {
       // An attribute that can be used as a flag as well as with a value.
       // When true, it should be present (set either to an empty string or its name).
@@ -2528,8 +2538,20 @@ function diffHydratedGenericElement(
         );
         continue;
       }
-      case 'capture':
       case 'hidden':
+        if (!enableNewDOMProps) {
+          // Some of these need to be lower case to remove them from the extraAttributes list.
+          hydrateBooleanAttribute(
+            domElement,
+            propKey,
+            propKey.toLowerCase(),
+            value,
+            extraAttributes,
+          );
+          continue;
+        }
+      // fallthrough to overloaded boolean with enableNewDOMProps
+      case 'capture':
       case 'download': {
         hydrateOverloadedBooleanAttribute(
           domElement,

diff --git a/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js b/packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
@@ -34,6 +34,7 @@ import {
   enableFloat,
   enableFormActions,
   enableFizzExternalRuntime,
+  enableNewDOMProps,
 } from 'shared/ReactFeatureFlags';
 
 import type {
@@ -1311,8 +1312,19 @@ function pushAttribute(
       }
       return;
     }
-    case 'capture':
     case 'hidden':
+      if (!enableNewDOMProps) {
+        // Boolean
+        if (value && typeof value !== 'function' && typeof value !== 'symbol') {
+          target.push(
+            attributeSeparator,
+            stringToChunk(name),
+            attributeEmptyString,
+          );
+        }
+      }
+    // fallthrough to overloaded boolean with enableNewDOMProps
+    case 'capture':
     case 'download': {
       // Overloaded Boolean
       if (value === true) {
@@ -4994,7 +5006,16 @@ function writeStyleResourceAttributeInJS(
       if (value === false) {
         return;
       }
-      attributeValue = '';
+      if (enableNewDOMProps) {
+        // overloaded Boolean
+        if (__DEV__) {
+          checkAttributeStringCoercion(value, attributeName);
+        }
+        attributeValue = '' + (value: any);
+      } else {
+        // just Boolean
+        attributeValue = '';
+      }
       break;
     }
     // Santized URLs
@@ -5189,10 +5210,16 @@ function writeStyleResourceAttributeInAttr(
       if (value === false) {
         return;
       }
-      if (__DEV__) {
-        checkAttributeStringCoercion(value, attributeName);
+      if (enableNewDOMProps) {
+        // overloaded Boolean
+        if (__DEV__) {
+          checkAttributeStringCoercion(value, attributeName);
+        }
+        attributeValue = '' + (value: any);
+      } else {
+        // just Boolean
+        attributeValue = '';
       }
-      attributeValue = '' + (value: any);
       break;
     }
 

diff --git a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js
@@ -12,6 +12,7 @@ import hasOwnProperty from 'shared/hasOwnProperty';
 import {
   enableCustomElementPropertySupport,
   enableFormActions,
+  enableNewDOMProps,
 } from 'shared/ReactFeatureFlags';
 
 const warnedProperties = {};
@@ -223,6 +224,7 @@ function validateProperty(tagName, name, value, eventRegistry) {
           case 'disablePictureInPicture':
           case 'disableRemotePlayback':
           case 'formNoValidate':
+          case 'hidden':
           case 'loop':
           case 'noModule':
           case 'noValidate':
@@ -235,7 +237,6 @@ function validateProperty(tagName, name, value, eventRegistry) {
           case 'seamless':
           case 'itemScope':
           case 'capture':
-          case 'hidden':
           case 'download': {
             // Boolean properties can accept boolean values
             return true;
@@ -313,6 +314,12 @@ function validateProperty(tagName, name, value, eventRegistry) {
             case 'itemScope': {
               break;
             }
+            case 'hidden': {
+              if (!enableNewDOMProps) {
+                break;
+              }
+            }
+            // fallthrough to overloaded boolean with enableNewDOMProps
             default: {
               return true;
             }

diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationAttributes-test.js
@@ -33,8 +33,14 @@ function initModules() {
   };
 }
 
-const {resetModules, itRenders, clientCleanRender} =
-  ReactDOMServerIntegrationUtils(initModules);
+const {
+  resetModules,
+  itRenders,
+  clientCleanRender,
+  clientRenderOnServerString,
+  serverRender,
+  streamRender,
+} = ReactDOMServerIntegrationUtils(initModules);
 
 describe('ReactDOMServerIntegration', () => {
   beforeEach(() => {
@@ -230,6 +236,77 @@ describe('ReactDOMServerIntegration', () => {
       });
     });
 
+    describe('hidden property (combined boolean/string attribute)', function () {
+      itRenders('hidden prop with true value', async render => {
+        const e = await render(<div hidden={true} />);
+        expect(e.getAttribute('hidden')).toBe('');
+      });
+
+      itRenders('hidden prop with false value', async render => {
+        const e = await render(<div hidden={false} />);
+        expect(e.getAttribute('hidden')).toBe(null);
+      });
+
+      itRenders('hidden prop with string value', async render => {
+        const e = await render(<div hidden="until-found" />);
+        expect(e.getAttribute('hidden')).toBe(
+          ReactFeatureFlags.enableNewDOMProps ? 'until-found' : '',
+        );
+      });
+
+      itRenders('hidden prop with string "false" value', async render => {
+        const e = await render(
+          <div hidden="false" />,
+          ReactFeatureFlags.enableNewDOMProps ? 0 : 1,
+        );
+        expect(e.getAttribute('hidden')).toBe(
+          ReactFeatureFlags.enableNewDOMProps ? 'false' : '',
+        );
+      });
+
+      itRenders('hidden prop with string "true" value', async render => {
+        const e = await render(
+          <div hidden={'true'} />,
+          ReactFeatureFlags.enableNewDOMProps ? 0 : 1,
+        );
+        expect(e.getAttribute('hidden')).toBe(
+          ReactFeatureFlags.enableNewDOMProps ? 'true' : '',
+        );
+      });
+
+      itRenders('hidden prop with number 0 value', async render => {
+        const e = await render(<div hidden={0} />);
+        expect(e.getAttribute('hidden')).toBe(
+          ReactFeatureFlags.enableNewDOMProps ||
+            render === clientRenderOnServerString ||
+            render === serverRender ||
+            render === streamRender
+            ? '0'
+            : null,
+        );
+      });
+
+      itRenders('no hidden prop with null value', async render => {
+        const e = await render(<div hidden={null} />);
+        expect(e.hasAttribute('hidden')).toBe(false);
+      });
+
+      itRenders('no hidden prop with undefined value', async render => {
+        const e = await render(<div hidden={undefined} />);
+        expect(e.hasAttribute('hidden')).toBe(false);
+      });
+
+      itRenders('no hidden prop with function value', async render => {
+        const e = await render(<div hidden={function () {}} />, 1);
+        expect(e.hasAttribute('hidden')).toBe(false);
+      });
+
+      itRenders('no hidden prop with symbol value', async render => {
+        const e = await render(<div hidden={Symbol('foo')} />, 1);
+        expect(e.hasAttribute('hidden')).toBe(false);
+      });
+    });
+
     describe('className property', function () {
       itRenders('className prop with string value', async render => {
         const e = await render(<div className="myClassName" />);

diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js
@@ -195,6 +195,12 @@ export const disableStringRefs = __NEXT_MAJOR__;
 // Warn on any usage of ReactTestRenderer
 export const enableReactTestRendererWarning = false;
 
+/**
+ * Catch-all for all changes to how we treat new HTML attributes.
+ * When attributes are newly recognized by React, or change their type, the rendered output changes.
+ */
+export const enableNewDOMProps = __NEXT_MAJOR__;
+
 // -----------------------------------------------------------------------------
 // Chopping Block
 //

diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js
@@ -107,5 +107,7 @@ export const enableReactTestRendererWarning = false;
 
 export const enableBigIntSupport = false;
 
+export const enableNewDOMProps = true;
+
 // Flow magic to verify the exports of this file match the original version.
 ((((null: any): ExportsType): FeatureFlagsType): ExportsType);
diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js
@@ -98,5 +98,7 @@ export const enableReactTestRendererWarning = false;
 
 export const enableBigIntSupport = false;
 
+export const enableNewDOMProps = true;
+
 // Flow magic to verify the exports of this file match the original version.
 ((((null: any): ExportsType): FeatureFlagsType): ExportsType);
diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js
@@ -101,6 +101,7 @@ export const enableRefAsProp = __NEXT_MAJOR__;
 export const disableStringRefs = __NEXT_MAJOR__;
 export const enableReactTestRendererWarning = false;
 export const enableBigIntSupport = __NEXT_MAJOR__;
+export const enableNewDOMProps = __NEXT_MAJOR__;
 
 // Flow magic to verify the exports of this file match the original version.
 ((((null: any): ExportsType): FeatureFlagsType): ExportsType);
diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
@@ -94,5 +94,7 @@ export const enableReactTestRendererWarning = false;
 
 export const enableBigIntSupport = false;
 
+export const enableNewDOMProps = true;
+
 // Flow magic to verify the exports of this file match the original version.
 ((((null: any): ExportsType): FeatureFlagsType): ExportsType);
diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
@@ -97,5 +97,7 @@ export const enableReactTestRendererWarning = false;
 
 export const enableBigIntSupport = false;
 
+export const enableNewDOMProps = false;
+
 // Flow magic to verify the exports of this file match the original version.
 ((((null: any): ExportsType): FeatureFlagsType): ExportsType);
diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js
@@ -122,6 +122,8 @@ export const enableReactTestRendererWarning = false;
 
 export const enableBigIntSupport = false;
 
+export const enableNewDOMProps = false;
+
 // TODO: Roll out with GK. Don't keep as dynamic flag for too long, though,
 // because JSX is an extremely hot path.
 export const disableStringRefs = false;