Inline mustUseProperty which is only used for special controlled comp…

…onents
facebook · Mar 30, 2023 · 7cbb062 · 7cbb062
1 parent ee1b716
commit 7cbb062
Show file tree

Hide file tree

Showing 6 changed files with 172 additions and 127 deletions.
diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js
@@ -38,11 +38,6 @@ export function getValueForProperty(
   propertyInfo: PropertyInfo,
 ): mixed {
   if (__DEV__) {
-    if (propertyInfo.mustUseProperty) {
-      const {propertyName} = propertyInfo;
-      return (node: any)[propertyName];
-    }
-
     const attributeName = propertyInfo.attributeName;
 
     if (!node.hasAttribute(attributeName)) {
@@ -292,16 +287,6 @@ export function setValueForProperty(
   propertyInfo: PropertyInfo,
   value: mixed,
 ) {
-  if (propertyInfo.mustUseProperty) {
-    // We assume mustUseProperty are of BOOLEAN type because that's the only way we use it
-    // right now.
-    (node: any)[propertyInfo.propertyName] =
-      value && typeof value !== 'function' && typeof value !== 'symbol';
-    return;
-  }
-
-  // The rest are treated as attributes with special cases.
-
   const attributeName = propertyInfo.attributeName;
 
   if (value === null) {

diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js
@@ -379,6 +379,18 @@ function setProp(
       }
       break;
     }
+    // Note: `option.selected` is not updated if `select.multiple` is
+    // disabled with `removeAttribute`. We have special logic for handling this.
+    case 'multiple': {
+      (domElement: any).multiple =
+        value && typeof value !== 'function' && typeof value !== 'symbol';
+      break;
+    }
+    case 'muted': {
+      (domElement: any).muted =
+        value && typeof value !== 'function' && typeof value !== 'symbol';
+      break;
+    }
     case 'suppressContentEditableWarning':
     case 'suppressHydrationWarning':
     case 'defaultValue': // Reserved
@@ -703,7 +715,19 @@ export function setInitialProperties(
         if (propValue == null) {
           continue;
         }
-        setProp(domElement, tag, propKey, propValue, false, props);
+        switch (propKey) {
+          case 'selected': {
+            // TODO: Remove support for selected on option.
+            (domElement: any).selected =
+              propValue &&
+              typeof propValue !== 'function' &&
+              typeof propValue !== 'symbol';
+            break;
+          }
+          default: {
+            setProp(domElement, tag, propKey, propValue, false, props);
+          }
+        }
       }
       ReactDOMOptionPostMountWrapper(domElement, props);
       return;
@@ -1018,6 +1042,26 @@ export function updateProperties(
       ReactDOMTextareaUpdateWrapper(domElement, nextProps);
       return;
     }
+    case 'option': {
+      for (let i = 0; i < updatePayload.length; i += 2) {
+        const propKey = updatePayload[i];
+        const propValue = updatePayload[i + 1];
+        switch (propKey) {
+          case 'selected': {
+            // TODO: Remove support for selected on option.
+            (domElement: any).selected =
+              propValue &&
+              typeof propValue !== 'function' &&
+              typeof propValue !== 'symbol';
+            break;
+          }
+          default: {
+            setProp(domElement, tag, propKey, propValue, false, nextProps);
+          }
+        }
+      }
+      return;
+    }
     case 'img':
     case 'link':
     case 'area':
@@ -1249,7 +1293,22 @@ function diffHydratedGenericElement(
         extraAttributeNames.delete(propKey);
         diffHydratedStyles(domElement, nextProp);
         continue;
-      // eslint-disable-next-line no-fallthrough
+      case 'multiple': {
+        extraAttributeNames.delete(propKey);
+        const serverValue = (domElement: any).multiple;
+        if (nextProp !== serverValue) {
+          warnForPropDifference('multiple', serverValue, nextProp);
+        }
+        continue;
+      }
+      case 'muted': {
+        extraAttributeNames.delete(propKey);
+        const serverValue = (domElement: any).muted;
+        if (nextProp !== serverValue) {
+          warnForPropDifference('muted', serverValue, nextProp);
+        }
+        continue;
+      }
       default:
         if (
           // shouldIgnoreAttribute

diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js
@@ -611,6 +611,16 @@ const attributeAssign = stringToPrecomputedChunk('="');
 const attributeEnd = stringToPrecomputedChunk('"');
 const attributeEmptyString = stringToPrecomputedChunk('=""');
 
+function pushBooleanAttribute(
+  target: Array<Chunk | PrecomputedChunk>,
+  name: string,
+  value: string | boolean | number | Function | Object, // not null or undefined
+): void {
+  if (value && typeof value !== 'function' && typeof value !== 'symbol') {
+    target.push(attributeSeparator, stringToChunk(name), attributeEmptyString);
+  }
+}
+
 function pushAttribute(
   target: Array<Chunk | PrecomputedChunk>,
   name: string,
@@ -628,6 +638,10 @@ function pushAttribute(
     case 'suppressHydrationWarning':
       // Ignored. These are built-in to React on the client.
       return;
+    case 'multiple':
+    case 'muted':
+      pushBooleanAttribute(target, name, value);
+      return;
   }
   if (
     // shouldIgnoreAttribute
@@ -1112,9 +1126,9 @@ function pushInput(
   }
 
   if (checked !== null) {
-    pushAttribute(target, 'checked', checked);
+    pushBooleanAttribute(target, 'checked', checked);
   } else if (defaultChecked !== null) {
-    pushAttribute(target, 'checked', defaultChecked);
+    pushBooleanAttribute(target, 'checked', defaultChecked);
   }
   if (value !== null) {
     pushAttribute(target, 'value', value);

diff --git a/packages/react-dom-bindings/src/shared/DOMProperty.js b/packages/react-dom-bindings/src/shared/DOMProperty.js
@@ -42,8 +42,6 @@ export type PropertyInfo = {
   +acceptsBooleans: boolean,
   +attributeName: string,
   +attributeNamespace: string | null,
-  +mustUseProperty: boolean,
-  +propertyName: string,
   +type: PropertyType,
   +sanitizeURL: boolean,
   +removeEmptyString: boolean,
@@ -55,9 +53,7 @@ export function getPropertyInfo(name: string): PropertyInfo | null {
 
 // $FlowFixMe[missing-this-annot]
 function PropertyInfoRecord(
-  name: string,
   type: PropertyType,
-  mustUseProperty: boolean,
   attributeName: string,
   attributeNamespace: string | null,
   sanitizeURL: boolean,
@@ -69,8 +65,6 @@ function PropertyInfoRecord(
     type === OVERLOADED_BOOLEAN;
   this.attributeName = attributeName;
   this.attributeNamespace = attributeNamespace;
-  this.mustUseProperty = mustUseProperty;
-  this.propertyName = name;
   this.type = type;
   this.sanitizeURL = sanitizeURL;
   this.removeEmptyString = removeEmptyString;
@@ -91,9 +85,7 @@ const properties: {[string]: $FlowFixMe} = {};
 ].forEach(([name, attributeName]) => {
   // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
   properties[name] = new PropertyInfoRecord(
-    name,
     STRING,
-    false, // mustUseProperty
     attributeName, // attributeName
     null, // attributeNamespace
     false, // sanitizeURL
@@ -107,9 +99,7 @@ const properties: {[string]: $FlowFixMe} = {};
 ['contentEditable', 'draggable', 'spellCheck', 'value'].forEach(name => {
   // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
   properties[name] = new PropertyInfoRecord(
-    name,
     BOOLEANISH_STRING,
-    false, // mustUseProperty
     name.toLowerCase(), // attributeName
     null, // attributeNamespace
     false, // sanitizeURL
@@ -129,9 +119,7 @@ const properties: {[string]: $FlowFixMe} = {};
 ].forEach(name => {
   // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
   properties[name] = new PropertyInfoRecord(
-    name,
     BOOLEANISH_STRING,
-    false, // mustUseProperty
     name, // attributeName
     null, // attributeNamespace
     false, // sanitizeURL
@@ -170,42 +158,14 @@ const properties: {[string]: $FlowFixMe} = {};
 ].forEach(name => {
   // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
   properties[name] = new PropertyInfoRecord(
-    name,
     BOOLEAN,
-    false, // mustUseProperty
     name.toLowerCase(), // attributeName
     null, // attributeNamespace
     false, // sanitizeURL
     false, // removeEmptyString
   );
 });
 
-// These are the few React props that we set as DOM properties
-// rather than attributes. These are all booleans.
-[
-  'checked',
-  // Note: `option.selected` is not updated if `select.multiple` is
-  // disabled with `removeAttribute`. We have special logic for handling this.
-  'multiple',
-  'muted',
-  'selected',
-
-  // NOTE: if you add a camelCased prop to this list,
-  // you'll need to set attributeName to name.toLowerCase()
-  // instead in the assignment below.
-].forEach(name => {
-  // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
-  properties[name] = new PropertyInfoRecord(
-    name,
-    BOOLEAN,
-    true, // mustUseProperty
-    name, // attributeName
-    null, // attributeNamespace
-    false, // sanitizeURL
-    false, // removeEmptyString
-  );
-});
-
 // These are HTML attributes that are "overloaded booleans": they behave like
 // booleans, but can also accept a string value.
 [
@@ -218,9 +178,7 @@ const properties: {[string]: $FlowFixMe} = {};
 ].forEach(name => {
   // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
   properties[name] = new PropertyInfoRecord(
-    name,
     OVERLOADED_BOOLEAN,
-    false, // mustUseProperty
     name, // attributeName
     null, // attributeNamespace
     false, // sanitizeURL
@@ -241,9 +199,7 @@ const properties: {[string]: $FlowFixMe} = {};
 ].forEach(name => {
   // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
   properties[name] = new PropertyInfoRecord(
-    name,
     POSITIVE_NUMERIC,
-    false, // mustUseProperty
     name, // attributeName
     null, // attributeNamespace
     false, // sanitizeURL
@@ -255,9 +211,7 @@ const properties: {[string]: $FlowFixMe} = {};
 ['rowSpan', 'start'].forEach(name => {
   // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
   properties[name] = new PropertyInfoRecord(
-    name,
     NUMERIC,
-    false, // mustUseProperty
     name.toLowerCase(), // attributeName
     null, // attributeNamespace
     false, // sanitizeURL
@@ -356,9 +310,7 @@ const capitalize = (token: string) => token[1].toUpperCase();
   const name = attributeName.replace(CAMELIZE, capitalize);
   // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
   properties[name] = new PropertyInfoRecord(
-    name,
     STRING,
-    false, // mustUseProperty
     attributeName,
     null, // attributeNamespace
     false, // sanitizeURL
@@ -382,9 +334,7 @@ const capitalize = (token: string) => token[1].toUpperCase();
   const name = attributeName.replace(CAMELIZE, capitalize);
   // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
   properties[name] = new PropertyInfoRecord(
-    name,
     STRING,
-    false, // mustUseProperty
     attributeName,
     'http://www.w3.org/1999/xlink',
     false, // sanitizeURL
@@ -405,9 +355,7 @@ const capitalize = (token: string) => token[1].toUpperCase();
   const name = attributeName.replace(CAMELIZE, capitalize);
   // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
   properties[name] = new PropertyInfoRecord(
-    name,
     STRING,
-    false, // mustUseProperty
     attributeName,
     'http://www.w3.org/XML/1998/namespace',
     false, // sanitizeURL
@@ -421,9 +369,7 @@ const capitalize = (token: string) => token[1].toUpperCase();
 ['tabIndex', 'crossOrigin'].forEach(attributeName => {
   // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
   properties[attributeName] = new PropertyInfoRecord(
-    attributeName,
     STRING,
-    false, // mustUseProperty
     attributeName.toLowerCase(), // attributeName
     null, // attributeNamespace
     false, // sanitizeURL
@@ -436,9 +382,7 @@ const capitalize = (token: string) => token[1].toUpperCase();
 const xlinkHref = 'xlinkHref';
 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
 properties[xlinkHref] = new PropertyInfoRecord(
-  'xlinkHref',
   STRING,
-  false, // mustUseProperty
   'xlink:href',
   'http://www.w3.org/1999/xlink',
   true, // sanitizeURL
@@ -448,9 +392,7 @@ properties[xlinkHref] = new PropertyInfoRecord(
 const formAction = 'formAction';
 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
 properties[formAction] = new PropertyInfoRecord(
-  'formAction',
   STRING,
-  false, // mustUseProperty
   'formaction', // attributeName
   null, // attributeNamespace
   true, // sanitizeURL
@@ -460,9 +402,7 @@ properties[formAction] = new PropertyInfoRecord(
 ['src', 'href', 'action'].forEach(attributeName => {
   // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
   properties[attributeName] = new PropertyInfoRecord(
-    attributeName,
     STRING,
-    false, // mustUseProperty
     attributeName.toLowerCase(), // attributeName
     null, // attributeNamespace
     true, // sanitizeURL