@samitouri / QOS-React-2 / commits / 73deff0d51

Refactor DOMProperty and CSSProperty (#26513)

This is a step towards getting rid of the meta programming in DOMProperty and CSSProperty. This moves isAttributeNameSafe and isUnitlessNumber to a separate shared modules. isUnitlessNumber is now a single switch instead of meta-programming. There is a slight behavior change here in that I hard code a specific set of vendor-prefixed attributes instead of prefixing all the unitless properties. I based this list on what getComputedStyle returns in current browsers. I removed Opera prefixes because they were [removed in Opera](https://dev.opera.com/blog/css-vendor-prefixes-in-opera-12-50-snapshots/) itself. I included the ms ones mentioned [in the original PR](https://github.com/facebook/react/commit/5abcce534382d85887f3d33475e8e54e3b5d8457). These shouldn't really be used anymore anyway so should be pretty safe. Worst case, they'll fallback to the other property if you specify both. Finally I inline the mustUseProperty special cases - which are also the only thing that uses propertyName. These are really all controlled components and all booleans. I'm making a small breaking change here by treating `checked` and `selected` specially only on the `input` and `option` tags instead of all tags. That's because those are the only DOM nodes that actually have those properties but we used to set them as expandos instead of attributes before. That's why one of the tests is updated to now use `input` instead of testing an expando on a `div` which isn't a real use case. Interestingly this also uncovered that we update checked twice for some reason but keeping that logic for now. Ideally `multiple` and `muted` should move into `select` and `audio`/`video` respectively for the same reason. No change to the attribute-behavior fixture.

Sebastian Markbåge committed Mar 30, 2023 at 14:30 UTC 73deff0d5162160c0aafa5cd0b87e11143fe9938
11 files changed +456 -393
packages/react-dom-bindings/src/client/CSSPropertyOperations.js
+3 -9
@@ -9,7 +9,7 @@ import {shorthandToLonghand} from './CSSShorthandProperty';
9
10 import hyphenateStyleName from '../shared/hyphenateStyleName';
11 import warnValidStyle from '../shared/warnValidStyle';
12 -import {isUnitlessNumber} from '../shared/CSSProperty';
12 +import isUnitlessNumber from '../shared/isUnitlessNumber';
13 import {checkCSSPropertyStringCoercion} from 'shared/CheckStringCoercion';
14
15 /**
@@ -42,10 +42,7 @@ export function createDangerousStringForStyles(styles) {
42 if (
43 typeof value === 'number' &&
44 value !== 0 &&
45 - !(
46 - isUnitlessNumber.hasOwnProperty(styleName) &&
47 - isUnitlessNumber[styleName]
48 - )
45 + !isUnitlessNumber(styleName)
46 ) {
47 serialized +=
48 delimiter + hyphenateStyleName(styleName) + ':' + value + 'px';
@@ -101,10 +98,7 @@ export function setValueForStyles(node, styles) {
98 } else if (
99 typeof value === 'number' &&
100 value !== 0 &&
104 - !(
105 - isUnitlessNumber.hasOwnProperty(styleName) &&
106 - isUnitlessNumber[styleName]
107 - )
101 + !isUnitlessNumber(styleName)
102 ) {
103 style[styleName] = value + 'px'; // Presumes implicit 'px' suffix for unitless numbers
104 } else {
packages/react-dom-bindings/src/client/DOMPropertyOperations.js
+108 -128
@@ -8,13 +8,13 @@
8 */
9
10 import {
11 - getPropertyInfo,
12 - isAttributeNameSafe,
11 BOOLEAN,
12 OVERLOADED_BOOLEAN,
13 NUMERIC,
14 POSITIVE_NUMERIC,
15 } from '../shared/DOMProperty';
16 +
17 +import isAttributeNameSafe from '../shared/isAttributeNameSafe';
18 import sanitizeURL from '../shared/sanitizeURL';
19 import {
20 enableTrustedTypesIntegration,
@@ -38,11 +38,6 @@ export function getValueForProperty(
38 propertyInfo: PropertyInfo,
39 ): mixed {
40 if (__DEV__) {
41 - if (propertyInfo.mustUseProperty) {
42 - const {propertyName} = propertyInfo;
43 - return (node: any)[propertyName];
44 - }
45 -
41 const attributeName = propertyInfo.attributeName;
42
43 if (!node.hasAttribute(attributeName)) {
@@ -287,152 +282,137 @@ export function getValueForAttributeOnCustomComponent(
282 * @param {string} name
283 * @param {*} value
284 */
290 -export function setValueForProperty(node: Element, name: string, value: mixed) {
291 - if (
292 - // shouldIgnoreAttribute
293 - // We have already filtered out reserved words.
294 - name.length > 2 &&
295 - (name[0] === 'o' || name[0] === 'O') &&
296 - (name[1] === 'n' || name[1] === 'N')
297 - ) {
285 +export function setValueForProperty(
286 + node: Element,
287 + propertyInfo: PropertyInfo,
288 + value: mixed,
289 +) {
290 + const attributeName = propertyInfo.attributeName;
291 +
292 + if (value === null) {
293 + node.removeAttribute(attributeName);
294 return;
295 }
296
301 - const propertyInfo = getPropertyInfo(name);
302 - if (propertyInfo !== null) {
303 - if (propertyInfo.mustUseProperty) {
304 - // We assume mustUseProperty are of BOOLEAN type because that's the only way we use it
305 - // right now.
306 - (node: any)[propertyInfo.propertyName] =
307 - value && typeof value !== 'function' && typeof value !== 'symbol';
297 + // shouldRemoveAttribute
298 + switch (typeof value) {
299 + case 'undefined':
300 + case 'function':
301 + case 'symbol': // eslint-disable-line
302 + node.removeAttribute(attributeName);
303 return;
304 + case 'boolean': {
305 + if (!propertyInfo.acceptsBooleans) {
306 + node.removeAttribute(attributeName);
307 + return;
308 + }
309 }
310 -
311 - // The rest are treated as attributes with special cases.
312 -
313 - const attributeName = propertyInfo.attributeName;
314 -
315 - if (value === null) {
310 + }
311 + if (enableFilterEmptyStringAttributesDOM) {
312 + if (propertyInfo.removeEmptyString && value === '') {
313 + if (__DEV__) {
314 + if (attributeName === 'src') {
315 + console.error(
316 + 'An empty string ("") was passed to the %s attribute. ' +
317 + 'This may cause the browser to download the whole page again over the network. ' +
318 + 'To fix this, either do not render the element at all ' +
319 + 'or pass null to %s instead of an empty string.',
320 + attributeName,
321 + attributeName,
322 + );
323 + } else {
324 + console.error(
325 + 'An empty string ("") was passed to the %s attribute. ' +
326 + 'To fix this, either do not render the element at all ' +
327 + 'or pass null to %s instead of an empty string.',
328 + attributeName,
329 + attributeName,
330 + );
331 + }
332 + }
333 node.removeAttribute(attributeName);
334 return;
335 }
336 + }
337
320 - // shouldRemoveAttribute
321 - switch (typeof value) {
322 - case 'undefined':
323 - case 'function':
324 - case 'symbol': // eslint-disable-line
338 + switch (propertyInfo.type) {
339 + case BOOLEAN:
340 + if (value) {
341 + node.setAttribute(attributeName, '');
342 + } else {
343 node.removeAttribute(attributeName);
344 return;
327 - case 'boolean': {
328 - if (!propertyInfo.acceptsBooleans) {
329 - node.removeAttribute(attributeName);
330 - return;
345 + }
346 + break;
347 + case OVERLOADED_BOOLEAN:
348 + if (value === true) {
349 + node.setAttribute(attributeName, '');
350 + } else if (value === false) {
351 + node.removeAttribute(attributeName);
352 + } else {
353 + if (__DEV__) {
354 + checkAttributeStringCoercion(value, attributeName);
355 }
356 + node.setAttribute(attributeName, (value: any));
357 }
333 - }
334 - if (enableFilterEmptyStringAttributesDOM) {
335 - if (propertyInfo.removeEmptyString && value === '') {
358 + return;
359 + case NUMERIC:
360 + if (!isNaN(value)) {
361 if (__DEV__) {
337 - if (name === 'src') {
338 - console.error(
339 - 'An empty string ("") was passed to the %s attribute. ' +
340 - 'This may cause the browser to download the whole page again over the network. ' +
341 - 'To fix this, either do not render the element at all ' +
342 - 'or pass null to %s instead of an empty string.',
343 - name,
344 - name,
345 - );
346 - } else {
347 - console.error(
348 - 'An empty string ("") was passed to the %s attribute. ' +
349 - 'To fix this, either do not render the element at all ' +
350 - 'or pass null to %s instead of an empty string.',
351 - name,
352 - name,
353 - );
354 - }
362 + checkAttributeStringCoercion(value, attributeName);
363 }
364 + node.setAttribute(attributeName, (value: any));
365 + } else {
366 node.removeAttribute(attributeName);
357 - return;
367 }
359 - }
360 -
361 - switch (propertyInfo.type) {
362 - case BOOLEAN:
363 - if (value) {
364 - node.setAttribute(attributeName, '');
365 - } else {
366 - node.removeAttribute(attributeName);
367 - return;
368 - }
369 - break;
370 - case OVERLOADED_BOOLEAN:
371 - if (value === true) {
372 - node.setAttribute(attributeName, '');
373 - } else if (value === false) {
374 - node.removeAttribute(attributeName);
375 - } else {
376 - if (__DEV__) {
377 - checkAttributeStringCoercion(value, attributeName);
378 - }
379 - node.setAttribute(attributeName, (value: any));
380 - }
381 - return;
382 - case NUMERIC:
383 - if (!isNaN(value)) {
384 - if (__DEV__) {
385 - checkAttributeStringCoercion(value, attributeName);
386 - }
387 - node.setAttribute(attributeName, (value: any));
388 - } else {
389 - node.removeAttribute(attributeName);
390 - }
391 - break;
392 - case POSITIVE_NUMERIC:
393 - if (!isNaN(value) && (value: any) >= 1) {
394 - if (__DEV__) {
395 - checkAttributeStringCoercion(value, attributeName);
396 - }
397 - node.setAttribute(attributeName, (value: any));
398 - } else {
399 - node.removeAttribute(attributeName);
400 - }
401 - break;
402 - default: {
368 + break;
369 + case POSITIVE_NUMERIC:
370 + if (!isNaN(value) && (value: any) >= 1) {
371 if (__DEV__) {
372 checkAttributeStringCoercion(value, attributeName);
373 }
406 - let attributeValue;
407 - // `setAttribute` with objects becomes only `[object]` in IE8/9,
408 - // ('' + value) makes it output the correct toString()-value.
409 - if (enableTrustedTypesIntegration) {
410 - if (propertyInfo.sanitizeURL) {
411 - attributeValue = (sanitizeURL(value): any);
412 - } else {
413 - attributeValue = (value: any);
414 - }
374 + node.setAttribute(attributeName, (value: any));
375 + } else {
376 + node.removeAttribute(attributeName);
377 + }
378 + break;
379 + default: {
380 + if (__DEV__) {
381 + checkAttributeStringCoercion(value, attributeName);
382 + }
383 + let attributeValue;
384 + // `setAttribute` with objects becomes only `[object]` in IE8/9,
385 + // ('' + value) makes it output the correct toString()-value.
386 + if (enableTrustedTypesIntegration) {
387 + if (propertyInfo.sanitizeURL) {
388 + attributeValue = (sanitizeURL(value): any);
389 } else {
416 - // We have already verified this above.
417 - // eslint-disable-next-line react-internal/safe-string-coercion
418 - attributeValue = '' + (value: any);
419 - if (propertyInfo.sanitizeURL) {
420 - attributeValue = sanitizeURL(attributeValue);
421 - }
390 + attributeValue = (value: any);
391 }
423 - const attributeNamespace = propertyInfo.attributeNamespace;
424 - if (attributeNamespace) {
425 - node.setAttributeNS(
426 - attributeNamespace,
427 - attributeName,
428 - attributeValue,
429 - );
430 - } else {
431 - node.setAttribute(attributeName, attributeValue);
392 + } else {
393 + // We have already verified this above.
394 + // eslint-disable-next-line react-internal/safe-string-coercion
395 + attributeValue = '' + (value: any);
396 + if (propertyInfo.sanitizeURL) {
397 + attributeValue = sanitizeURL(attributeValue);
398 }
399 }
400 + const attributeNamespace = propertyInfo.attributeNamespace;
401 + if (attributeNamespace) {
402 + node.setAttributeNS(attributeNamespace, attributeName, attributeValue);
403 + } else {
404 + node.setAttribute(attributeName, attributeValue);
405 + }
406 }
435 - } else if (isAttributeNameSafe(name)) {
407 + }
408 +}
409 +
410 +export function setValueForAttribute(
411 + node: Element,
412 + name: string,
413 + value: mixed,
414 +) {
415 + if (isAttributeNameSafe(name)) {
416 // If the prop isn't in the special list, treat it as a simple attribute.
417 // shouldRemoveAttribute
418 if (value === null) {
packages/react-dom-bindings/src/client/ReactDOMComponent.js
+78 -3
@@ -24,6 +24,7 @@ import {
24 getValueForProperty,
25 setValueForProperty,
26 setValueForPropertyOnCustomComponent,
27 + setValueForAttribute,
28 } from './DOMPropertyOperations';
29 import {
30 initWrapperState as ReactDOMInputInitWrapperState,
@@ -378,6 +379,18 @@ function setProp(
379 }
380 break;
381 }
382 + // Note: `option.selected` is not updated if `select.multiple` is
383 + // disabled with `removeAttribute`. We have special logic for handling this.
384 + case 'multiple': {
385 + (domElement: any).multiple =
386 + value && typeof value !== 'function' && typeof value !== 'symbol';
387 + break;
388 + }
389 + case 'muted': {
390 + (domElement: any).muted =
391 + value && typeof value !== 'function' && typeof value !== 'symbol';
392 + break;
393 + }
394 case 'suppressContentEditableWarning':
395 case 'suppressHydrationWarning':
396 case 'defaultValue': // Reserved
@@ -408,7 +421,22 @@ function setProp(
421 if (isCustomComponentTag) {
422 setValueForPropertyOnCustomComponent(domElement, key, value);
423 } else {
411 - setValueForProperty(domElement, key, value);
424 + if (
425 + // shouldIgnoreAttribute
426 + // We have already filtered out reserved words.
427 + key.length > 2 &&
428 + (key[0] === 'o' || key[0] === 'O') &&
429 + (key[1] === 'n' || key[1] === 'N')
430 + ) {
431 + return;
432 + }
433 +
434 + const propertyInfo = getPropertyInfo(key);
435 + if (propertyInfo !== null) {
436 + setValueForProperty(domElement, propertyInfo, value);
437 + } else {
438 + setValueForAttribute(domElement, key, value);
439 + }
440 }
441 }
442 }
@@ -687,7 +715,19 @@ export function setInitialProperties(
715 if (propValue == null) {
716 continue;
717 }
690 - setProp(domElement, tag, propKey, propValue, false, props);
718 + switch (propKey) {
719 + case 'selected': {
720 + // TODO: Remove support for selected on option.
721 + (domElement: any).selected =
722 + propValue &&
723 + typeof propValue !== 'function' &&
724 + typeof propValue !== 'symbol';
725 + break;
726 + }
727 + default: {
728 + setProp(domElement, tag, propKey, propValue, false, props);
729 + }
730 + }
731 }
732 ReactDOMOptionPostMountWrapper(domElement, props);
733 return;
@@ -1002,6 +1042,26 @@ export function updateProperties(
1042 ReactDOMTextareaUpdateWrapper(domElement, nextProps);
1043 return;
1044 }
1045 + case 'option': {
1046 + for (let i = 0; i < updatePayload.length; i += 2) {
1047 + const propKey = updatePayload[i];
1048 + const propValue = updatePayload[i + 1];
1049 + switch (propKey) {
1050 + case 'selected': {
1051 + // TODO: Remove support for selected on option.
1052 + (domElement: any).selected =
1053 + propValue &&
1054 + typeof propValue !== 'function' &&
1055 + typeof propValue !== 'symbol';
1056 + break;
1057 + }
1058 + default: {
1059 + setProp(domElement, tag, propKey, propValue, false, nextProps);
1060 + }
1061 + }
1062 + }
1063 + return;
1064 + }
1065 case 'img':
1066 case 'link':
1067 case 'area':
@@ -1233,7 +1293,22 @@ function diffHydratedGenericElement(
1293 extraAttributeNames.delete(propKey);
1294 diffHydratedStyles(domElement, nextProp);
1295 continue;
1236 - // eslint-disable-next-line no-fallthrough
1296 + case 'multiple': {
1297 + extraAttributeNames.delete(propKey);
1298 + const serverValue = (domElement: any).multiple;
1299 + if (nextProp !== serverValue) {
1300 + warnForPropDifference('multiple', serverValue, nextProp);
1301 + }
1302 + continue;
1303 + }
1304 + case 'muted': {
1305 + extraAttributeNames.delete(propKey);
1306 + const serverValue = (domElement: any).muted;
1307 + if (nextProp !== serverValue) {
1308 + warnForPropDifference('muted', serverValue, nextProp);
1309 + }
1310 + continue;
1311 + }
1312 default:
1313 if (
1314 // shouldIgnoreAttribute
packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js
+19 -8
@@ -38,15 +38,15 @@ import {
38 clonePrecomputedChunk,
39 } from 'react-server/src/ReactServerStreamConfig';
40
41 +import isAttributeNameSafe from '../shared/isAttributeNameSafe';
42 import {
43 getPropertyInfo,
43 - isAttributeNameSafe,
44 BOOLEAN,
45 OVERLOADED_BOOLEAN,
46 NUMERIC,
47 POSITIVE_NUMERIC,
48 } from '../shared/DOMProperty';
49 -import {isUnitlessNumber} from '../shared/CSSProperty';
49 +import isUnitlessNumber from '../shared/isUnitlessNumber';
50
51 import {checkControlledValueProps} from '../shared/ReactControlledValuePropTypes';
52 import {validateProperties as validateARIAProperties} from '../shared/ReactDOMInvalidARIAHook';
@@ -579,10 +579,7 @@ function pushStyleAttribute(
579
580 nameChunk = processStyleName(styleName);
581 if (typeof styleValue === 'number') {
582 - if (
583 - styleValue !== 0 &&
584 - !hasOwnProperty.call(isUnitlessNumber, styleName)
585 - ) {
582 + if (styleValue !== 0 && !isUnitlessNumber(styleName)) {
583 valueChunk = stringToChunk(styleValue + 'px'); // Presumes implicit 'px' suffix for unitless numbers
584 } else {
585 valueChunk = stringToChunk('' + styleValue);
@@ -614,6 +611,16 @@ const attributeAssign = stringToPrecomputedChunk('="');
611 const attributeEnd = stringToPrecomputedChunk('"');
612 const attributeEmptyString = stringToPrecomputedChunk('=""');
613
614 +function pushBooleanAttribute(
615 + target: Array<Chunk | PrecomputedChunk>,
616 + name: string,
617 + value: string | boolean | number | Function | Object, // not null or undefined
618 +): void {
619 + if (value && typeof value !== 'function' && typeof value !== 'symbol') {
620 + target.push(attributeSeparator, stringToChunk(name), attributeEmptyString);
621 + }
622 +}
623 +
624 function pushAttribute(
625 target: Array<Chunk | PrecomputedChunk>,
626 name: string,
@@ -631,6 +638,10 @@ function pushAttribute(
638 case 'suppressHydrationWarning':
639 // Ignored. These are built-in to React on the client.
640 return;
641 + case 'multiple':
642 + case 'muted':
643 + pushBooleanAttribute(target, name, value);
644 + return;
645 }
646 if (
647 // shouldIgnoreAttribute
@@ -1115,9 +1126,9 @@ function pushInput(
1126 }
1127
1128 if (checked !== null) {
1118 - pushAttribute(target, 'checked', checked);
1129 + pushBooleanAttribute(target, 'checked', checked);
1130 } else if (defaultChecked !== null) {
1120 - pushAttribute(target, 'checked', defaultChecked);
1131 + pushBooleanAttribute(target, 'checked', defaultChecked);
1132 }
1133 if (value !== null) {
1134 pushAttribute(target, 'value', value);
packages/react-dom-bindings/src/shared/CSSProperty.js deleted
-82
@@ -1,82 +0,0 @@
1 -/**
2 - * Copyright (c) Meta Platforms, Inc. and affiliates.
3 - *
4 - * This source code is licensed under the MIT license found in the
5 - * LICENSE file in the root directory of this source tree.
6 - */
7 -
8 -/**
9 - * CSS properties which accept numbers but are not in units of "px".
10 - */
11 -export const isUnitlessNumber = {
12 - animationIterationCount: true,
13 - aspectRatio: true,
14 - borderImageOutset: true,
15 - borderImageSlice: true,
16 - borderImageWidth: true,
17 - boxFlex: true,
18 - boxFlexGroup: true,
19 - boxOrdinalGroup: true,
20 - columnCount: true,
21 - columns: true,
22 - flex: true,
23 - flexGrow: true,
24 - flexPositive: true,
25 - flexShrink: true,
26 - flexNegative: true,
27 - flexOrder: true,
28 - gridArea: true,
29 - gridRow: true,
30 - gridRowEnd: true,
31 - gridRowSpan: true,
32 - gridRowStart: true,
33 - gridColumn: true,
34 - gridColumnEnd: true,
35 - gridColumnSpan: true,
36 - gridColumnStart: true,
37 - fontWeight: true,
38 - lineClamp: true,
39 - lineHeight: true,
40 - opacity: true,
41 - order: true,
42 - orphans: true,
43 - scale: true,
44 - tabSize: true,
45 - widows: true,
46 - zIndex: true,
47 - zoom: true,
48 -
49 - // SVG-related properties
50 - fillOpacity: true,
51 - floodOpacity: true,
52 - stopOpacity: true,
53 - strokeDasharray: true,
54 - strokeDashoffset: true,
55 - strokeMiterlimit: true,
56 - strokeOpacity: true,
57 - strokeWidth: true,
58 -};
59 -
60 -/**
61 - * @param {string} prefix vendor-specific prefix, eg: Webkit
62 - * @param {string} key style name, eg: transitionDuration
63 - * @return {string} style name prefixed with `prefix`, properly camelCased, eg:
64 - * WebkitTransitionDuration
65 - */
66 -function prefixKey(prefix, key) {
67 - return prefix + key.charAt(0).toUpperCase() + key.substring(1);
68 -}
69 -
70 -/**
71 - * Support style names that may come passed in prefixed by adding permutations
72 - * of vendor prefixes.
73 - */
74 -const prefixes = ['Webkit', 'ms', 'Moz', 'O'];
75 -
76 -// Using Object.keys here, or else the vanilla for-in loop makes IE8 go into an
77 -// infinite loop, because it iterates over the newly added props too.
78 -Object.keys(isUnitlessNumber).forEach(function (prop) {
79 - prefixes.forEach(function (prefix) {
80 - isUnitlessNumber[prefixKey(prefix, prop)] = isUnitlessNumber[prop];
81 - });
82 -});
packages/react-dom-bindings/src/shared/DOMProperty.js
-94
@@ -7,8 +7,6 @@
7 * @flow
8 */
9
10 -import hasOwnProperty from 'shared/hasOwnProperty';
11 -
10 type PropertyType = 0 | 1 | 2 | 3 | 4 | 5 | 6;
11
12 // A simple string attribute.
@@ -44,54 +42,18 @@ export type PropertyInfo = {
42 +acceptsBooleans: boolean,
43 +attributeName: string,
44 +attributeNamespace: string | null,
47 - +mustUseProperty: boolean,
48 - +propertyName: string,
45 +type: PropertyType,
46 +sanitizeURL: boolean,
47 +removeEmptyString: boolean,
48 };
49
54 -/* eslint-disable max-len */
55 -export const ATTRIBUTE_NAME_START_CHAR =
56 - ':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';
57 -/* eslint-enable max-len */
58 -export const ATTRIBUTE_NAME_CHAR: string =
59 - ATTRIBUTE_NAME_START_CHAR + '\\-.0-9\\u00B7\\u0300-\\u036F\\u203F-\\u2040';
60 -
61 -export const VALID_ATTRIBUTE_NAME_REGEX: RegExp = new RegExp(
62 - '^[' + ATTRIBUTE_NAME_START_CHAR + '][' + ATTRIBUTE_NAME_CHAR + ']*$',
63 -);
64 -
65 -const illegalAttributeNameCache: {[string]: boolean} = {};
66 -const validatedAttributeNameCache: {[string]: boolean} = {};
67 -
68 -export function isAttributeNameSafe(attributeName: string): boolean {
69 - if (hasOwnProperty.call(validatedAttributeNameCache, attributeName)) {
70 - return true;
71 - }
72 - if (hasOwnProperty.call(illegalAttributeNameCache, attributeName)) {
73 - return false;
74 - }
75 - if (VALID_ATTRIBUTE_NAME_REGEX.test(attributeName)) {
76 - validatedAttributeNameCache[attributeName] = true;
77 - return true;
78 - }
79 - illegalAttributeNameCache[attributeName] = true;
80 - if (__DEV__) {
81 - console.error('Invalid attribute name: `%s`', attributeName);
82 - }
83 - return false;
84 -}
85 -
50 export function getPropertyInfo(name: string): PropertyInfo | null {
51 return properties.hasOwnProperty(name) ? properties[name] : null;
52 }
53
54 // $FlowFixMe[missing-this-annot]
55 function PropertyInfoRecord(
92 - name: string,
56 type: PropertyType,
94 - mustUseProperty: boolean,
57 attributeName: string,
58 attributeNamespace: string | null,
59 sanitizeURL: boolean,
@@ -103,8 +65,6 @@ function PropertyInfoRecord(
65 type === OVERLOADED_BOOLEAN;
66 this.attributeName = attributeName;
67 this.attributeNamespace = attributeNamespace;
106 - this.mustUseProperty = mustUseProperty;
107 - this.propertyName = name;
68 this.type = type;
69 this.sanitizeURL = sanitizeURL;
70 this.removeEmptyString = removeEmptyString;
@@ -125,9 +85,7 @@ const properties: {[string]: $FlowFixMe} = {};
85 ].forEach(([name, attributeName]) => {
86 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
87 properties[name] = new PropertyInfoRecord(
128 - name,
88 STRING,
130 - false, // mustUseProperty
89 attributeName, // attributeName
90 null, // attributeNamespace
91 false, // sanitizeURL
@@ -141,9 +99,7 @@ const properties: {[string]: $FlowFixMe} = {};
99 ['contentEditable', 'draggable', 'spellCheck', 'value'].forEach(name => {
100 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
101 properties[name] = new PropertyInfoRecord(
144 - name,
102 BOOLEANISH_STRING,
146 - false, // mustUseProperty
103 name.toLowerCase(), // attributeName
104 null, // attributeNamespace
105 false, // sanitizeURL
@@ -163,9 +119,7 @@ const properties: {[string]: $FlowFixMe} = {};
119 ].forEach(name => {
120 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
121 properties[name] = new PropertyInfoRecord(
166 - name,
122 BOOLEANISH_STRING,
168 - false, // mustUseProperty
123 name, // attributeName
124 null, // attributeNamespace
125 false, // sanitizeURL
@@ -204,9 +158,7 @@ const properties: {[string]: $FlowFixMe} = {};
158 ].forEach(name => {
159 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
160 properties[name] = new PropertyInfoRecord(
207 - name,
161 BOOLEAN,
209 - false, // mustUseProperty
162 name.toLowerCase(), // attributeName
163 null, // attributeNamespace
164 false, // sanitizeURL
@@ -214,32 +166,6 @@ const properties: {[string]: $FlowFixMe} = {};
166 );
167 });
168
217 -// These are the few React props that we set as DOM properties
218 -// rather than attributes. These are all booleans.
219 -[
220 - 'checked',
221 - // Note: `option.selected` is not updated if `select.multiple` is
222 - // disabled with `removeAttribute`. We have special logic for handling this.
223 - 'multiple',
224 - 'muted',
225 - 'selected',
226 -
227 - // NOTE: if you add a camelCased prop to this list,
228 - // you'll need to set attributeName to name.toLowerCase()
229 - // instead in the assignment below.
230 -].forEach(name => {
231 - // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
232 - properties[name] = new PropertyInfoRecord(
233 - name,
234 - BOOLEAN,
235 - true, // mustUseProperty
236 - name, // attributeName
237 - null, // attributeNamespace
238 - false, // sanitizeURL
239 - false, // removeEmptyString
240 - );
241 -});
242 -
169 // These are HTML attributes that are "overloaded booleans": they behave like
170 // booleans, but can also accept a string value.
171 [
@@ -252,9 +178,7 @@ const properties: {[string]: $FlowFixMe} = {};
178 ].forEach(name => {
179 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
180 properties[name] = new PropertyInfoRecord(
255 - name,
181 OVERLOADED_BOOLEAN,
257 - false, // mustUseProperty
182 name, // attributeName
183 null, // attributeNamespace
184 false, // sanitizeURL
@@ -275,9 +199,7 @@ const properties: {[string]: $FlowFixMe} = {};
199 ].forEach(name => {
200 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
201 properties[name] = new PropertyInfoRecord(
278 - name,
202 POSITIVE_NUMERIC,
280 - false, // mustUseProperty
203 name, // attributeName
204 null, // attributeNamespace
205 false, // sanitizeURL
@@ -289,9 +211,7 @@ const properties: {[string]: $FlowFixMe} = {};
211 ['rowSpan', 'start'].forEach(name => {
212 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
213 properties[name] = new PropertyInfoRecord(
292 - name,
214 NUMERIC,
294 - false, // mustUseProperty
215 name.toLowerCase(), // attributeName
216 null, // attributeNamespace
217 false, // sanitizeURL
@@ -390,9 +310,7 @@ const capitalize = (token: string) => token[1].toUpperCase();
310 const name = attributeName.replace(CAMELIZE, capitalize);
311 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
312 properties[name] = new PropertyInfoRecord(
393 - name,
313 STRING,
395 - false, // mustUseProperty
314 attributeName,
315 null, // attributeNamespace
316 false, // sanitizeURL
@@ -416,9 +334,7 @@ const capitalize = (token: string) => token[1].toUpperCase();
334 const name = attributeName.replace(CAMELIZE, capitalize);
335 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
336 properties[name] = new PropertyInfoRecord(
419 - name,
337 STRING,
421 - false, // mustUseProperty
338 attributeName,
339 'http://www.w3.org/1999/xlink',
340 false, // sanitizeURL
@@ -439,9 +355,7 @@ const capitalize = (token: string) => token[1].toUpperCase();
355 const name = attributeName.replace(CAMELIZE, capitalize);
356 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
357 properties[name] = new PropertyInfoRecord(
442 - name,
358 STRING,
444 - false, // mustUseProperty
359 attributeName,
360 'http://www.w3.org/XML/1998/namespace',
361 false, // sanitizeURL
@@ -455,9 +369,7 @@ const capitalize = (token: string) => token[1].toUpperCase();
369 ['tabIndex', 'crossOrigin'].forEach(attributeName => {
370 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
371 properties[attributeName] = new PropertyInfoRecord(
458 - attributeName,
372 STRING,
460 - false, // mustUseProperty
373 attributeName.toLowerCase(), // attributeName
374 null, // attributeNamespace
375 false, // sanitizeURL
@@ -470,9 +382,7 @@ const capitalize = (token: string) => token[1].toUpperCase();
382 const xlinkHref = 'xlinkHref';
383 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
384 properties[xlinkHref] = new PropertyInfoRecord(
473 - 'xlinkHref',
385 STRING,
475 - false, // mustUseProperty
386 'xlink:href',
387 'http://www.w3.org/1999/xlink',
388 true, // sanitizeURL
@@ -482,9 +392,7 @@ properties[xlinkHref] = new PropertyInfoRecord(
392 const formAction = 'formAction';
393 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
394 properties[formAction] = new PropertyInfoRecord(
485 - 'formAction',
395 STRING,
487 - false, // mustUseProperty
396 'formaction', // attributeName
397 null, // attributeNamespace
398 true, // sanitizeURL
@@ -494,9 +402,7 @@ properties[formAction] = new PropertyInfoRecord(
402 ['src', 'href', 'action'].forEach(attributeName => {
403 // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions
404 properties[attributeName] = new PropertyInfoRecord(
497 - attributeName,
405 STRING,
499 - false, // mustUseProperty
406 attributeName.toLowerCase(), // attributeName
407 null, // attributeNamespace
408 true, // sanitizeURL
packages/react-dom-bindings/src/shared/ReactDOMInvalidARIAHook.js
+1 -1
@@ -5,7 +5,7 @@
5 * LICENSE file in the root directory of this source tree.
6 */
7
8 -import {ATTRIBUTE_NAME_CHAR} from './DOMProperty';
8 +import {ATTRIBUTE_NAME_CHAR} from './isAttributeNameSafe';
9 import isCustomComponent from './isCustomComponent';
10 import validAriaProperties from './validAriaProperties';
11 import hasOwnProperty from 'shared/hasOwnProperty';
packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js
+82 -61
@@ -5,7 +5,8 @@
5 * LICENSE file in the root directory of this source tree.
6 */
7
8 -import {ATTRIBUTE_NAME_CHAR, BOOLEAN, getPropertyInfo} from './DOMProperty';
8 +import {BOOLEAN, getPropertyInfo} from './DOMProperty';
9 +import {ATTRIBUTE_NAME_CHAR} from './isAttributeNameSafe';
10 import isCustomComponent from './isCustomComponent';
11 import possibleStandardNames from './possibleStandardNames';
12 import hasOwnProperty from 'shared/hasOwnProperty';
@@ -180,75 +181,95 @@ function validateProperty(tagName, name, value, eventRegistry) {
181 }
182 }
183
183 - if (typeof value === 'boolean') {
184 - const prefix = name.toLowerCase().slice(0, 5);
185 - const acceptsBooleans =
186 - propertyInfo !== null
187 - ? propertyInfo.acceptsBooleans
188 - : prefix === 'data-' || prefix === 'aria-';
189 - if (!acceptsBooleans) {
190 - if (value) {
191 - console.error(
192 - 'Received `%s` for a non-boolean attribute `%s`.\n\n' +
193 - 'If you want to write it to the DOM, pass a string instead: ' +
194 - '%s="%s" or %s={value.toString()}.',
195 - value,
196 - name,
197 - name,
198 - value,
199 - name,
200 - );
201 - } else {
184 + switch (typeof value) {
185 + case 'boolean': {
186 + switch (name) {
187 + case 'checked':
188 + case 'selected':
189 + case 'multiple':
190 + case 'muted': {
191 + // Boolean properties can accept boolean values
192 + return true;
193 + }
194 + default: {
195 + if (propertyInfo === null) {
196 + const prefix = name.toLowerCase().slice(0, 5);
197 + if (prefix === 'data-' || prefix === 'aria-') {
198 + return true;
199 + }
200 + } else if (propertyInfo.acceptsBooleans) {
201 + return true;
202 + }
203 + if (value) {
204 + console.error(
205 + 'Received `%s` for a non-boolean attribute `%s`.\n\n' +
206 + 'If you want to write it to the DOM, pass a string instead: ' +
207 + '%s="%s" or %s={value.toString()}.',
208 + value,
209 + name,
210 + name,
211 + value,
212 + name,
213 + );
214 + } else {
215 + console.error(
216 + 'Received `%s` for a non-boolean attribute `%s`.\n\n' +
217 + 'If you want to write it to the DOM, pass a string instead: ' +
218 + '%s="%s" or %s={value.toString()}.\n\n' +
219 + 'If you used to conditionally omit it with %s={condition && value}, ' +
220 + 'pass %s={condition ? value : undefined} instead.',
221 + value,
222 + name,
223 + name,
224 + value,
225 + name,
226 + name,
227 + name,
228 + );
229 + }
230 + warnedProperties[name] = true;
231 + return true;
232 + }
233 + }
234 + }
235 + case 'function':
236 + case 'symbol': // eslint-disable-line
237 + // Warn when a known attribute is a bad type
238 + warnedProperties[name] = true;
239 + return false;
240 + case 'string': {
241 + // Warn when passing the strings 'false' or 'true' into a boolean prop
242 + if (value === 'false' || value === 'true') {
243 + switch (name) {
244 + case 'checked':
245 + case 'selected':
246 + case 'multiple':
247 + case 'muted': {
248 + break;
249 + }
250 + default: {
251 + if (propertyInfo === null || propertyInfo.type !== BOOLEAN) {
252 + return true;
253 + }
254 + }
255 + }
256 console.error(
203 - 'Received `%s` for a non-boolean attribute `%s`.\n\n' +
204 - 'If you want to write it to the DOM, pass a string instead: ' +
205 - '%s="%s" or %s={value.toString()}.\n\n' +
206 - 'If you used to conditionally omit it with %s={condition && value}, ' +
207 - 'pass %s={condition ? value : undefined} instead.',
257 + 'Received the string `%s` for the boolean attribute `%s`. ' +
258 + '%s ' +
259 + 'Did you mean %s={%s}?',
260 value,
261 name,
262 + value === 'false'
263 + ? 'The browser will interpret it as a truthy value.'
264 + : 'Although this works, it will not work as expected if you pass the string "false".',
265 name,
266 value,
212 - name,
213 - name,
214 - name,
267 );
268 + warnedProperties[name] = true;
269 + return true;
270 }
271 }
218 - warnedProperties[name] = true;
219 - return true;
220 - }
221 -
222 - // Warn when a known attribute is a bad type
223 - switch (typeof value) {
224 - case 'function':
225 - case 'symbol': // eslint-disable-line
226 - warnedProperties[name] = true;
227 - return false;
272 }
229 -
230 - // Warn when passing the strings 'false' or 'true' into a boolean prop
231 - if (
232 - (value === 'false' || value === 'true') &&
233 - propertyInfo !== null &&
234 - propertyInfo.type === BOOLEAN
235 - ) {
236 - console.error(
237 - 'Received the string `%s` for the boolean attribute `%s`. ' +
238 - '%s ' +
239 - 'Did you mean %s={%s}?',
240 - value,
241 - name,
242 - value === 'false'
243 - ? 'The browser will interpret it as a truthy value.'
244 - : 'Although this works, it will not work as expected if you pass the string "false".',
245 - name,
246 - value,
247 - );
248 - warnedProperties[name] = true;
249 - return true;
250 - }
251 -
273 return true;
274 }
275 }
packages/react-dom-bindings/src/shared/isAttributeNameSafe.js new
+42
@@ -0,0 +1,42 @@
1 +/**
2 + * Copyright (c) Meta Platforms, Inc. and affiliates.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + *
7 + * @flow
8 + */
9 +
10 +import hasOwnProperty from 'shared/hasOwnProperty';
11 +
12 +/* eslint-disable max-len */
13 +const ATTRIBUTE_NAME_START_CHAR =
14 + ':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';
15 +/* eslint-enable max-len */
16 +export const ATTRIBUTE_NAME_CHAR: string =
17 + ATTRIBUTE_NAME_START_CHAR + '\\-.0-9\\u00B7\\u0300-\\u036F\\u203F-\\u2040';
18 +
19 +const VALID_ATTRIBUTE_NAME_REGEX: RegExp = new RegExp(
20 + '^[' + ATTRIBUTE_NAME_START_CHAR + '][' + ATTRIBUTE_NAME_CHAR + ']*$',
21 +);
22 +
23 +const illegalAttributeNameCache: {[string]: boolean} = {};
24 +const validatedAttributeNameCache: {[string]: boolean} = {};
25 +
26 +export default function isAttributeNameSafe(attributeName: string): boolean {
27 + if (hasOwnProperty.call(validatedAttributeNameCache, attributeName)) {
28 + return true;
29 + }
30 + if (hasOwnProperty.call(illegalAttributeNameCache, attributeName)) {
31 + return false;
32 + }
33 + if (VALID_ATTRIBUTE_NAME_REGEX.test(attributeName)) {
34 + validatedAttributeNameCache[attributeName] = true;
35 + return true;
36 + }
37 + illegalAttributeNameCache[attributeName] = true;
38 + if (__DEV__) {
39 + console.error('Invalid attribute name: `%s`', attributeName);
40 + }
41 + return false;
42 +}
packages/react-dom-bindings/src/shared/isUnitlessNumber.js new
+90
@@ -0,0 +1,90 @@
1 +/**
2 + * Copyright (c) Meta Platforms, Inc. and affiliates.
3 + *
4 + * This source code is licensed under the MIT license found in the
5 + * LICENSE file in the root directory of this source tree.
6 + *
7 + * @flow
8 + */
9 +
10 +/**
11 + * CSS properties which accept numbers but are not in units of "px".
12 + */
13 +export default function (name: string): boolean {
14 + switch (name) {
15 + case 'animationIterationCount':
16 + case 'aspectRatio':
17 + case 'borderImageOutset':
18 + case 'borderImageSlice':
19 + case 'borderImageWidth':
20 + case 'boxFlex':
21 + case 'boxFlexGroup':
22 + case 'boxOrdinalGroup':
23 + case 'columnCount':
24 + case 'columns':
25 + case 'flex':
26 + case 'flexGrow':
27 + case 'flexPositive':
28 + case 'flexShrink':
29 + case 'flexNegative':
30 + case 'flexOrder':
31 + case 'gridArea':
32 + case 'gridRow':
33 + case 'gridRowEnd':
34 + case 'gridRowSpan':
35 + case 'gridRowStart':
36 + case 'gridColumn':
37 + case 'gridColumnEnd':
38 + case 'gridColumnSpan':
39 + case 'gridColumnStart':
40 + case 'fontWeight':
41 + case 'lineClamp':
42 + case 'lineHeight':
43 + case 'opacity':
44 + case 'order':
45 + case 'orphans':
46 + case 'scale':
47 + case 'tabSize':
48 + case 'widows':
49 + case 'zIndex':
50 + case 'zoom':
51 + case 'fillOpacity': // SVG-related properties
52 + case 'floodOpacity':
53 + case 'stopOpacity':
54 + case 'strokeDasharray':
55 + case 'strokeDashoffset':
56 + case 'strokeMiterlimit':
57 + case 'strokeOpacity':
58 + case 'strokeWidth':
59 + case 'MozAnimationIterationCount': // Known Prefixed Properties
60 + case 'MozBoxFlex': // TODO: Remove these since they shouldn't be used in modern code
61 + case 'MozBoxFlexGroup':
62 + case 'MozLineClamp':
63 + case 'msAnimationIterationCount':
64 + case 'msFlex':
65 + case 'msZoom':
66 + case 'msFlexGrow':
67 + case 'msFlexNegative':
68 + case 'msFlexOrder':
69 + case 'msFlexPositive':
70 + case 'msFlexShrink':
71 + case 'msGridColumn':
72 + case 'msGridColumnSpan':
73 + case 'msGridRow':
74 + case 'msGridRowSpan':
75 + case 'WebkitAnimationIterationCount':
76 + case 'WebkitBoxFlex':
77 + case 'WebKitBoxFlexGroup':
78 + case 'WebkitBoxOrdinalGroup':
79 + case 'WebkitColumnCount':
80 + case 'WebkitColumns':
81 + case 'WebkitFlex':
82 + case 'WebkitFlexGrow':
83 + case 'WebkitFlexPositive':
84 + case 'WebkitFlexShrink':
85 + case 'WebkitLineClamp':
86 + return true;
87 + default:
88 + return false;
89 + }
90 +}
packages/react-dom/src/__tests__/ReactDOMComponent-test.js
+33 -7
@@ -1045,7 +1045,13 @@ describe('ReactDOMComponent', () => {
1045
1046 it('should not incur unnecessary DOM mutations for boolean properties', () => {
1047 const container = document.createElement('div');
1048 - ReactDOM.render(<div checked={true} />, container);
1048 + function onChange() {
1049 + // noop
1050 + }
1051 + ReactDOM.render(
1052 + <input type="checkbox" onChange={onChange} checked={true} />,
1053 + container,
1054 + );
1055
1056 const node = container.firstChild;
1057 let nodeValue = true;
@@ -1059,17 +1065,37 @@ describe('ReactDOMComponent', () => {
1065 }),
1066 });
1067
1062 - ReactDOM.render(<div checked={true} />, container);
1068 + ReactDOM.render(
1069 + <input type="checkbox" onChange={onChange} checked={true} />,
1070 + container,
1071 + );
1072 expect(nodeValueSetter).toHaveBeenCalledTimes(0);
1073
1065 - ReactDOM.render(<div />, container);
1074 + expect(() => {
1075 + ReactDOM.render(
1076 + <input type="checkbox" onChange={onChange} />,
1077 + container,
1078 + );
1079 + }).toErrorDev(
1080 + 'A component is changing a controlled input to be uncontrolled. This is likely caused by ' +
1081 + 'the value changing from a defined to undefined, which should not happen. Decide between ' +
1082 + 'using a controlled or uncontrolled input element for the lifetime of the component.',
1083 + );
1084 expect(nodeValueSetter).toHaveBeenCalledTimes(1);
1085
1068 - ReactDOM.render(<div checked={false} />, container);
1069 - expect(nodeValueSetter).toHaveBeenCalledTimes(2);
1070 -
1071 - ReactDOM.render(<div checked={true} />, container);
1086 + ReactDOM.render(
1087 + <input type="checkbox" onChange={onChange} checked={false} />,
1088 + container,
1089 + );
1090 + // TODO: Non-null values are updated twice on inputs. This is should ideally be fixed.
1091 expect(nodeValueSetter).toHaveBeenCalledTimes(3);
1092 +
1093 + ReactDOM.render(
1094 + <input type="checkbox" onChange={onChange} checked={true} />,
1095 + container,
1096 + );
1097 + // TODO: Non-null values are updated twice on inputs. This is should ideally be fixed.
1098 + expect(nodeValueSetter).toHaveBeenCalledTimes(5);
1099 });
1100
1101 it('should ignore attribute list for elements with the "is" attribute', () => {