From 8e50e754b92140ef3f0e69b9ef15af86496e44c8 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 29 Mar 2023 16:00:20 -0400 Subject: [PATCH 1/7] Move getPropertyInfo split into ReactDOMComponent --- .../src/client/DOMPropertyOperations.js | 234 +++++++++--------- .../src/client/ReactDOMComponent.js | 18 +- 2 files changed, 131 insertions(+), 121 deletions(-) diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js index c27066b0cc31d..68332b177fda4 100644 --- a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js @@ -8,7 +8,6 @@ */ import { - getPropertyInfo, isAttributeNameSafe, BOOLEAN, OVERLOADED_BOOLEAN, @@ -287,152 +286,147 @@ export function getValueForAttributeOnCustomComponent( * @param {string} name * @param {*} value */ -export function setValueForProperty(node: Element, name: string, value: mixed) { - if ( - // shouldIgnoreAttribute - // We have already filtered out reserved words. - name.length > 2 && - (name[0] === 'o' || name[0] === 'O') && - (name[1] === 'n' || name[1] === 'N') - ) { +export function setValueForProperty( + node: Element, + 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; } - const propertyInfo = getPropertyInfo(name); - if (propertyInfo !== null) { - 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. - // The rest are treated as attributes with special cases. + const attributeName = propertyInfo.attributeName; - const attributeName = propertyInfo.attributeName; + if (value === null) { + node.removeAttribute(attributeName); + return; + } - if (value === null) { + // shouldRemoveAttribute + switch (typeof value) { + case 'undefined': + case 'function': + case 'symbol': // eslint-disable-line node.removeAttribute(attributeName); return; - } - - // shouldRemoveAttribute - switch (typeof value) { - case 'undefined': - case 'function': - case 'symbol': // eslint-disable-line + case 'boolean': { + if (!propertyInfo.acceptsBooleans) { node.removeAttribute(attributeName); return; - case 'boolean': { - if (!propertyInfo.acceptsBooleans) { - node.removeAttribute(attributeName); - return; - } } } - if (enableFilterEmptyStringAttributesDOM) { - if (propertyInfo.removeEmptyString && value === '') { - if (__DEV__) { - if (name === 'src') { - console.error( - 'An empty string ("") was passed to the %s attribute. ' + - 'This may cause the browser to download the whole page again over the network. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to %s instead of an empty string.', - name, - name, - ); - } else { - console.error( - 'An empty string ("") was passed to the %s attribute. ' + - 'To fix this, either do not render the element at all ' + - 'or pass null to %s instead of an empty string.', - name, - name, - ); - } + } + if (enableFilterEmptyStringAttributesDOM) { + if (propertyInfo.removeEmptyString && value === '') { + if (__DEV__) { + if (attributeName === 'src') { + console.error( + 'An empty string ("") was passed to the %s attribute. ' + + 'This may cause the browser to download the whole page again over the network. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to %s instead of an empty string.', + attributeName, + attributeName, + ); + } else { + console.error( + 'An empty string ("") was passed to the %s attribute. ' + + 'To fix this, either do not render the element at all ' + + 'or pass null to %s instead of an empty string.', + attributeName, + attributeName, + ); } - node.removeAttribute(attributeName); - return; } + node.removeAttribute(attributeName); + return; } + } - switch (propertyInfo.type) { - case BOOLEAN: - if (value) { - node.setAttribute(attributeName, ''); - } else { - node.removeAttribute(attributeName); - return; - } - break; - case OVERLOADED_BOOLEAN: - if (value === true) { - node.setAttribute(attributeName, ''); - } else if (value === false) { - node.removeAttribute(attributeName); - } else { - if (__DEV__) { - checkAttributeStringCoercion(value, attributeName); - } - node.setAttribute(attributeName, (value: any)); - } + switch (propertyInfo.type) { + case BOOLEAN: + if (value) { + node.setAttribute(attributeName, ''); + } else { + node.removeAttribute(attributeName); return; - case NUMERIC: - if (!isNaN(value)) { - if (__DEV__) { - checkAttributeStringCoercion(value, attributeName); - } - node.setAttribute(attributeName, (value: any)); - } else { - node.removeAttribute(attributeName); + } + break; + case OVERLOADED_BOOLEAN: + if (value === true) { + node.setAttribute(attributeName, ''); + } else if (value === false) { + node.removeAttribute(attributeName); + } else { + if (__DEV__) { + checkAttributeStringCoercion(value, attributeName); } - break; - case POSITIVE_NUMERIC: - if (!isNaN(value) && (value: any) >= 1) { - if (__DEV__) { - checkAttributeStringCoercion(value, attributeName); - } - node.setAttribute(attributeName, (value: any)); - } else { - node.removeAttribute(attributeName); + node.setAttribute(attributeName, (value: any)); + } + return; + case NUMERIC: + if (!isNaN(value)) { + if (__DEV__) { + checkAttributeStringCoercion(value, attributeName); } - break; - default: { + node.setAttribute(attributeName, (value: any)); + } else { + node.removeAttribute(attributeName); + } + break; + case POSITIVE_NUMERIC: + if (!isNaN(value) && (value: any) >= 1) { if (__DEV__) { checkAttributeStringCoercion(value, attributeName); } - let attributeValue; - // `setAttribute` with objects becomes only `[object]` in IE8/9, - // ('' + value) makes it output the correct toString()-value. - if (enableTrustedTypesIntegration) { - if (propertyInfo.sanitizeURL) { - attributeValue = (sanitizeURL(value): any); - } else { - attributeValue = (value: any); - } + node.setAttribute(attributeName, (value: any)); + } else { + node.removeAttribute(attributeName); + } + break; + default: { + if (__DEV__) { + checkAttributeStringCoercion(value, attributeName); + } + let attributeValue; + // `setAttribute` with objects becomes only `[object]` in IE8/9, + // ('' + value) makes it output the correct toString()-value. + if (enableTrustedTypesIntegration) { + if (propertyInfo.sanitizeURL) { + attributeValue = (sanitizeURL(value): any); } else { - // We have already verified this above. - // eslint-disable-next-line react-internal/safe-string-coercion - attributeValue = '' + (value: any); - if (propertyInfo.sanitizeURL) { - attributeValue = sanitizeURL(attributeValue); - } + attributeValue = (value: any); } - const attributeNamespace = propertyInfo.attributeNamespace; - if (attributeNamespace) { - node.setAttributeNS( - attributeNamespace, - attributeName, - attributeValue, - ); - } else { - node.setAttribute(attributeName, attributeValue); + } else { + // We have already verified this above. + // eslint-disable-next-line react-internal/safe-string-coercion + attributeValue = '' + (value: any); + if (propertyInfo.sanitizeURL) { + attributeValue = sanitizeURL(attributeValue); } } + const attributeNamespace = propertyInfo.attributeNamespace; + if (attributeNamespace) { + node.setAttributeNS(attributeNamespace, attributeName, attributeValue); + } else { + node.setAttribute(attributeName, attributeValue); + } } - } else if (isAttributeNameSafe(name)) { + } +} + +export function setValueForAttribute( + node: Element, + name: string, + value: mixed, +) { + if (isAttributeNameSafe(name)) { // If the prop isn't in the special list, treat it as a simple attribute. // shouldRemoveAttribute if (value === null) { diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index c1f5ed366ae2a..bf527f3d3659a 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -24,6 +24,7 @@ import { getValueForProperty, setValueForProperty, setValueForPropertyOnCustomComponent, + setValueForAttribute, } from './DOMPropertyOperations'; import { initWrapperState as ReactDOMInputInitWrapperState, @@ -408,7 +409,22 @@ function setProp( if (isCustomComponentTag) { setValueForPropertyOnCustomComponent(domElement, key, value); } else { - setValueForProperty(domElement, key, value); + if ( + // shouldIgnoreAttribute + // We have already filtered out reserved words. + key.length > 2 && + (key[0] === 'o' || key[0] === 'O') && + (key[1] === 'n' || key[1] === 'N') + ) { + return; + } + + const propertyInfo = getPropertyInfo(key); + if (propertyInfo !== null) { + setValueForProperty(domElement, propertyInfo, value); + } else { + setValueForAttribute(domElement, key, value); + } } } } From e38f229deeeea52e996ef79748e02828ca494528 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 29 Mar 2023 16:05:44 -0400 Subject: [PATCH 2/7] Move isAttributeNameSafe to separate module --- .../src/client/DOMPropertyOperations.js | 3 +- .../src/server/ReactDOMServerFormatConfig.js | 2 +- .../src/shared/DOMProperty.js | 34 --------------- .../src/shared/ReactDOMInvalidARIAHook.js | 2 +- .../src/shared/ReactDOMUnknownPropertyHook.js | 3 +- .../src/shared/isAttributeNameSafe.js | 42 +++++++++++++++++++ 6 files changed, 48 insertions(+), 38 deletions(-) create mode 100644 packages/react-dom-bindings/src/shared/isAttributeNameSafe.js diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js index 68332b177fda4..fae87f3d1c0f9 100644 --- a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js @@ -8,12 +8,13 @@ */ import { - isAttributeNameSafe, BOOLEAN, OVERLOADED_BOOLEAN, NUMERIC, POSITIVE_NUMERIC, } from '../shared/DOMProperty'; + +import isAttributeNameSafe from '../shared/isAttributeNameSafe'; import sanitizeURL from '../shared/sanitizeURL'; import { enableTrustedTypesIntegration, diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index 543fa064ea388..a85a18a7c967b 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -38,9 +38,9 @@ import { clonePrecomputedChunk, } from 'react-server/src/ReactServerStreamConfig'; +import isAttributeNameSafe from '../shared/isAttributeNameSafe'; import { getPropertyInfo, - isAttributeNameSafe, BOOLEAN, OVERLOADED_BOOLEAN, NUMERIC, diff --git a/packages/react-dom-bindings/src/shared/DOMProperty.js b/packages/react-dom-bindings/src/shared/DOMProperty.js index 6b3178418551c..52d197fac5575 100644 --- a/packages/react-dom-bindings/src/shared/DOMProperty.js +++ b/packages/react-dom-bindings/src/shared/DOMProperty.js @@ -7,8 +7,6 @@ * @flow */ -import hasOwnProperty from 'shared/hasOwnProperty'; - type PropertyType = 0 | 1 | 2 | 3 | 4 | 5 | 6; // A simple string attribute. @@ -51,38 +49,6 @@ export type PropertyInfo = { +removeEmptyString: boolean, }; -/* eslint-disable max-len */ -export const ATTRIBUTE_NAME_START_CHAR = - ':A-Z_a-z\\u00C0-\\u00D6\\u00D8-\\u00F6\\u00F8-\\u02FF\\u0370-\\u037D\\u037F-\\u1FFF\\u200C-\\u200D\\u2070-\\u218F\\u2C00-\\u2FEF\\u3001-\\uD7FF\\uF900-\\uFDCF\\uFDF0-\\uFFFD'; -/* eslint-enable max-len */ -export const ATTRIBUTE_NAME_CHAR: string = - ATTRIBUTE_NAME_START_CHAR + '\\-.0-9\\u00B7\\u0300-\\u036F\\u203F-\\u2040'; - -export const VALID_ATTRIBUTE_NAME_REGEX: RegExp = new RegExp( - '^[' + ATTRIBUTE_NAME_START_CHAR + '][' + ATTRIBUTE_NAME_CHAR + ']*$', -); - -const illegalAttributeNameCache: {[string]: boolean} = {}; -const validatedAttributeNameCache: {[string]: boolean} = {}; - -export function isAttributeNameSafe(attributeName: string): boolean { - if (hasOwnProperty.call(validatedAttributeNameCache, attributeName)) { - return true; - } - if (hasOwnProperty.call(illegalAttributeNameCache, attributeName)) { - return false; - } - if (VALID_ATTRIBUTE_NAME_REGEX.test(attributeName)) { - validatedAttributeNameCache[attributeName] = true; - return true; - } - illegalAttributeNameCache[attributeName] = true; - if (__DEV__) { - console.error('Invalid attribute name: `%s`', attributeName); - } - return false; -} - export function getPropertyInfo(name: string): PropertyInfo | null { return properties.hasOwnProperty(name) ? properties[name] : null; } diff --git a/packages/react-dom-bindings/src/shared/ReactDOMInvalidARIAHook.js b/packages/react-dom-bindings/src/shared/ReactDOMInvalidARIAHook.js index 2eab4ffdfb643..1546c32ed965e 100644 --- a/packages/react-dom-bindings/src/shared/ReactDOMInvalidARIAHook.js +++ b/packages/react-dom-bindings/src/shared/ReactDOMInvalidARIAHook.js @@ -5,7 +5,7 @@ * LICENSE file in the root directory of this source tree. */ -import {ATTRIBUTE_NAME_CHAR} from './DOMProperty'; +import {ATTRIBUTE_NAME_CHAR} from './isAttributeNameSafe'; import isCustomComponent from './isCustomComponent'; import validAriaProperties from './validAriaProperties'; import hasOwnProperty from 'shared/hasOwnProperty'; diff --git a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js index 91ac166d13760..bd4312c57dd60 100644 --- a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js +++ b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js @@ -5,7 +5,8 @@ * LICENSE file in the root directory of this source tree. */ -import {ATTRIBUTE_NAME_CHAR, BOOLEAN, getPropertyInfo} from './DOMProperty'; +import {BOOLEAN, getPropertyInfo} from './DOMProperty'; +import {ATTRIBUTE_NAME_CHAR} from './isAttributeNameSafe'; import isCustomComponent from './isCustomComponent'; import possibleStandardNames from './possibleStandardNames'; import hasOwnProperty from 'shared/hasOwnProperty'; diff --git a/packages/react-dom-bindings/src/shared/isAttributeNameSafe.js b/packages/react-dom-bindings/src/shared/isAttributeNameSafe.js new file mode 100644 index 0000000000000..d1378a64de37c --- /dev/null +++ b/packages/react-dom-bindings/src/shared/isAttributeNameSafe.js @@ -0,0 +1,42 @@ +/** + * Copyright (c) Meta Platforms, Inc. and affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @flow + */ + +import hasOwnProperty from 'shared/hasOwnProperty'; + +/* eslint-disable max-len */ +const ATTRIBUTE_NAME_START_CHAR = + ':A-Z_a-z\\u00C0-\\u00D6\\u00D8-\\u00F6\\u00F8-\\u02FF\\u0370-\\u037D\\u037F-\\u1FFF\\u200C-\\u200D\\u2070-\\u218F\\u2C00-\\u2FEF\\u3001-\\uD7FF\\uF900-\\uFDCF\\uFDF0-\\uFFFD'; +/* eslint-enable max-len */ +export const ATTRIBUTE_NAME_CHAR: string = + ATTRIBUTE_NAME_START_CHAR + '\\-.0-9\\u00B7\\u0300-\\u036F\\u203F-\\u2040'; + +const VALID_ATTRIBUTE_NAME_REGEX: RegExp = new RegExp( + '^[' + ATTRIBUTE_NAME_START_CHAR + '][' + ATTRIBUTE_NAME_CHAR + ']*$', +); + +const illegalAttributeNameCache: {[string]: boolean} = {}; +const validatedAttributeNameCache: {[string]: boolean} = {}; + +export default function isAttributeNameSafe(attributeName: string): boolean { + if (hasOwnProperty.call(validatedAttributeNameCache, attributeName)) { + return true; + } + if (hasOwnProperty.call(illegalAttributeNameCache, attributeName)) { + return false; + } + if (VALID_ATTRIBUTE_NAME_REGEX.test(attributeName)) { + validatedAttributeNameCache[attributeName] = true; + return true; + } + illegalAttributeNameCache[attributeName] = true; + if (__DEV__) { + console.error('Invalid attribute name: `%s`', attributeName); + } + return false; +} From de637ed6f037b9fed921fb5a53484db0c240982e Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 29 Mar 2023 16:08:02 -0400 Subject: [PATCH 3/7] Move isUnitlessNumber to a separate module function --- .../src/client/CSSPropertyOperations.js | 12 +++--------- .../src/server/ReactDOMServerFormatConfig.js | 7 ++----- .../shared/{CSSProperty.js => isUnitlessNumber.js} | 4 ++++ 3 files changed, 9 insertions(+), 14 deletions(-) rename packages/react-dom-bindings/src/shared/{CSSProperty.js => isUnitlessNumber.js} (96%) diff --git a/packages/react-dom-bindings/src/client/CSSPropertyOperations.js b/packages/react-dom-bindings/src/client/CSSPropertyOperations.js index c2abd6f4f5674..3f101ae8282c6 100644 --- a/packages/react-dom-bindings/src/client/CSSPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/CSSPropertyOperations.js @@ -9,7 +9,7 @@ import {shorthandToLonghand} from './CSSShorthandProperty'; import hyphenateStyleName from '../shared/hyphenateStyleName'; import warnValidStyle from '../shared/warnValidStyle'; -import {isUnitlessNumber} from '../shared/CSSProperty'; +import isUnitlessNumber from '../shared/isUnitlessNumber'; import {checkCSSPropertyStringCoercion} from 'shared/CheckStringCoercion'; /** @@ -42,10 +42,7 @@ export function createDangerousStringForStyles(styles) { if ( typeof value === 'number' && value !== 0 && - !( - isUnitlessNumber.hasOwnProperty(styleName) && - isUnitlessNumber[styleName] - ) + !isUnitlessNumber(styleName) ) { serialized += delimiter + hyphenateStyleName(styleName) + ':' + value + 'px'; @@ -101,10 +98,7 @@ export function setValueForStyles(node, styles) { } else if ( typeof value === 'number' && value !== 0 && - !( - isUnitlessNumber.hasOwnProperty(styleName) && - isUnitlessNumber[styleName] - ) + !isUnitlessNumber(styleName) ) { style[styleName] = value + 'px'; // Presumes implicit 'px' suffix for unitless numbers } else { diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index a85a18a7c967b..a84a43048a69e 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -46,7 +46,7 @@ import { NUMERIC, POSITIVE_NUMERIC, } from '../shared/DOMProperty'; -import {isUnitlessNumber} from '../shared/CSSProperty'; +import isUnitlessNumber from '../shared/isUnitlessNumber'; import {checkControlledValueProps} from '../shared/ReactControlledValuePropTypes'; import {validateProperties as validateARIAProperties} from '../shared/ReactDOMInvalidARIAHook'; @@ -579,10 +579,7 @@ function pushStyleAttribute( nameChunk = processStyleName(styleName); if (typeof styleValue === 'number') { - if ( - styleValue !== 0 && - !hasOwnProperty.call(isUnitlessNumber, styleName) - ) { + if (styleValue !== 0 && !isUnitlessNumber(styleName)) { valueChunk = stringToChunk(styleValue + 'px'); // Presumes implicit 'px' suffix for unitless numbers } else { valueChunk = stringToChunk('' + styleValue); diff --git a/packages/react-dom-bindings/src/shared/CSSProperty.js b/packages/react-dom-bindings/src/shared/isUnitlessNumber.js similarity index 96% rename from packages/react-dom-bindings/src/shared/CSSProperty.js rename to packages/react-dom-bindings/src/shared/isUnitlessNumber.js index 5952aca573fa1..e6c80e0d382a8 100644 --- a/packages/react-dom-bindings/src/shared/CSSProperty.js +++ b/packages/react-dom-bindings/src/shared/isUnitlessNumber.js @@ -80,3 +80,7 @@ Object.keys(isUnitlessNumber).forEach(function (prop) { isUnitlessNumber[prefixKey(prefix, prop)] = isUnitlessNumber[prop]; }); }); + +export default function (prop) { + return isUnitlessNumber.hasOwnProperty(prop); +} From b7c3089c8090facb2eab445d9fb4019044020102 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 29 Mar 2023 17:06:22 -0400 Subject: [PATCH 4/7] Use a switch statement for isUnitlessNumber This changes behavior for prefixes. I checked for which prefixes are still returned by getComputedStyle in browsers. I also added the ones mentioned in the original PR, whether used or not. I removed Opera prefixes since they have since been removed in the browser itself. We should ideally just remove all prefixes. --- .../src/shared/isUnitlessNumber.js | 144 +++++++++--------- 1 file changed, 69 insertions(+), 75 deletions(-) diff --git a/packages/react-dom-bindings/src/shared/isUnitlessNumber.js b/packages/react-dom-bindings/src/shared/isUnitlessNumber.js index e6c80e0d382a8..de07a8785b61a 100644 --- a/packages/react-dom-bindings/src/shared/isUnitlessNumber.js +++ b/packages/react-dom-bindings/src/shared/isUnitlessNumber.js @@ -3,84 +3,78 @@ * * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. + * + * @flow */ /** * CSS properties which accept numbers but are not in units of "px". */ -export const isUnitlessNumber = { - animationIterationCount: true, - aspectRatio: true, - borderImageOutset: true, - borderImageSlice: true, - borderImageWidth: true, - boxFlex: true, - boxFlexGroup: true, - boxOrdinalGroup: true, - columnCount: true, - columns: true, - flex: true, - flexGrow: true, - flexPositive: true, - flexShrink: true, - flexNegative: true, - flexOrder: true, - gridArea: true, - gridRow: true, - gridRowEnd: true, - gridRowSpan: true, - gridRowStart: true, - gridColumn: true, - gridColumnEnd: true, - gridColumnSpan: true, - gridColumnStart: true, - fontWeight: true, - lineClamp: true, - lineHeight: true, - opacity: true, - order: true, - orphans: true, - scale: true, - tabSize: true, - widows: true, - zIndex: true, - zoom: true, - - // SVG-related properties - fillOpacity: true, - floodOpacity: true, - stopOpacity: true, - strokeDasharray: true, - strokeDashoffset: true, - strokeMiterlimit: true, - strokeOpacity: true, - strokeWidth: true, -}; - -/** - * @param {string} prefix vendor-specific prefix, eg: Webkit - * @param {string} key style name, eg: transitionDuration - * @return {string} style name prefixed with `prefix`, properly camelCased, eg: - * WebkitTransitionDuration - */ -function prefixKey(prefix, key) { - return prefix + key.charAt(0).toUpperCase() + key.substring(1); -} - -/** - * Support style names that may come passed in prefixed by adding permutations - * of vendor prefixes. - */ -const prefixes = ['Webkit', 'ms', 'Moz', 'O']; - -// Using Object.keys here, or else the vanilla for-in loop makes IE8 go into an -// infinite loop, because it iterates over the newly added props too. -Object.keys(isUnitlessNumber).forEach(function (prop) { - prefixes.forEach(function (prefix) { - isUnitlessNumber[prefixKey(prefix, prop)] = isUnitlessNumber[prop]; - }); -}); - -export default function (prop) { - return isUnitlessNumber.hasOwnProperty(prop); +export default function (name: string): boolean { + switch (name) { + case 'animationIterationCount': + case 'aspectRatio': + case 'borderImageOutset': + case 'borderImageSlice': + case 'borderImageWidth': + case 'boxFlex': + case 'boxFlexGroup': + case 'boxOrdinalGroup': + case 'columnCount': + case 'columns': + case 'flex': + case 'flexGrow': + case 'flexPositive': + case 'flexShrink': + case 'flexNegative': + case 'flexOrder': + case 'gridArea': + case 'gridRow': + case 'gridRowEnd': + case 'gridRowSpan': + case 'gridRowStart': + case 'gridColumn': + case 'gridColumnEnd': + case 'gridColumnSpan': + case 'gridColumnStart': + case 'fontWeight': + case 'lineClamp': + case 'lineHeight': + case 'opacity': + case 'order': + case 'orphans': + case 'scale': + case 'tabSize': + case 'widows': + case 'zIndex': + case 'zoom': + case 'fillOpacity': // SVG-related properties + case 'floodOpacity': + case 'stopOpacity': + case 'strokeDasharray': + case 'strokeDashoffset': + case 'strokeMiterlimit': + case 'strokeOpacity': + case 'strokeWidth': + case 'MozAnimationIterationCount': // Known Prefixed Properties + case 'MozBoxFlex': // TODO: Remove these since they shouldn't be used in modern code + case 'MozBoxFlexGroup': + case 'MozLineClamp': + case 'msFlexGrow': + case 'msLineClamp': + case 'WebkitAnimationIterationCount': + case 'WebkitBoxFlex': + case 'WebKitBoxFlexGroup': + case 'WebkitBoxOrdinalGroup': + case 'WebkitColumnCount': + case 'WebkitColumns': + case 'WebkitFlex': + case 'WebkitFlexGrow': + case 'WebkitFlexPositive': + case 'WebkitFlexShrink': + case 'WebkitLineClamp': + return true; + default: + return false; + } } From 7f99711b139c78dd46bc51033929a95bf9543a5b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 29 Mar 2023 18:20:38 -0400 Subject: [PATCH 5/7] Join type checks into one switch --- .../src/shared/ReactDOMUnknownPropertyHook.js | 121 +++++++++--------- 1 file changed, 60 insertions(+), 61 deletions(-) diff --git a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js index bd4312c57dd60..4bf1f8aebed7a 100644 --- a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js +++ b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js @@ -181,75 +181,74 @@ function validateProperty(tagName, name, value, eventRegistry) { } } - if (typeof value === 'boolean') { - const prefix = name.toLowerCase().slice(0, 5); - const acceptsBooleans = - propertyInfo !== null - ? propertyInfo.acceptsBooleans - : prefix === 'data-' || prefix === 'aria-'; - if (!acceptsBooleans) { - if (value) { - console.error( - 'Received `%s` for a non-boolean attribute `%s`.\n\n' + - 'If you want to write it to the DOM, pass a string instead: ' + - '%s="%s" or %s={value.toString()}.', - value, - name, - name, - value, - name, - ); - } else { + switch (typeof value) { + case 'boolean': { + const prefix = name.toLowerCase().slice(0, 5); + const acceptsBooleans = + propertyInfo !== null + ? propertyInfo.acceptsBooleans + : prefix === 'data-' || prefix === 'aria-'; + if (!acceptsBooleans) { + if (value) { + console.error( + 'Received `%s` for a non-boolean attribute `%s`.\n\n' + + 'If you want to write it to the DOM, pass a string instead: ' + + '%s="%s" or %s={value.toString()}.', + value, + name, + name, + value, + name, + ); + } else { + console.error( + 'Received `%s` for a non-boolean attribute `%s`.\n\n' + + 'If you want to write it to the DOM, pass a string instead: ' + + '%s="%s" or %s={value.toString()}.\n\n' + + 'If you used to conditionally omit it with %s={condition && value}, ' + + 'pass %s={condition ? value : undefined} instead.', + value, + name, + name, + value, + name, + name, + name, + ); + } + warnedProperties[name] = true; + return true; + } + return true; + } + case 'function': + case 'symbol': // eslint-disable-line + // Warn when a known attribute is a bad type + warnedProperties[name] = true; + return false; + case 'string': + // Warn when passing the strings 'false' or 'true' into a boolean prop + if ( + (value === 'false' || value === 'true') && + propertyInfo !== null && + propertyInfo.type === BOOLEAN + ) { console.error( - 'Received `%s` for a non-boolean attribute `%s`.\n\n' + - 'If you want to write it to the DOM, pass a string instead: ' + - '%s="%s" or %s={value.toString()}.\n\n' + - 'If you used to conditionally omit it with %s={condition && value}, ' + - 'pass %s={condition ? value : undefined} instead.', + 'Received the string `%s` for the boolean attribute `%s`. ' + + '%s ' + + 'Did you mean %s={%s}?', value, name, + value === 'false' + ? 'The browser will interpret it as a truthy value.' + : 'Although this works, it will not work as expected if you pass the string "false".', name, value, - name, - name, - name, ); + warnedProperties[name] = true; + return true; } - } - warnedProperties[name] = true; - return true; - } - - // Warn when a known attribute is a bad type - switch (typeof value) { - case 'function': - case 'symbol': // eslint-disable-line - warnedProperties[name] = true; - return false; - } - - // Warn when passing the strings 'false' or 'true' into a boolean prop - if ( - (value === 'false' || value === 'true') && - propertyInfo !== null && - propertyInfo.type === BOOLEAN - ) { - console.error( - 'Received the string `%s` for the boolean attribute `%s`. ' + - '%s ' + - 'Did you mean %s={%s}?', - value, - name, - value === 'false' - ? 'The browser will interpret it as a truthy value.' - : 'Although this works, it will not work as expected if you pass the string "false".', - name, - value, - ); - warnedProperties[name] = true; - return true; } - return true; } } From 470ddc86c6e3d7fa2d6eab217c2211c8f0b3603d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 29 Mar 2023 17:53:38 -0400 Subject: [PATCH 6/7] Inline mustUseProperty which is only used for special controlled components --- .../src/client/DOMPropertyOperations.js | 15 --- .../src/client/ReactDOMComponent.js | 63 ++++++++++- .../src/server/ReactDOMServerFormatConfig.js | 18 ++- .../src/shared/DOMProperty.js | 60 ---------- .../src/shared/ReactDOMUnknownPropertyHook.js | 103 +++++++++++------- .../src/__tests__/ReactDOMComponent-test.js | 40 +++++-- 6 files changed, 172 insertions(+), 127 deletions(-) diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js index fae87f3d1c0f9..f0649b97522b4 100644 --- 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 index bf527f3d3659a..93686a54b8b7e 100644 --- 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 index a84a43048a69e..4ede9b9ddba4d 100644 --- 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, + 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, 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 index 52d197fac5575..8bd96110f3a50 100644 --- 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,9 +158,7 @@ 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 @@ -180,32 +166,6 @@ const properties: {[string]: $FlowFixMe} = {}; ); }); -// 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 diff --git a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js index 4bf1f8aebed7a..ca469bc792690 100644 --- a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js +++ b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js @@ -183,56 +183,76 @@ function validateProperty(tagName, name, value, eventRegistry) { switch (typeof value) { case 'boolean': { - const prefix = name.toLowerCase().slice(0, 5); - const acceptsBooleans = - propertyInfo !== null - ? propertyInfo.acceptsBooleans - : prefix === 'data-' || prefix === 'aria-'; - if (!acceptsBooleans) { - if (value) { - console.error( - 'Received `%s` for a non-boolean attribute `%s`.\n\n' + - 'If you want to write it to the DOM, pass a string instead: ' + - '%s="%s" or %s={value.toString()}.', - value, - name, - name, - value, - name, - ); - } else { - console.error( - 'Received `%s` for a non-boolean attribute `%s`.\n\n' + - 'If you want to write it to the DOM, pass a string instead: ' + - '%s="%s" or %s={value.toString()}.\n\n' + - 'If you used to conditionally omit it with %s={condition && value}, ' + - 'pass %s={condition ? value : undefined} instead.', - value, - name, - name, - value, - name, - name, - name, - ); + switch (name) { + case 'checked': + case 'selected': + case 'multiple': + case 'muted': { + // Boolean properties can accept boolean values + return true; + } + default: { + if (propertyInfo === null) { + const prefix = name.toLowerCase().slice(0, 5); + if (prefix === 'data-' || prefix === 'aria-') { + return true; + } + } else if (propertyInfo.acceptsBooleans) { + return true; + } + if (value) { + console.error( + 'Received `%s` for a non-boolean attribute `%s`.\n\n' + + 'If you want to write it to the DOM, pass a string instead: ' + + '%s="%s" or %s={value.toString()}.', + value, + name, + name, + value, + name, + ); + } else { + console.error( + 'Received `%s` for a non-boolean attribute `%s`.\n\n' + + 'If you want to write it to the DOM, pass a string instead: ' + + '%s="%s" or %s={value.toString()}.\n\n' + + 'If you used to conditionally omit it with %s={condition && value}, ' + + 'pass %s={condition ? value : undefined} instead.', + value, + name, + name, + value, + name, + name, + name, + ); + } + warnedProperties[name] = true; + return true; } - warnedProperties[name] = true; - return true; } - return true; } case 'function': case 'symbol': // eslint-disable-line // Warn when a known attribute is a bad type warnedProperties[name] = true; return false; - case 'string': + case 'string': { // Warn when passing the strings 'false' or 'true' into a boolean prop - if ( - (value === 'false' || value === 'true') && - propertyInfo !== null && - propertyInfo.type === BOOLEAN - ) { + if (value === 'false' || value === 'true') { + switch (name) { + case 'checked': + case 'selected': + case 'multiple': + case 'muted': { + break; + } + default: { + if (propertyInfo === null || propertyInfo.type !== BOOLEAN) { + return true; + } + } + } console.error( 'Received the string `%s` for the boolean attribute `%s`. ' + '%s ' + @@ -248,6 +268,7 @@ function validateProperty(tagName, name, value, eventRegistry) { warnedProperties[name] = true; return true; } + } } return true; } diff --git a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js index 7343ff3bbad7b..f373af9335ea1 100644 --- a/packages/react-dom/src/__tests__/ReactDOMComponent-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMComponent-test.js @@ -1045,7 +1045,13 @@ describe('ReactDOMComponent', () => { it('should not incur unnecessary DOM mutations for boolean properties', () => { const container = document.createElement('div'); - ReactDOM.render(
, container); + function onChange() { + // noop + } + ReactDOM.render( + , + container, + ); const node = container.firstChild; let nodeValue = true; @@ -1059,17 +1065,37 @@ describe('ReactDOMComponent', () => { }), }); - ReactDOM.render(
, container); + ReactDOM.render( + , + container, + ); expect(nodeValueSetter).toHaveBeenCalledTimes(0); - ReactDOM.render(
, container); + expect(() => { + ReactDOM.render( + , + container, + ); + }).toErrorDev( + 'A component is changing a controlled input to be uncontrolled. This is likely caused by ' + + 'the value changing from a defined to undefined, which should not happen. Decide between ' + + 'using a controlled or uncontrolled input element for the lifetime of the component.', + ); expect(nodeValueSetter).toHaveBeenCalledTimes(1); - ReactDOM.render(
, container); - expect(nodeValueSetter).toHaveBeenCalledTimes(2); - - ReactDOM.render(
, container); + ReactDOM.render( + , + container, + ); + // TODO: Non-null values are updated twice on inputs. This is should ideally be fixed. expect(nodeValueSetter).toHaveBeenCalledTimes(3); + + ReactDOM.render( + , + container, + ); + // TODO: Non-null values are updated twice on inputs. This is should ideally be fixed. + expect(nodeValueSetter).toHaveBeenCalledTimes(5); }); it('should ignore attribute list for elements with the "is" attribute', () => { From 6b2bc316aa047cd16accad9f95806c5af4c23c89 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 30 Mar 2023 14:06:36 -0400 Subject: [PATCH 7/7] Expand ms prefix set --- .../src/shared/isUnitlessNumber.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/packages/react-dom-bindings/src/shared/isUnitlessNumber.js b/packages/react-dom-bindings/src/shared/isUnitlessNumber.js index de07a8785b61a..334111fa7a385 100644 --- a/packages/react-dom-bindings/src/shared/isUnitlessNumber.js +++ b/packages/react-dom-bindings/src/shared/isUnitlessNumber.js @@ -60,8 +60,18 @@ export default function (name: string): boolean { case 'MozBoxFlex': // TODO: Remove these since they shouldn't be used in modern code case 'MozBoxFlexGroup': case 'MozLineClamp': + case 'msAnimationIterationCount': + case 'msFlex': + case 'msZoom': case 'msFlexGrow': - case 'msLineClamp': + case 'msFlexNegative': + case 'msFlexOrder': + case 'msFlexPositive': + case 'msFlexShrink': + case 'msGridColumn': + case 'msGridColumnSpan': + case 'msGridRow': + case 'msGridRowSpan': case 'WebkitAnimationIterationCount': case 'WebkitBoxFlex': case 'WebKitBoxFlexGroup':