From 916f2d9860b674056a1aa80710fd357498077f3d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 30 Mar 2023 23:16:59 -0400 Subject: [PATCH 01/16] Split setProp into separate one for Custom Elements This ensures that we can hard code special cases directly in the switch to get the correct semantics. --- .../src/client/CSSPropertyOperations.js | 15 ++ .../src/client/ReactDOMComponent.js | 214 ++++++++++++------ 2 files changed, 166 insertions(+), 63 deletions(-) diff --git a/packages/react-dom-bindings/src/client/CSSPropertyOperations.js b/packages/react-dom-bindings/src/client/CSSPropertyOperations.js index 3f101ae8282c6..9c07945aecff9 100644 --- a/packages/react-dom-bindings/src/client/CSSPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/CSSPropertyOperations.js @@ -72,6 +72,21 @@ export function createDangerousStringForStyles(styles) { * @param {object} styles */ export function setValueForStyles(node, styles) { + if (styles != null && typeof styles !== 'object') { + throw new Error( + 'The `style` prop expects a mapping from style properties to values, ' + + "not a string. For example, style={{marginRight: spacing + 'em'}} when " + + 'using JSX.', + ); + } + if (__DEV__) { + if (styles) { + // Freeze the next style object so that we can assume it won't be + // mutated. We have already warned for this in the past. + Object.freeze(styles); + } + } + const style = node.style; for (const styleName in styles) { if (!styles.hasOwnProperty(styleName)) { diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index e587d84707189..0bc1826bd11aa 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -264,25 +264,10 @@ function setProp( tag: string, key: string, value: mixed, - isCustomElementTag: boolean, props: any, ): void { switch (key) { case 'style': { - if (value != null && typeof value !== 'object') { - throw new Error( - 'The `style` prop expects a mapping from style properties to values, ' + - "not a string. For example, style={{marginRight: spacing + 'em'}} when " + - 'using JSX.', - ); - } - if (__DEV__) { - if (value) { - // Freeze the next style object so that we can assume it won't be - // mutated. We have already warned for this in the past. - Object.freeze(value); - } - } // Relies on `updateStylesByID` not mutating `styleUpdates`. setValueForStyles(domElement, value); break; @@ -385,40 +370,124 @@ function setProp( } // eslint-disable-next-line no-fallthrough default: { - if (registrationNameDependencies.hasOwnProperty(key)) { - if (__DEV__ && value != null && typeof value !== 'function') { + if ( + key.length > 2 && + (key[0] === 'o' || key[0] === 'O') && + (key[1] === 'n' || key[1] === 'N') + ) { + if ( + __DEV__ && + registrationNameDependencies.hasOwnProperty(key) && + value != null && + typeof value !== 'function' + ) { warnForInvalidEventListener(key, value); } } else { - if (isCustomElementTag) { - if (enableCustomElementPropertySupport) { - setValueForPropertyOnCustomComponent(domElement, key, value); - } else { - if (typeof value === 'boolean') { - // Special case before the new flag is on - value = '' + (value: any); - } - setValueForAttribute(domElement, key, value); - } + const propertyInfo = getPropertyInfo(key); + if (propertyInfo !== null) { + setValueForProperty(domElement, propertyInfo, value); } else { - 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; - } + setValueForAttribute(domElement, key, value); + } + } + } + } +} - const propertyInfo = getPropertyInfo(key); - if (propertyInfo !== null) { - setValueForProperty(domElement, propertyInfo, value); +function setPropOnCustomElement( + domElement: Element, + tag: string, + key: string, + value: mixed, + props: any, +): void { + switch (key) { + case 'style': { + // Relies on `updateStylesByID` not mutating `styleUpdates`. + setValueForStyles(domElement, value); + break; + } + case 'dangerouslySetInnerHTML': { + if (value != null) { + if (typeof value !== 'object' || !('__html' in value)) { + throw new Error( + '`props.dangerouslySetInnerHTML` must be in the form `{__html: ...}`. ' + + 'Please visit https://reactjs.org/link/dangerously-set-inner-html ' + + 'for more information.', + ); + } + const nextHtml: any = value.__html; + if (nextHtml != null) { + if (props.children != null) { + throw new Error( + 'Can only set one of `children` or `props.dangerouslySetInnerHTML`.', + ); + } + if (disableIEWorkarounds) { + domElement.innerHTML = nextHtml; } else { - setValueForAttribute(domElement, key, value); + setInnerHTML(domElement, nextHtml); } } } + break; + } + case 'children': { + if (typeof value === 'string') { + setTextContent(domElement, value); + } else if (typeof value === 'number') { + setTextContent(domElement, '' + value); + } + break; + } + case 'onScroll': { + if (value != null) { + if (__DEV__ && typeof value !== 'function') { + warnForInvalidEventListener(key, value); + } + listenToNonDelegatedEvent('scroll', domElement); + } + break; + } + case 'onClick': { + // TODO: This cast may not be sound for SVG, MathML or custom elements. + if (value != null) { + if (__DEV__ && typeof value !== 'function') { + warnForInvalidEventListener(key, value); + } + trapClickOnNonInteractiveElement(((domElement: any): HTMLElement)); + } + break; + } + case 'suppressContentEditableWarning': + case 'suppressHydrationWarning': + case 'innerHTML': { + // Noop + break; + } + case 'innerText': // Properties + case 'textContent': + if (enableCustomElementPropertySupport) { + break; + } + // eslint-disable-next-line no-fallthrough + default: { + if (registrationNameDependencies.hasOwnProperty(key)) { + if (__DEV__ && value != null && typeof value !== 'function') { + warnForInvalidEventListener(key, value); + } + } else { + if (enableCustomElementPropertySupport) { + setValueForPropertyOnCustomComponent(domElement, key, value); + } else { + if (typeof value === 'boolean') { + // Special case before the new flag is on + value = '' + (value: any); + } + setValueForAttribute(domElement, key, value); + } + } } } } @@ -475,7 +544,7 @@ export function setInitialProperties( } // defaultChecked and defaultValue are ignored by setProp default: { - setProp(domElement, tag, propKey, propValue, false, props); + setProp(domElement, tag, propKey, propValue, props); } } } @@ -505,7 +574,7 @@ export function setInitialProperties( } // defaultValue are ignored by setProp default: { - setProp(domElement, tag, propKey, propValue, false, props); + setProp(domElement, tag, propKey, propValue, props); } } } @@ -545,7 +614,7 @@ export function setInitialProperties( } // defaultValue is ignored by setProp default: { - setProp(domElement, tag, propKey, propValue, false, props); + setProp(domElement, tag, propKey, propValue, props); } } } @@ -575,7 +644,7 @@ export function setInitialProperties( break; } default: { - setProp(domElement, tag, propKey, propValue, false, props); + setProp(domElement, tag, propKey, propValue, props); } } } @@ -657,7 +726,7 @@ export function setInitialProperties( } // defaultChecked and defaultValue are ignored by setProp default: { - setProp(domElement, tag, propKey, propValue, false, props); + setProp(domElement, tag, propKey, propValue, props); } } } @@ -665,16 +734,28 @@ export function setInitialProperties( } } - const isCustomElementTag = isCustomElement(tag, props); - for (const propKey in props) { - if (!props.hasOwnProperty(propKey)) { - continue; + if (isCustomElement(tag, props)) { + for (const propKey in props) { + if (!props.hasOwnProperty(propKey)) { + continue; + } + const propValue = props[propKey]; + if (propValue == null) { + continue; + } + setPropOnCustomElement(domElement, tag, propKey, propValue, props); } - const propValue = props[propKey]; - if (propValue == null) { - continue; + } else { + for (const propKey in props) { + if (!props.hasOwnProperty(propKey)) { + continue; + } + const propValue = props[propKey]; + if (propValue == null) { + continue; + } + setProp(domElement, tag, propKey, propValue, props); } - setProp(domElement, tag, propKey, propValue, isCustomElementTag, props); } } @@ -837,7 +918,7 @@ export function updateProperties( } // defaultChecked and defaultValue are ignored by setProp default: { - setProp(domElement, tag, propKey, propValue, false, nextProps); + setProp(domElement, tag, propKey, propValue, nextProps); } } } @@ -858,7 +939,7 @@ export function updateProperties( } // defaultValue are ignored by setProp default: { - setProp(domElement, tag, propKey, propValue, false, nextProps); + setProp(domElement, tag, propKey, propValue, nextProps); } } } @@ -891,7 +972,7 @@ export function updateProperties( } // defaultValue is ignored by setProp default: { - setProp(domElement, tag, propKey, propValue, false, nextProps); + setProp(domElement, tag, propKey, propValue, nextProps); } } } @@ -912,7 +993,7 @@ export function updateProperties( break; } default: { - setProp(domElement, tag, propKey, propValue, false, nextProps); + setProp(domElement, tag, propKey, propValue, nextProps); } } } @@ -951,7 +1032,7 @@ export function updateProperties( } // defaultChecked and defaultValue are ignored by setProp default: { - setProp(domElement, tag, propKey, propValue, false, nextProps); + setProp(domElement, tag, propKey, propValue, nextProps); } } } @@ -959,12 +1040,19 @@ export function updateProperties( } } - const isCustomElementTag = isCustomElement(tag, nextProps); // Apply the diff. - for (let i = 0; i < updatePayload.length; i += 2) { - const propKey = updatePayload[i]; - const propValue = updatePayload[i + 1]; - setProp(domElement, tag, propKey, propValue, isCustomElementTag, nextProps); + if (isCustomElement(tag, nextProps)) { + for (let i = 0; i < updatePayload.length; i += 2) { + const propKey = updatePayload[i]; + const propValue = updatePayload[i + 1]; + setPropOnCustomElement(domElement, tag, propKey, propValue, nextProps); + } + } else { + for (let i = 0; i < updatePayload.length; i += 2) { + const propKey = updatePayload[i]; + const propValue = updatePayload[i + 1]; + setProp(domElement, tag, propKey, propValue, nextProps); + } } } From 02f96291438eb22dfe41fc6e17eead05e564bc19 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Thu, 30 Mar 2023 23:18:23 -0400 Subject: [PATCH 02/16] Make autoFocus fully a special case --- .../react-dom-bindings/src/client/ReactDOMComponent.js | 8 ++++++++ .../src/server/ReactDOMServerFormatConfig.js | 3 ++- packages/react-dom-bindings/src/shared/DOMProperty.js | 3 --- .../src/shared/ReactDOMUnknownPropertyHook.js | 5 +++-- 4 files changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 0bc1826bd11aa..4b1143163f8d4 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -1258,6 +1258,14 @@ function diffHydratedGenericElement( } continue; } + case 'autoFocus': { + extraAttributeNames.delete('autofocus'); + const serverValue = (domElement: any).autofocus; + if (nextProp !== serverValue) { + warnForPropDifference('autoFocus', 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 4168b40cb7afc..74a3e406ad52d 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -638,9 +638,10 @@ function pushAttribute( case 'suppressHydrationWarning': // Ignored. These are built-in to React on the client. return; + case 'autoFocus': case 'multiple': case 'muted': - pushBooleanAttribute(target, name, value); + pushBooleanAttribute(target, name.toLowerCase(), value); return; } if ( diff --git a/packages/react-dom-bindings/src/shared/DOMProperty.js b/packages/react-dom-bindings/src/shared/DOMProperty.js index 8bd96110f3a50..521ded4a01749 100644 --- a/packages/react-dom-bindings/src/shared/DOMProperty.js +++ b/packages/react-dom-bindings/src/shared/DOMProperty.js @@ -131,9 +131,6 @@ const properties: {[string]: $FlowFixMe} = {}; [ 'allowFullScreen', 'async', - // Note: there is a special case that prevents it from being written to the DOM - // on the client side because the browsers are inconsistent. Instead we call focus(). - 'autoFocus', 'autoPlay', 'controls', 'default', diff --git a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js index e941899ab4d76..20f5411d6047d 100644 --- a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js +++ b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js @@ -184,10 +184,11 @@ function validateProperty(tagName, name, value, eventRegistry) { switch (typeof value) { case 'boolean': { switch (name) { + case 'autoFocus': case 'checked': - case 'selected': case 'multiple': - case 'muted': { + case 'muted': + case 'selected': { // Boolean properties can accept boolean values return true; } From c6afab95b97392532c5e67139381a02a7ba16b3b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Fri, 31 Mar 2023 00:53:21 -0400 Subject: [PATCH 03/16] Move BOOLEANISH_STRING properties to each of the switch statements --- .../src/client/ReactDOMComponent.js | 106 ++++++++++++++++-- .../src/server/ReactDOMServerFormatConfig.js | 39 ++++++- .../src/shared/DOMProperty.js | 45 +------- .../src/shared/ReactDOMUnknownPropertyHook.js | 10 +- 4 files changed, 144 insertions(+), 56 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 4b1143163f8d4..eb5db2f1a4622 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -16,6 +16,7 @@ import { import {canUseDOM} from 'shared/ExecutionEnvironment'; import {checkHtmlStringCoercion} from 'shared/CheckStringCoercion'; +import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; import { getValueForAttribute, @@ -363,6 +364,41 @@ function setProp( // on server rendering (but we *do* want to emit it in SSR). break; } + case 'contentEditable': + case 'spellCheck': { + // Lower-case Booleanish String + // These are "enumerated" HTML attributes that accept "true" and "false". + // In React, we let users pass `true` and `false` even though technically + // these aren't boolean attributes (they are coerced to strings). + key = key.toLowerCase(); + // Fall-through to the case-sensitive cases + } + // eslint-disable-next-line no-fallthrough + case 'draggable': + case 'value': + case 'autoReverse': + case 'externalResourcesRequired': + case 'focusable': + case 'preserveAlpha': { + // Case-sensitive Booleanish String + // These are "enumerated" SVG attributes that accept "true" and "false". + // In React, we let users pass `true` and `false` even though technically + // these aren't boolean attributes (they are coerced to strings). + // Since these are SVG attributes, their attribute names are case-sensitive. + if ( + value != null && + typeof value !== 'function' && + typeof value !== 'symbol' + ) { + if (__DEV__) { + checkAttributeStringCoercion(value, key); + } + domElement.setAttribute(key, (value: any)); + } else { + domElement.removeAttribute(key); + } + break; + } case 'innerText': // Properties case 'textContent': if (enableCustomElementPropertySupport) { @@ -1215,8 +1251,9 @@ function diffHydratedGenericElement( // Don't bother comparing. We're ignoring all these warnings. continue; } + let key = propKey; // Validate that the properties correspond to their expected values. - switch (propKey) { + switch (key) { case 'children': // Checked above already case 'suppressContentEditableWarning': case 'suppressHydrationWarning': @@ -1239,22 +1276,22 @@ function diffHydratedGenericElement( } continue; case 'style': - extraAttributeNames.delete(propKey); + extraAttributeNames.delete(key); diffHydratedStyles(domElement, nextProp); continue; case 'multiple': { - extraAttributeNames.delete(propKey); + extraAttributeNames.delete(key); const serverValue = (domElement: any).multiple; if (nextProp !== serverValue) { - warnForPropDifference('multiple', serverValue, nextProp); + warnForPropDifference(propKey, serverValue, nextProp); } continue; } case 'muted': { - extraAttributeNames.delete(propKey); + extraAttributeNames.delete(key); const serverValue = (domElement: any).muted; if (nextProp !== serverValue) { - warnForPropDifference('muted', serverValue, nextProp); + warnForPropDifference(propKey, serverValue, nextProp); } continue; } @@ -1262,11 +1299,59 @@ function diffHydratedGenericElement( extraAttributeNames.delete('autofocus'); const serverValue = (domElement: any).autofocus; if (nextProp !== serverValue) { - warnForPropDifference('autoFocus', serverValue, nextProp); + warnForPropDifference(propKey, serverValue, nextProp); } continue; } - default: + case 'contentEditable': + case 'spellCheck': { + // Lower-case Booleanish String + key = key.toLowerCase(); + // Fall-through to the case-sensitive cases + } + // eslint-disable-next-line no-fallthrough + case 'draggable': + case 'autoReverse': + case 'externalResourcesRequired': + case 'focusable': + case 'preserveAlpha': { + // Case-sensitive Booleanish String + extraAttributeNames.delete(key); + let serverValue = domElement.getAttribute(key); + if (serverValue === null) { + // shouldRemoveAttribute + switch (typeof nextProp) { + case 'function': + case 'symbol': // eslint-disable-line + serverValue = nextProp; + } + serverValue = nextProp === undefined ? undefined : null; + } else { + if (nextProp == null) { + // We had an attribute but shouldn't have had one, so read it + // for the error message. + } else { + switch (typeof nextProp) { + case 'function': + case 'symbol': // eslint-disable-line + break; + default: { + if (__DEV__) { + checkAttributeStringCoercion(nextProp, key); + } + if (serverValue === '' + (nextProp: any)) { + serverValue = nextProp; + } + } + } + } + } + if (serverValue !== nextProp) { + warnForPropDifference(propKey, serverValue, nextProp); + } + continue; + } + default: { if ( // shouldIgnoreAttribute // We have already filtered out null/undefined and reserved words. @@ -1293,7 +1378,7 @@ function diffHydratedGenericElement( ownNamespaceDev = getIntrinsicNamespace(tag); } if (ownNamespaceDev === HTML_NAMESPACE) { - extraAttributeNames.delete(propKey.toLowerCase()); + extraAttributeNames.delete(key.toLowerCase()); } else { const standardName = getPossibleStandardName(propKey); if (standardName !== null && standardName !== propKey) { @@ -1305,7 +1390,7 @@ function diffHydratedGenericElement( isMismatchDueToBadCasing = true; extraAttributeNames.delete(standardName); } - extraAttributeNames.delete(propKey); + extraAttributeNames.delete(key); } serverValue = getValueForAttribute(domElement, propKey, nextProp); } @@ -1313,6 +1398,7 @@ function diffHydratedGenericElement( if (nextProp !== serverValue && !isMismatchDueToBadCasing) { warnForPropDifference(propKey, serverValue, nextProp); } + } } } } diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index 74a3e406ad52d..ace4788bd97f7 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -640,9 +640,46 @@ function pushAttribute( return; case 'autoFocus': case 'multiple': - case 'muted': + case 'muted': { pushBooleanAttribute(target, name.toLowerCase(), value); return; + } + case 'contentEditable': + case 'spellCheck': { + // Lower-case Booleanish String + // These are "enumerated" HTML attributes that accept "true" and "false". + // In React, we let users pass `true` and `false` even though technically + // these aren't boolean attributes (they are coerced to strings). + name = name.toLowerCase(); + // Fall-through to the case-sensitive cases + } + // eslint-disable-next-line no-fallthrough + case 'draggable': + case 'value': + case 'autoReverse': + case 'externalResourcesRequired': + case 'focusable': + case 'preserveAlpha': { + // Case-sensitive Booleanish String + // These are "enumerated" SVG attributes that accept "true" and "false". + // In React, we let users pass `true` and `false` even though technically + // these aren't boolean attributes (they are coerced to strings). + // Since these are SVG attributes, their attribute names are case-sensitive. + if ( + value != null && + typeof value !== 'function' && + typeof value !== 'symbol' + ) { + target.push( + attributeSeparator, + stringToChunk(name), + attributeAssign, + stringToChunk(escapeTextForBrowser(value)), + attributeEnd, + ); + } + return; + } } if ( // shouldIgnoreAttribute diff --git a/packages/react-dom-bindings/src/shared/DOMProperty.js b/packages/react-dom-bindings/src/shared/DOMProperty.js index 521ded4a01749..daa1627a972c2 100644 --- a/packages/react-dom-bindings/src/shared/DOMProperty.js +++ b/packages/react-dom-bindings/src/shared/DOMProperty.js @@ -13,12 +13,6 @@ type PropertyType = 0 | 1 | 2 | 3 | 4 | 5 | 6; // Attributes that aren't in the filter are presumed to have this type. export const STRING = 1; -// A string attribute that accepts booleans in React. In HTML, these are called -// "enumerated" attributes with "true" and "false" as possible values. -// When true, it should be set to a "true" string. -// When false, it should be set to a "false" string. -export const BOOLEANISH_STRING = 2; - // A real boolean attribute. // When true, it should be present (set either to an empty string or its name). // When false, it should be omitted. @@ -59,10 +53,7 @@ function PropertyInfoRecord( sanitizeURL: boolean, removeEmptyString: boolean, ) { - this.acceptsBooleans = - type === BOOLEANISH_STRING || - type === BOOLEAN || - type === OVERLOADED_BOOLEAN; + this.acceptsBooleans = type === BOOLEAN || type === OVERLOADED_BOOLEAN; this.attributeName = attributeName; this.attributeNamespace = attributeNamespace; this.type = type; @@ -93,40 +84,6 @@ const properties: {[string]: $FlowFixMe} = {}; ); }); -// These are "enumerated" HTML attributes that accept "true" and "false". -// In React, we let users pass `true` and `false` even though technically -// these aren't boolean attributes (they are coerced to strings). -['contentEditable', 'draggable', 'spellCheck', 'value'].forEach(name => { - // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions - properties[name] = new PropertyInfoRecord( - BOOLEANISH_STRING, - name.toLowerCase(), // attributeName - null, // attributeNamespace - false, // sanitizeURL - false, // removeEmptyString - ); -}); - -// These are "enumerated" SVG attributes that accept "true" and "false". -// In React, we let users pass `true` and `false` even though technically -// these aren't boolean attributes (they are coerced to strings). -// Since these are SVG attributes, their attribute names are case-sensitive. -[ - 'autoReverse', - 'externalResourcesRequired', - 'focusable', - 'preserveAlpha', -].forEach(name => { - // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions - properties[name] = new PropertyInfoRecord( - BOOLEANISH_STRING, - name, // attributeName - null, // attributeNamespace - false, // sanitizeURL - false, // removeEmptyString - ); -}); - // These are HTML boolean attributes. [ 'allowFullScreen', diff --git a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js index 20f5411d6047d..4722b4ee1b97a 100644 --- a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js +++ b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js @@ -188,7 +188,15 @@ function validateProperty(tagName, name, value, eventRegistry) { case 'checked': case 'multiple': case 'muted': - case 'selected': { + case 'selected': + case 'contentEditable': + case 'spellCheck': + case 'draggable': + case 'value': + case 'autoReverse': + case 'externalResourcesRequired': + case 'focusable': + case 'preserveAlpha': { // Boolean properties can accept boolean values return true; } From 6e6543b9dd26cd538f895936cf3f5cb687b1c2ff Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 2 Apr 2023 16:33:04 -0400 Subject: [PATCH 04/16] Inline SVG attribute special cases --- .../src/client/ReactDOMComponent.js | 708 ++++++++++++++++-- .../src/server/ReactDOMServerFormatConfig.js | 244 ++++++ .../src/shared/DOMProperty.js | 96 --- 3 files changed, 888 insertions(+), 160 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index eb5db2f1a4622..351cda7ef4cad 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -399,7 +399,232 @@ function setProp( } break; } - case 'innerText': // Properties + // This is a list of all SVG attributes that need special casing. + // Regular attributes that just accept strings. + case 'accentHeight': + setValueForAttribute(domElement, 'accent-height', value); + break; + case 'alignmentBaseline': + setValueForAttribute(domElement, 'alignment-baseline', value); + break; + case 'arabicForm': + setValueForAttribute(domElement, 'arabic-form', value); + break; + case 'baselineShift': + setValueForAttribute(domElement, 'baseline-shift', value); + break; + case 'capHeight': + setValueForAttribute(domElement, 'cap-height', value); + break; + case 'clipPath': + setValueForAttribute(domElement, 'clip-path', value); + break; + case 'clipRule': + setValueForAttribute(domElement, 'clip-rule', value); + break; + case 'colorInterpolation': + setValueForAttribute(domElement, 'color-interpolation', value); + break; + case 'colorInterpolationFilters': + setValueForAttribute(domElement, 'color-interpolation-filters', value); + break; + case 'colorProfile': + setValueForAttribute(domElement, 'color-profile', value); + break; + case 'colorRendering': + setValueForAttribute(domElement, 'color-rendering', value); + break; + case 'dominantBaseline': + setValueForAttribute(domElement, 'dominant-baseline', value); + break; + case 'enableBackground': + setValueForAttribute(domElement, 'enable-background', value); + break; + case 'fillOpacity': + setValueForAttribute(domElement, 'fill-opacity', value); + break; + case 'fillRule': + setValueForAttribute(domElement, 'fill-rule', value); + break; + case 'floodColor': + setValueForAttribute(domElement, 'flood-color', value); + break; + case 'floodOpacity': + setValueForAttribute(domElement, 'flood-opacity', value); + break; + case 'fontFamily': + setValueForAttribute(domElement, 'font-family', value); + break; + case 'fontSize': + setValueForAttribute(domElement, 'font-size', value); + break; + case 'fontSizeAdjust': + setValueForAttribute(domElement, 'font-size-adjust', value); + break; + case 'fontStretch': + setValueForAttribute(domElement, 'font-stretch', value); + break; + case 'fontStyle': + setValueForAttribute(domElement, 'font-style', value); + break; + case 'fontVariant': + setValueForAttribute(domElement, 'font-variant', value); + break; + case 'fontWeight': + setValueForAttribute(domElement, 'font-weight', value); + break; + case 'glyphName': + setValueForAttribute(domElement, 'glyph-name', value); + break; + case 'glyphOrientationHorizontal': + setValueForAttribute(domElement, 'glyph-orientation-horizontal', value); + break; + case 'glyphOrientationVertical': + setValueForAttribute(domElement, 'glyph-orientation-vertical', value); + break; + case 'horizAdvX': + setValueForAttribute(domElement, 'horiz-adv-x', value); + break; + case 'horizOriginX': + setValueForAttribute(domElement, 'horiz-origin-x', value); + break; + case 'imageRendering': + setValueForAttribute(domElement, 'image-rendering', value); + break; + case 'letterSpacing': + setValueForAttribute(domElement, 'letter-spacing', value); + break; + case 'lightingColor': + setValueForAttribute(domElement, 'lighting-color', value); + break; + case 'markerEnd': + setValueForAttribute(domElement, 'marker-end', value); + break; + case 'markerMid': + setValueForAttribute(domElement, 'marker-mid', value); + break; + case 'markerStart': + setValueForAttribute(domElement, 'marker-start', value); + break; + case 'overlinePosition': + setValueForAttribute(domElement, 'overline-position', value); + break; + case 'overlineThickness': + setValueForAttribute(domElement, 'overline-thickness', value); + break; + case 'paintOrder': + setValueForAttribute(domElement, 'paint-order', value); + break; + case 'panose-1': + setValueForAttribute(domElement, 'panose-1', value); + break; + case 'pointerEvents': + setValueForAttribute(domElement, 'pointer-events', value); + break; + case 'renderingIntent': + setValueForAttribute(domElement, 'rendering-intent', value); + break; + case 'shapeRendering': + setValueForAttribute(domElement, 'shape-rendering', value); + break; + case 'stopColor': + setValueForAttribute(domElement, 'stop-color', value); + break; + case 'stopOpacity': + setValueForAttribute(domElement, 'stop-opacity', value); + break; + case 'strikethroughPosition': + setValueForAttribute(domElement, 'strikethrough-position', value); + break; + case 'strikethroughThickness': + setValueForAttribute(domElement, 'strikethrough-thickness', value); + break; + case 'strokeDasharray': + setValueForAttribute(domElement, 'stroke-dasharray', value); + break; + case 'strokeDashoffset': + setValueForAttribute(domElement, 'stroke-dashoffset', value); + break; + case 'strokeLinecap': + setValueForAttribute(domElement, 'stroke-linecap', value); + break; + case 'strokeLinejoin': + setValueForAttribute(domElement, 'stroke-linejoin', value); + break; + case 'strokeMiterlimit': + setValueForAttribute(domElement, 'stroke-miterlimit', value); + break; + case 'strokeOpacity': + setValueForAttribute(domElement, 'stroke-opacity', value); + break; + case 'strokeWidth': + setValueForAttribute(domElement, 'stroke-width', value); + break; + case 'textAnchor': + setValueForAttribute(domElement, 'text-anchor', value); + break; + case 'textDecoration': + setValueForAttribute(domElement, 'text-decoration', value); + break; + case 'textRendering': + setValueForAttribute(domElement, 'text-rendering', value); + break; + case 'transformOrigin': + setValueForAttribute(domElement, 'transform-origin', value); + break; + case 'underlinePosition': + setValueForAttribute(domElement, 'underline-position', value); + break; + case 'underlineThickness': + setValueForAttribute(domElement, 'underline-thickness', value); + break; + case 'unicodeBidi': + setValueForAttribute(domElement, 'unicode-bidi', value); + break; + case 'unicodeRange': + setValueForAttribute(domElement, 'unicode-range', value); + break; + case 'unitsPerEm': + setValueForAttribute(domElement, 'units-per-em', value); + break; + case 'vAlphabetic': + setValueForAttribute(domElement, 'v-alphabetic', value); + break; + case 'vHanging': + setValueForAttribute(domElement, 'v-hanging', value); + break; + case 'vIdeographic': + setValueForAttribute(domElement, 'v-ideographic', value); + break; + case 'vMathematical': + setValueForAttribute(domElement, 'v-mathematical', value); + break; + case 'vectorEffect': + setValueForAttribute(domElement, 'vector-effect', value); + break; + case 'vertAdvY': + setValueForAttribute(domElement, 'vert-adv-y', value); + break; + case 'vertOriginX': + setValueForAttribute(domElement, 'vert-origin-x', value); + break; + case 'vertOriginY': + setValueForAttribute(domElement, 'vert-origin-y', value); + break; + case 'wordSpacing': + setValueForAttribute(domElement, 'word-spacing', value); + break; + case 'writingMode': + setValueForAttribute(domElement, 'writing-mode', value); + break; + case 'xmlnsXlink': + setValueForAttribute(domElement, 'xmlns:xlink', value); + break; + case 'xHeight': + setValueForAttribute(domElement, 'x-height', value); + break; + // Properties that should not be allowed on custom elements. + case 'innerText': case 'textContent': if (enableCustomElementPropertySupport) { break; @@ -1120,24 +1345,65 @@ function diffHydratedStyles(domElement: Element, value: mixed) { } } +function hydrateAttribute( + domElement: Element, + attributeName: string, + value: any, + extraAttributes: Set, +): void { + extraAttributes.delete(attributeName); + let serverValue = domElement.getAttribute(attributeName); + if (serverValue === null) { + // shouldRemoveAttribute + switch (typeof value) { + case 'function': + case 'symbol': // eslint-disable-line + serverValue = value; + } + serverValue = value === undefined ? undefined : null; + } else { + if (value == null) { + // We had an attribute but shouldn't have had one, so read it + // for the error message. + } else { + switch (typeof value) { + case 'function': + case 'symbol': // eslint-disable-line + break; + default: { + if (__DEV__) { + checkAttributeStringCoercion(value, attributeName); + } + if (serverValue === '' + value) { + serverValue = value; + } + } + } + } + } + if (serverValue !== value) { + warnForPropDifference(attributeName, serverValue, value); + } +} + function diffHydratedCustomComponent( domElement: Element, tag: string, props: Object, parentNamespaceDev: string, - extraAttributeNames: Set, + extraAttributes: Set, ) { for (const propKey in props) { if (!props.hasOwnProperty(propKey)) { continue; } - const nextProp = props[propKey]; - if (nextProp == null) { + const value = props[propKey]; + if (value == null) { continue; } if (registrationNameDependencies.hasOwnProperty(propKey)) { - if (typeof nextProp !== 'function') { - warnForInvalidEventListener(propKey, nextProp); + if (typeof value !== 'function') { + warnForInvalidEventListener(propKey, value); } continue; } @@ -1157,7 +1423,7 @@ function diffHydratedCustomComponent( continue; case 'dangerouslySetInnerHTML': const serverHTML = domElement.innerHTML; - const nextHtml = nextProp ? nextProp.__html : undefined; + const nextHtml = value ? value.__html : undefined; if (nextHtml != null) { const expectedHTML = normalizeHTML(domElement, nextHtml); if (expectedHTML !== serverHTML) { @@ -1166,8 +1432,8 @@ function diffHydratedCustomComponent( } continue; case 'style': - extraAttributeNames.delete(propKey); - diffHydratedStyles(domElement, nextProp); + extraAttributes.delete(propKey); + diffHydratedStyles(domElement, value); continue; case 'offsetParent': case 'offsetTop': @@ -1178,7 +1444,7 @@ function diffHydratedCustomComponent( case 'outerText': case 'outerHTML': if (enableCustomElementPropertySupport) { - extraAttributeNames.delete(propKey.toLowerCase()); + extraAttributes.delete(propKey.toLowerCase()); if (__DEV__) { console.error( 'Assignment to read-only property will result in a no-op: `%s`', @@ -1191,14 +1457,14 @@ function diffHydratedCustomComponent( case 'className': if (enableCustomElementPropertySupport) { // className is a special cased property on the server to render as an attribute. - extraAttributeNames.delete('class'); + extraAttributes.delete('class'); const serverValue = getValueForAttributeOnCustomComponent( domElement, 'class', - nextProp, + value, ); - if (nextProp !== serverValue) { - warnForPropDifference('className', serverValue, nextProp); + if (value !== serverValue) { + warnForPropDifference('className', serverValue, value); } continue; } @@ -1209,17 +1475,17 @@ function diffHydratedCustomComponent( ownNamespaceDev = getIntrinsicNamespace(tag); } if (ownNamespaceDev === HTML_NAMESPACE) { - extraAttributeNames.delete(propKey.toLowerCase()); + extraAttributes.delete(propKey.toLowerCase()); } else { - extraAttributeNames.delete(propKey); + extraAttributes.delete(propKey); } const serverValue = getValueForAttributeOnCustomComponent( domElement, propKey, - nextProp, + value, ); - if (nextProp !== serverValue) { - warnForPropDifference(propKey, serverValue, nextProp); + if (value !== serverValue) { + warnForPropDifference(propKey, serverValue, value); } } } @@ -1231,19 +1497,19 @@ function diffHydratedGenericElement( tag: string, props: Object, parentNamespaceDev: string, - extraAttributeNames: Set, + extraAttributes: Set, ) { for (const propKey in props) { if (!props.hasOwnProperty(propKey)) { continue; } - const nextProp = props[propKey]; - if (nextProp == null) { + const value = props[propKey]; + if (value == null) { continue; } if (registrationNameDependencies.hasOwnProperty(propKey)) { - if (typeof nextProp !== 'function') { - warnForInvalidEventListener(propKey, nextProp); + if (typeof value !== 'function') { + warnForInvalidEventListener(propKey, value); } continue; } @@ -1267,7 +1533,7 @@ function diffHydratedGenericElement( continue; case 'dangerouslySetInnerHTML': const serverHTML = domElement.innerHTML; - const nextHtml = nextProp ? nextProp.__html : undefined; + const nextHtml = value ? value.__html : undefined; if (nextHtml != null) { const expectedHTML = normalizeHTML(domElement, nextHtml); if (expectedHTML !== serverHTML) { @@ -1276,30 +1542,30 @@ function diffHydratedGenericElement( } continue; case 'style': - extraAttributeNames.delete(key); - diffHydratedStyles(domElement, nextProp); + extraAttributes.delete(key); + diffHydratedStyles(domElement, value); continue; case 'multiple': { - extraAttributeNames.delete(key); + extraAttributes.delete(key); const serverValue = (domElement: any).multiple; - if (nextProp !== serverValue) { - warnForPropDifference(propKey, serverValue, nextProp); + if (value !== serverValue) { + warnForPropDifference(propKey, serverValue, value); } continue; } case 'muted': { - extraAttributeNames.delete(key); + extraAttributes.delete(key); const serverValue = (domElement: any).muted; - if (nextProp !== serverValue) { - warnForPropDifference(propKey, serverValue, nextProp); + if (value !== serverValue) { + warnForPropDifference(propKey, serverValue, value); } continue; } case 'autoFocus': { - extraAttributeNames.delete('autofocus'); + extraAttributes.delete('autofocus'); const serverValue = (domElement: any).autofocus; - if (nextProp !== serverValue) { - warnForPropDifference(propKey, serverValue, nextProp); + if (value !== serverValue) { + warnForPropDifference(propKey, serverValue, value); } continue; } @@ -1316,41 +1582,358 @@ function diffHydratedGenericElement( case 'focusable': case 'preserveAlpha': { // Case-sensitive Booleanish String - extraAttributeNames.delete(key); + extraAttributes.delete(key); let serverValue = domElement.getAttribute(key); if (serverValue === null) { // shouldRemoveAttribute - switch (typeof nextProp) { + switch (typeof value) { case 'function': case 'symbol': // eslint-disable-line - serverValue = nextProp; + serverValue = value; } - serverValue = nextProp === undefined ? undefined : null; + serverValue = value === undefined ? undefined : null; } else { - if (nextProp == null) { + if (value == null) { // We had an attribute but shouldn't have had one, so read it // for the error message. } else { - switch (typeof nextProp) { + switch (typeof value) { case 'function': case 'symbol': // eslint-disable-line break; default: { if (__DEV__) { - checkAttributeStringCoercion(nextProp, key); + checkAttributeStringCoercion(value, key); } - if (serverValue === '' + (nextProp: any)) { - serverValue = nextProp; + if (serverValue === '' + (value: any)) { + serverValue = value; } } } } } - if (serverValue !== nextProp) { - warnForPropDifference(propKey, serverValue, nextProp); + if (serverValue !== value) { + warnForPropDifference(propKey, serverValue, value); } continue; } + case 'accentHeight': + hydrateAttribute(domElement, 'accent-height', value, extraAttributes); + continue; + case 'alignmentBaseline': + hydrateAttribute( + domElement, + 'alignment-baseline', + value, + extraAttributes, + ); + continue; + case 'arabicForm': + hydrateAttribute(domElement, 'arabic-form', value, extraAttributes); + continue; + case 'baselineShift': + hydrateAttribute(domElement, 'baseline-shift', value, extraAttributes); + continue; + case 'capHeight': + hydrateAttribute(domElement, 'cap-height', value, extraAttributes); + continue; + case 'clipPath': + hydrateAttribute(domElement, 'clip-path', value, extraAttributes); + continue; + case 'clipRule': + hydrateAttribute(domElement, 'clip-rule', value, extraAttributes); + continue; + case 'colorInterpolation': + hydrateAttribute( + domElement, + 'color-interpolation', + value, + extraAttributes, + ); + continue; + case 'colorInterpolationFilters': + hydrateAttribute( + domElement, + 'color-interpolation-filters', + value, + extraAttributes, + ); + continue; + case 'colorProfile': + hydrateAttribute(domElement, 'color-profile', value, extraAttributes); + continue; + case 'colorRendering': + hydrateAttribute(domElement, 'color-rendering', value, extraAttributes); + continue; + case 'dominantBaseline': + hydrateAttribute( + domElement, + 'dominant-baseline', + value, + extraAttributes, + ); + continue; + case 'enableBackground': + hydrateAttribute( + domElement, + 'enable-background', + value, + extraAttributes, + ); + continue; + case 'fillOpacity': + hydrateAttribute(domElement, 'fill-opacity', value, extraAttributes); + continue; + case 'fillRule': + hydrateAttribute(domElement, 'fill-rule', value, extraAttributes); + continue; + case 'floodColor': + hydrateAttribute(domElement, 'flood-color', value, extraAttributes); + continue; + case 'floodOpacity': + hydrateAttribute(domElement, 'flood-opacity', value, extraAttributes); + continue; + case 'fontFamily': + hydrateAttribute(domElement, 'font-family', value, extraAttributes); + continue; + case 'fontSize': + hydrateAttribute(domElement, 'font-size', value, extraAttributes); + continue; + case 'fontSizeAdjust': + hydrateAttribute( + domElement, + 'font-size-adjust', + value, + extraAttributes, + ); + continue; + case 'fontStretch': + hydrateAttribute(domElement, 'font-stretch', value, extraAttributes); + continue; + case 'fontStyle': + hydrateAttribute(domElement, 'font-style', value, extraAttributes); + continue; + case 'fontVariant': + hydrateAttribute(domElement, 'font-variant', value, extraAttributes); + continue; + case 'fontWeight': + hydrateAttribute(domElement, 'font-weight', value, extraAttributes); + continue; + case 'glyphName': + hydrateAttribute(domElement, 'glyph-name', value, extraAttributes); + continue; + case 'glyphOrientationHorizontal': + hydrateAttribute( + domElement, + 'glyph-orientation-horizontal', + value, + extraAttributes, + ); + continue; + case 'glyphOrientationVertical': + hydrateAttribute( + domElement, + 'glyph-orientation-vertical', + value, + extraAttributes, + ); + continue; + case 'horizAdvX': + hydrateAttribute(domElement, 'horiz-adv-x', value, extraAttributes); + continue; + case 'horizOriginX': + hydrateAttribute(domElement, 'horiz-origin-x', value, extraAttributes); + continue; + case 'imageRendering': + hydrateAttribute(domElement, 'image-rendering', value, extraAttributes); + continue; + case 'letterSpacing': + hydrateAttribute(domElement, 'letter-spacing', value, extraAttributes); + continue; + case 'lightingColor': + hydrateAttribute(domElement, 'lighting-color', value, extraAttributes); + continue; + case 'markerEnd': + hydrateAttribute(domElement, 'marker-end', value, extraAttributes); + continue; + case 'markerMid': + hydrateAttribute(domElement, 'marker-mid', value, extraAttributes); + continue; + case 'markerStart': + hydrateAttribute(domElement, 'marker-start', value, extraAttributes); + continue; + case 'overlinePosition': + hydrateAttribute( + domElement, + 'overline-position', + value, + extraAttributes, + ); + continue; + case 'overlineThickness': + hydrateAttribute( + domElement, + 'overline-thickness', + value, + extraAttributes, + ); + continue; + case 'paintOrder': + hydrateAttribute(domElement, 'paint-order', value, extraAttributes); + continue; + case 'panose-1': + hydrateAttribute(domElement, 'panose-1', value, extraAttributes); + continue; + case 'pointerEvents': + hydrateAttribute(domElement, 'pointer-events', value, extraAttributes); + continue; + case 'renderingIntent': + hydrateAttribute( + domElement, + 'rendering-intent', + value, + extraAttributes, + ); + continue; + case 'shapeRendering': + hydrateAttribute(domElement, 'shape-rendering', value, extraAttributes); + continue; + case 'stopColor': + hydrateAttribute(domElement, 'stop-color', value, extraAttributes); + continue; + case 'stopOpacity': + hydrateAttribute(domElement, 'stop-opacity', value, extraAttributes); + continue; + case 'strikethroughPosition': + hydrateAttribute( + domElement, + 'strikethrough-position', + value, + extraAttributes, + ); + continue; + case 'strikethroughThickness': + hydrateAttribute( + domElement, + 'strikethrough-thickness', + value, + extraAttributes, + ); + continue; + case 'strokeDasharray': + hydrateAttribute( + domElement, + 'stroke-dasharray', + value, + extraAttributes, + ); + continue; + case 'strokeDashoffset': + hydrateAttribute( + domElement, + 'stroke-dashoffset', + value, + extraAttributes, + ); + continue; + case 'strokeLinecap': + hydrateAttribute(domElement, 'stroke-linecap', value, extraAttributes); + continue; + case 'strokeLinejoin': + hydrateAttribute(domElement, 'stroke-linejoin', value, extraAttributes); + continue; + case 'strokeMiterlimit': + hydrateAttribute( + domElement, + 'stroke-miterlimit', + value, + extraAttributes, + ); + continue; + case 'strokeOpacity': + hydrateAttribute(domElement, 'stroke-opacity', value, extraAttributes); + continue; + case 'strokeWidth': + hydrateAttribute(domElement, 'stroke-width', value, extraAttributes); + continue; + case 'textAnchor': + hydrateAttribute(domElement, 'text-anchor', value, extraAttributes); + continue; + case 'textDecoration': + hydrateAttribute(domElement, 'text-decoration', value, extraAttributes); + continue; + case 'textRendering': + hydrateAttribute(domElement, 'text-rendering', value, extraAttributes); + continue; + case 'transformOrigin': + hydrateAttribute( + domElement, + 'transform-origin', + value, + extraAttributes, + ); + continue; + case 'underlinePosition': + hydrateAttribute( + domElement, + 'underline-position', + value, + extraAttributes, + ); + continue; + case 'underlineThickness': + hydrateAttribute( + domElement, + 'underline-thickness', + value, + extraAttributes, + ); + continue; + case 'unicodeBidi': + hydrateAttribute(domElement, 'unicode-bidi', value, extraAttributes); + continue; + case 'unicodeRange': + hydrateAttribute(domElement, 'unicode-range', value, extraAttributes); + continue; + case 'unitsPerEm': + hydrateAttribute(domElement, 'units-per-em', value, extraAttributes); + continue; + case 'vAlphabetic': + hydrateAttribute(domElement, 'v-alphabetic', value, extraAttributes); + continue; + case 'vHanging': + hydrateAttribute(domElement, 'v-hanging', value, extraAttributes); + continue; + case 'vIdeographic': + hydrateAttribute(domElement, 'v-ideographic', value, extraAttributes); + continue; + case 'vMathematical': + hydrateAttribute(domElement, 'v-mathematical', value, extraAttributes); + continue; + case 'vectorEffect': + hydrateAttribute(domElement, 'vector-effect', value, extraAttributes); + continue; + case 'vertAdvY': + hydrateAttribute(domElement, 'vert-adv-y', value, extraAttributes); + continue; + case 'vertOriginX': + hydrateAttribute(domElement, 'vert-origin-x', value, extraAttributes); + continue; + case 'vertOriginY': + hydrateAttribute(domElement, 'vert-origin-y', value, extraAttributes); + continue; + case 'wordSpacing': + hydrateAttribute(domElement, 'word-spacing', value, extraAttributes); + continue; + case 'writingMode': + hydrateAttribute(domElement, 'writing-mode', value, extraAttributes); + continue; + case 'xmlnsXlink': + hydrateAttribute(domElement, 'xmlns:xlink', value, extraAttributes); + continue; + case 'xHeight': + hydrateAttribute(domElement, 'x-height', value, extraAttributes); + continue; default: { if ( // shouldIgnoreAttribute @@ -1365,11 +1948,11 @@ function diffHydratedGenericElement( let isMismatchDueToBadCasing = false; let serverValue; if (propertyInfo !== null) { - extraAttributeNames.delete(propertyInfo.attributeName); + extraAttributes.delete(propertyInfo.attributeName); serverValue = getValueForProperty( domElement, propKey, - nextProp, + value, propertyInfo, ); } else { @@ -1378,7 +1961,7 @@ function diffHydratedGenericElement( ownNamespaceDev = getIntrinsicNamespace(tag); } if (ownNamespaceDev === HTML_NAMESPACE) { - extraAttributeNames.delete(key.toLowerCase()); + extraAttributes.delete(key.toLowerCase()); } else { const standardName = getPossibleStandardName(propKey); if (standardName !== null && standardName !== propKey) { @@ -1388,15 +1971,15 @@ function diffHydratedGenericElement( // However, we already warn about bad casing elsewhere. // So we'll skip the misleading extra mismatch warning in this case. isMismatchDueToBadCasing = true; - extraAttributeNames.delete(standardName); + extraAttributes.delete(standardName); } - extraAttributeNames.delete(key); + extraAttributes.delete(key); } - serverValue = getValueForAttribute(domElement, propKey, nextProp); + serverValue = getValueForAttribute(domElement, propKey, value); } - if (nextProp !== serverValue && !isMismatchDueToBadCasing) { - warnForPropDifference(propKey, serverValue, nextProp); + if (value !== serverValue && !isMismatchDueToBadCasing) { + warnForPropDifference(propKey, serverValue, value); } } } @@ -1528,7 +2111,7 @@ export function diffHydratedProperties( } if (__DEV__ && shouldWarnDev) { - const extraAttributeNames: Set = new Set(); + const extraAttributes: Set = new Set(); const attributes = domElement.attributes; for (let i = 0; i < attributes.length; i++) { const name = attributes[i].name.toLowerCase(); @@ -1544,7 +2127,7 @@ export function diffHydratedProperties( default: // Intentionally use the original name. // See discussion in https://github.com/facebook/react/pull/10676. - extraAttributeNames.add(attributes[i].name); + extraAttributes.add(attributes[i].name); } } if (isCustomElement(tag, props)) { @@ -1553,7 +2136,7 @@ export function diffHydratedProperties( tag, props, parentNamespaceDev, - extraAttributeNames, + extraAttributes, ); } else { diffHydratedGenericElement( @@ -1561,14 +2144,11 @@ export function diffHydratedProperties( tag, props, parentNamespaceDev, - extraAttributeNames, + extraAttributes, ); } - if ( - extraAttributeNames.size > 0 && - props.suppressHydrationWarning !== true - ) { - warnForExtraAttributes(extraAttributeNames); + if (extraAttributes.size > 0 && props.suppressHydrationWarning !== true) { + warnForExtraAttributes(extraAttributes); } } diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index ace4788bd97f7..5c32e7d4f9beb 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -621,6 +621,26 @@ function pushBooleanAttribute( } } +function pushStringAttribute( + target: Array, + name: string, + value: string | boolean | number | Function | Object, // not null or undefined +): void { + if ( + typeof value !== 'function' && + typeof value !== 'symbol' && + typeof value !== 'boolean' + ) { + target.push( + attributeSeparator, + stringToChunk(name), + attributeAssign, + stringToChunk(escapeTextForBrowser(value)), + attributeEnd, + ); + } +} + function pushAttribute( target: Array, name: string, @@ -680,6 +700,230 @@ function pushAttribute( } return; } + // This is a list of all SVG attributes that need special casing. + // Regular attributes that just accept strings. + case 'accentHeight': + pushStringAttribute(target, 'accent-height', value); + return; + case 'alignmentBaseline': + pushStringAttribute(target, 'alignment-baseline', value); + return; + case 'arabicForm': + pushStringAttribute(target, 'arabic-form', value); + return; + case 'baselineShift': + pushStringAttribute(target, 'baseline-shift', value); + return; + case 'capHeight': + pushStringAttribute(target, 'cap-height', value); + return; + case 'clipPath': + pushStringAttribute(target, 'clip-path', value); + return; + case 'clipRule': + pushStringAttribute(target, 'clip-rule', value); + return; + case 'colorInterpolation': + pushStringAttribute(target, 'color-interpolation', value); + return; + case 'colorInterpolationFilters': + pushStringAttribute(target, 'color-interpolation-filters', value); + return; + case 'colorProfile': + pushStringAttribute(target, 'color-profile', value); + return; + case 'colorRendering': + pushStringAttribute(target, 'color-rendering', value); + return; + case 'dominantBaseline': + pushStringAttribute(target, 'dominant-baseline', value); + return; + case 'enableBackground': + pushStringAttribute(target, 'enable-background', value); + return; + case 'fillOpacity': + pushStringAttribute(target, 'fill-opacity', value); + return; + case 'fillRule': + pushStringAttribute(target, 'fill-rule', value); + return; + case 'floodColor': + pushStringAttribute(target, 'flood-color', value); + return; + case 'floodOpacity': + pushStringAttribute(target, 'flood-opacity', value); + return; + case 'fontFamily': + pushStringAttribute(target, 'font-family', value); + return; + case 'fontSize': + pushStringAttribute(target, 'font-size', value); + return; + case 'fontSizeAdjust': + pushStringAttribute(target, 'font-size-adjust', value); + return; + case 'fontStretch': + pushStringAttribute(target, 'font-stretch', value); + return; + case 'fontStyle': + pushStringAttribute(target, 'font-style', value); + return; + case 'fontVariant': + pushStringAttribute(target, 'font-variant', value); + return; + case 'fontWeight': + pushStringAttribute(target, 'font-weight', value); + return; + case 'glyphName': + pushStringAttribute(target, 'glyph-name', value); + return; + case 'glyphOrientationHorizontal': + pushStringAttribute(target, 'glyph-orientation-horizontal', value); + return; + case 'glyphOrientationVertical': + pushStringAttribute(target, 'glyph-orientation-vertical', value); + return; + case 'horizAdvX': + pushStringAttribute(target, 'horiz-adv-x', value); + return; + case 'horizOriginX': + pushStringAttribute(target, 'horiz-origin-x', value); + return; + case 'imageRendering': + pushStringAttribute(target, 'image-rendering', value); + return; + case 'letterSpacing': + pushStringAttribute(target, 'letter-spacing', value); + return; + case 'lightingColor': + pushStringAttribute(target, 'lighting-color', value); + return; + case 'markerEnd': + pushStringAttribute(target, 'marker-end', value); + return; + case 'markerMid': + pushStringAttribute(target, 'marker-mid', value); + return; + case 'markerStart': + pushStringAttribute(target, 'marker-start', value); + return; + case 'overlinePosition': + pushStringAttribute(target, 'overline-position', value); + return; + case 'overlineThickness': + pushStringAttribute(target, 'overline-thickness', value); + return; + case 'paintOrder': + pushStringAttribute(target, 'paint-order', value); + return; + case 'panose-1': + pushStringAttribute(target, 'panose-1', value); + return; + case 'pointerEvents': + pushStringAttribute(target, 'pointer-events', value); + return; + case 'renderingIntent': + pushStringAttribute(target, 'rendering-intent', value); + return; + case 'shapeRendering': + pushStringAttribute(target, 'shape-rendering', value); + return; + case 'stopColor': + pushStringAttribute(target, 'stop-color', value); + return; + case 'stopOpacity': + pushStringAttribute(target, 'stop-opacity', value); + return; + case 'strikethroughPosition': + pushStringAttribute(target, 'strikethrough-position', value); + return; + case 'strikethroughThickness': + pushStringAttribute(target, 'strikethrough-thickness', value); + return; + case 'strokeDasharray': + pushStringAttribute(target, 'stroke-dasharray', value); + return; + case 'strokeDashoffset': + pushStringAttribute(target, 'stroke-dashoffset', value); + return; + case 'strokeLinecap': + pushStringAttribute(target, 'stroke-linecap', value); + return; + case 'strokeLinejoin': + pushStringAttribute(target, 'stroke-linejoin', value); + return; + case 'strokeMiterlimit': + pushStringAttribute(target, 'stroke-miterlimit', value); + return; + case 'strokeOpacity': + pushStringAttribute(target, 'stroke-opacity', value); + return; + case 'strokeWidth': + pushStringAttribute(target, 'stroke-width', value); + return; + case 'textAnchor': + pushStringAttribute(target, 'text-anchor', value); + return; + case 'textDecoration': + pushStringAttribute(target, 'text-decoration', value); + return; + case 'textRendering': + pushStringAttribute(target, 'text-rendering', value); + return; + case 'transformOrigin': + pushStringAttribute(target, 'transform-origin', value); + return; + case 'underlinePosition': + pushStringAttribute(target, 'underline-position', value); + return; + case 'underlineThickness': + pushStringAttribute(target, 'underline-thickness', value); + return; + case 'unicodeBidi': + pushStringAttribute(target, 'unicode-bidi', value); + return; + case 'unicodeRange': + pushStringAttribute(target, 'unicode-range', value); + return; + case 'unitsPerEm': + pushStringAttribute(target, 'units-per-em', value); + return; + case 'vAlphabetic': + pushStringAttribute(target, 'v-alphabetic', value); + return; + case 'vHanging': + pushStringAttribute(target, 'v-hanging', value); + return; + case 'vIdeographic': + pushStringAttribute(target, 'v-ideographic', value); + return; + case 'vMathematical': + pushStringAttribute(target, 'v-mathematical', value); + return; + case 'vectorEffect': + pushStringAttribute(target, 'vector-effect', value); + return; + case 'vertAdvY': + pushStringAttribute(target, 'vert-adv-y', value); + return; + case 'vertOriginX': + pushStringAttribute(target, 'vert-origin-x', value); + return; + case 'vertOriginY': + pushStringAttribute(target, 'vert-origin-y', value); + return; + case 'wordSpacing': + pushStringAttribute(target, 'word-spacing', value); + return; + case 'writingMode': + pushStringAttribute(target, 'writing-mode', value); + return; + case 'xmlnsXlink': + pushStringAttribute(target, 'xmlns:xlink', value); + return; + case 'xHeight': + pushStringAttribute(target, 'x-height', value); + return; } if ( // shouldIgnoreAttribute diff --git a/packages/react-dom-bindings/src/shared/DOMProperty.js b/packages/react-dom-bindings/src/shared/DOMProperty.js index daa1627a972c2..2a783c5ba6858 100644 --- a/packages/react-dom-bindings/src/shared/DOMProperty.js +++ b/packages/react-dom-bindings/src/shared/DOMProperty.js @@ -176,102 +176,6 @@ const properties: {[string]: $FlowFixMe} = {}; const CAMELIZE = /[\-\:]([a-z])/g; const capitalize = (token: string) => token[1].toUpperCase(); -// This is a list of all SVG attributes that need special casing, namespacing, -// or boolean value assignment. Regular attributes that just accept strings -// and have the same names are omitted, just like in the HTML attribute filter. -// Some of these attributes can be hard to find. This list was created by -// scraping the MDN documentation. -[ - 'accent-height', - 'alignment-baseline', - 'arabic-form', - 'baseline-shift', - 'cap-height', - 'clip-path', - 'clip-rule', - 'color-interpolation', - 'color-interpolation-filters', - 'color-profile', - 'color-rendering', - 'dominant-baseline', - 'enable-background', - 'fill-opacity', - 'fill-rule', - 'flood-color', - 'flood-opacity', - 'font-family', - 'font-size', - 'font-size-adjust', - 'font-stretch', - 'font-style', - 'font-variant', - 'font-weight', - 'glyph-name', - 'glyph-orientation-horizontal', - 'glyph-orientation-vertical', - 'horiz-adv-x', - 'horiz-origin-x', - 'image-rendering', - 'letter-spacing', - 'lighting-color', - 'marker-end', - 'marker-mid', - 'marker-start', - 'overline-position', - 'overline-thickness', - 'paint-order', - 'panose-1', - 'pointer-events', - 'rendering-intent', - 'shape-rendering', - 'stop-color', - 'stop-opacity', - 'strikethrough-position', - 'strikethrough-thickness', - 'stroke-dasharray', - 'stroke-dashoffset', - 'stroke-linecap', - 'stroke-linejoin', - 'stroke-miterlimit', - 'stroke-opacity', - 'stroke-width', - 'text-anchor', - 'text-decoration', - 'text-rendering', - 'transform-origin', - 'underline-position', - 'underline-thickness', - 'unicode-bidi', - 'unicode-range', - 'units-per-em', - 'v-alphabetic', - 'v-hanging', - 'v-ideographic', - 'v-mathematical', - 'vector-effect', - 'vert-adv-y', - 'vert-origin-x', - 'vert-origin-y', - 'word-spacing', - 'writing-mode', - 'xmlns:xlink', - 'x-height', - - // 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(attributeName => { - const name = attributeName.replace(CAMELIZE, capitalize); - // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions - properties[name] = new PropertyInfoRecord( - STRING, - attributeName, - null, // attributeNamespace - false, // sanitizeURL - false, // removeEmptyString - ); -}); - // String SVG attributes with the xlink namespace. [ 'xlink:actuate', From e105c2d748159b11dadf52fb2f2df09af7ab9508 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 2 Apr 2023 16:38:13 -0400 Subject: [PATCH 05/16] Move comparison in just to save some code --- .../src/client/ReactDOMComponent.js | 45 ++++++------------- 1 file changed, 14 insertions(+), 31 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 351cda7ef4cad..d828571615eed 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -123,6 +123,9 @@ function warnForPropDifference( if (didWarnInvalidHydration) { return; } + if (serverValue === clientValue) { + return; + } const normalizedClientValue = normalizeMarkupForTextOrAttribute(clientValue); const normalizedServerValue = @@ -1339,9 +1342,7 @@ function diffHydratedStyles(domElement: Element, value: mixed) { if (canDiffStyleForHydrationWarning) { const expectedStyle = createDangerousStringForStyles(value); const serverValue = domElement.getAttribute('style'); - if (expectedStyle !== serverValue) { - warnForPropDifference('style', serverValue, expectedStyle); - } + warnForPropDifference('style', serverValue, expectedStyle); } } @@ -1381,9 +1382,7 @@ function hydrateAttribute( } } } - if (serverValue !== value) { - warnForPropDifference(attributeName, serverValue, value); - } + warnForPropDifference(attributeName, serverValue, value); } function diffHydratedCustomComponent( @@ -1426,9 +1425,7 @@ function diffHydratedCustomComponent( const nextHtml = value ? value.__html : undefined; if (nextHtml != null) { const expectedHTML = normalizeHTML(domElement, nextHtml); - if (expectedHTML !== serverHTML) { - warnForPropDifference(propKey, serverHTML, expectedHTML); - } + warnForPropDifference(propKey, serverHTML, expectedHTML); } continue; case 'style': @@ -1463,9 +1460,7 @@ function diffHydratedCustomComponent( 'class', value, ); - if (value !== serverValue) { - warnForPropDifference('className', serverValue, value); - } + warnForPropDifference('className', serverValue, value); continue; } // eslint-disable-next-line no-fallthrough @@ -1484,9 +1479,7 @@ function diffHydratedCustomComponent( propKey, value, ); - if (value !== serverValue) { - warnForPropDifference(propKey, serverValue, value); - } + warnForPropDifference(propKey, serverValue, value); } } } @@ -1536,9 +1529,7 @@ function diffHydratedGenericElement( const nextHtml = value ? value.__html : undefined; if (nextHtml != null) { const expectedHTML = normalizeHTML(domElement, nextHtml); - if (expectedHTML !== serverHTML) { - warnForPropDifference(propKey, serverHTML, expectedHTML); - } + warnForPropDifference(propKey, serverHTML, expectedHTML); } continue; case 'style': @@ -1548,25 +1539,19 @@ function diffHydratedGenericElement( case 'multiple': { extraAttributes.delete(key); const serverValue = (domElement: any).multiple; - if (value !== serverValue) { - warnForPropDifference(propKey, serverValue, value); - } + warnForPropDifference(propKey, serverValue, value); continue; } case 'muted': { extraAttributes.delete(key); const serverValue = (domElement: any).muted; - if (value !== serverValue) { - warnForPropDifference(propKey, serverValue, value); - } + warnForPropDifference(propKey, serverValue, value); continue; } case 'autoFocus': { extraAttributes.delete('autofocus'); const serverValue = (domElement: any).autofocus; - if (value !== serverValue) { - warnForPropDifference(propKey, serverValue, value); - } + warnForPropDifference(propKey, serverValue, value); continue; } case 'contentEditable': @@ -1612,9 +1597,7 @@ function diffHydratedGenericElement( } } } - if (serverValue !== value) { - warnForPropDifference(propKey, serverValue, value); - } + warnForPropDifference(propKey, serverValue, value); continue; } case 'accentHeight': @@ -1978,7 +1961,7 @@ function diffHydratedGenericElement( serverValue = getValueForAttribute(domElement, propKey, value); } - if (value !== serverValue && !isMismatchDueToBadCasing) { + if (!isMismatchDueToBadCasing) { warnForPropDifference(propKey, serverValue, value); } } From 715194a1c12aedf79cddf565ac5415ec876ab723 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 2 Apr 2023 16:42:13 -0400 Subject: [PATCH 06/16] Lower case HTML attributes don't need to be special cased when emitting What matters is that we treat them as lower case when we read them. --- .../src/client/ReactDOMComponent.js | 122 ++++++++++-------- .../src/server/ReactDOMServerFormatConfig.js | 15 +-- 2 files changed, 71 insertions(+), 66 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index d828571615eed..9cc983890a840 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -368,26 +368,19 @@ function setProp( break; } case 'contentEditable': - case 'spellCheck': { - // Lower-case Booleanish String - // These are "enumerated" HTML attributes that accept "true" and "false". - // In React, we let users pass `true` and `false` even though technically - // these aren't boolean attributes (they are coerced to strings). - key = key.toLowerCase(); - // Fall-through to the case-sensitive cases - } - // eslint-disable-next-line no-fallthrough + case 'spellCheck': case 'draggable': case 'value': case 'autoReverse': case 'externalResourcesRequired': case 'focusable': case 'preserveAlpha': { - // Case-sensitive Booleanish String - // These are "enumerated" SVG attributes that accept "true" and "false". + // Booleanish String + // These are "enumerated" attributes that accept "true" and "false". // In React, we let users pass `true` and `false` even though technically // these aren't boolean attributes (they are coerced to strings). - // Since these are SVG attributes, their attribute names are case-sensitive. + // The SVG attributes are case-sensitive. Since the HTML attributes are + // insensitive they also work even though we canonically use lower case. if ( value != null && typeof value !== 'function' && @@ -1385,6 +1378,45 @@ function hydrateAttribute( warnForPropDifference(attributeName, serverValue, value); } +function hydrateBooleanishAttribute( + domElement: Element, + attributeName: string, + value: any, + extraAttributes: Set, +): void { + extraAttributes.delete(attributeName); + let serverValue = domElement.getAttribute(attributeName); + if (serverValue === null) { + // shouldRemoveAttribute + switch (typeof value) { + case 'function': + case 'symbol': // eslint-disable-line + serverValue = value; + } + serverValue = value === undefined ? undefined : null; + } else { + if (value == null) { + // We had an attribute but shouldn't have had one, so read it + // for the error message. + } else { + switch (typeof value) { + case 'function': + case 'symbol': // eslint-disable-line + break; + default: { + if (__DEV__) { + checkAttributeStringCoercion(value, attributeName); + } + if (serverValue === '' + (value: any)) { + serverValue = value; + } + } + } + } + } + warnForPropDifference(attributeName, serverValue, value); +} + function diffHydratedCustomComponent( domElement: Element, tag: string, @@ -1510,9 +1542,8 @@ function diffHydratedGenericElement( // Don't bother comparing. We're ignoring all these warnings. continue; } - let key = propKey; // Validate that the properties correspond to their expected values. - switch (key) { + switch (propKey) { case 'children': // Checked above already case 'suppressContentEditableWarning': case 'suppressHydrationWarning': @@ -1533,17 +1564,17 @@ function diffHydratedGenericElement( } continue; case 'style': - extraAttributes.delete(key); + extraAttributes.delete(propKey); diffHydratedStyles(domElement, value); continue; case 'multiple': { - extraAttributes.delete(key); + extraAttributes.delete(propKey); const serverValue = (domElement: any).multiple; warnForPropDifference(propKey, serverValue, value); continue; } case 'muted': { - extraAttributes.delete(key); + extraAttributes.delete(propKey); const serverValue = (domElement: any).muted; warnForPropDifference(propKey, serverValue, value); continue; @@ -1554,50 +1585,33 @@ function diffHydratedGenericElement( warnForPropDifference(propKey, serverValue, value); continue; } - case 'contentEditable': + case 'contentEditable': { + // Lower-case Booleanish String + hydrateBooleanishAttribute( + domElement, + 'contenteditable', + value, + extraAttributes, + ); + continue; + } case 'spellCheck': { // Lower-case Booleanish String - key = key.toLowerCase(); - // Fall-through to the case-sensitive cases + hydrateBooleanishAttribute( + domElement, + 'spellcheck', + value, + extraAttributes, + ); + continue; } - // eslint-disable-next-line no-fallthrough case 'draggable': case 'autoReverse': case 'externalResourcesRequired': case 'focusable': case 'preserveAlpha': { // Case-sensitive Booleanish String - extraAttributes.delete(key); - let serverValue = domElement.getAttribute(key); - if (serverValue === null) { - // shouldRemoveAttribute - switch (typeof value) { - case 'function': - case 'symbol': // eslint-disable-line - serverValue = value; - } - serverValue = value === undefined ? undefined : null; - } else { - if (value == null) { - // We had an attribute but shouldn't have had one, so read it - // for the error message. - } else { - switch (typeof value) { - case 'function': - case 'symbol': // eslint-disable-line - break; - default: { - if (__DEV__) { - checkAttributeStringCoercion(value, key); - } - if (serverValue === '' + (value: any)) { - serverValue = value; - } - } - } - } - } - warnForPropDifference(propKey, serverValue, value); + hydrateBooleanishAttribute(domElement, propKey, value, extraAttributes); continue; } case 'accentHeight': @@ -1944,7 +1958,7 @@ function diffHydratedGenericElement( ownNamespaceDev = getIntrinsicNamespace(tag); } if (ownNamespaceDev === HTML_NAMESPACE) { - extraAttributes.delete(key.toLowerCase()); + extraAttributes.delete(propKey.toLowerCase()); } else { const standardName = getPossibleStandardName(propKey); if (standardName !== null && standardName !== propKey) { @@ -1956,7 +1970,7 @@ function diffHydratedGenericElement( isMismatchDueToBadCasing = true; extraAttributes.delete(standardName); } - extraAttributes.delete(key); + extraAttributes.delete(propKey); } serverValue = getValueForAttribute(domElement, propKey, value); } diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index 5c32e7d4f9beb..79ab2446c54dd 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -665,26 +665,17 @@ function pushAttribute( return; } case 'contentEditable': - case 'spellCheck': { - // Lower-case Booleanish String - // These are "enumerated" HTML attributes that accept "true" and "false". - // In React, we let users pass `true` and `false` even though technically - // these aren't boolean attributes (they are coerced to strings). - name = name.toLowerCase(); - // Fall-through to the case-sensitive cases - } - // eslint-disable-next-line no-fallthrough + case 'spellCheck': case 'draggable': case 'value': case 'autoReverse': case 'externalResourcesRequired': case 'focusable': case 'preserveAlpha': { - // Case-sensitive Booleanish String - // These are "enumerated" SVG attributes that accept "true" and "false". + // Booleanish String + // These are "enumerated" attributes that accept "true" and "false". // In React, we let users pass `true` and `false` even though technically // these aren't boolean attributes (they are coerced to strings). - // Since these are SVG attributes, their attribute names are case-sensitive. if ( value != null && typeof value !== 'function' && From afba3131d39eed3e119bdb6ed4579244f0a07d05 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 2 Apr 2023 17:06:18 -0400 Subject: [PATCH 07/16] Inline special cases for case-sensitive cases where the React name doesn't line up --- .../src/client/ReactDOMComponent.js | 13 +++++++++++++ .../src/server/ReactDOMServerFormatConfig.js | 7 +++++++ .../react-dom-bindings/src/shared/DOMProperty.js | 14 -------------- 3 files changed, 20 insertions(+), 14 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 9cc983890a840..11cbc25166bbf 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -395,6 +395,13 @@ function setProp( } break; } + // HTML and SVG attributes, but the SVG attribute is case sensitive. + case 'tabIndex': + setValueForAttribute(domElement, 'tabindex', value); + break; + case 'crossOrigin': + setValueForAttribute(domElement, 'crossorigin', value); + break; // This is a list of all SVG attributes that need special casing. // Regular attributes that just accept strings. case 'accentHeight': @@ -1614,6 +1621,12 @@ function diffHydratedGenericElement( hydrateBooleanishAttribute(domElement, propKey, value, extraAttributes); continue; } + case 'tabIndex': + hydrateAttribute(domElement, 'tabindex', value, extraAttributes); + continue; + case 'crossOrigin': + hydrateAttribute(domElement, 'crossorigin', value, extraAttributes); + continue; case 'accentHeight': hydrateAttribute(domElement, 'accent-height', value, extraAttributes); continue; diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index 79ab2446c54dd..49f9158277f70 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -691,6 +691,13 @@ function pushAttribute( } return; } + // HTML and SVG attributes, but the SVG attribute is case sensitive. + case 'tabIndex': + pushStringAttribute(target, 'tabindex', value); + return; + case 'crossOrigin': + pushStringAttribute(target, 'crossorigin', value); + return; // This is a list of all SVG attributes that need special casing. // Regular attributes that just accept strings. case 'accentHeight': diff --git a/packages/react-dom-bindings/src/shared/DOMProperty.js b/packages/react-dom-bindings/src/shared/DOMProperty.js index 2a783c5ba6858..bcd02b94f0788 100644 --- a/packages/react-dom-bindings/src/shared/DOMProperty.js +++ b/packages/react-dom-bindings/src/shared/DOMProperty.js @@ -221,20 +221,6 @@ const capitalize = (token: string) => token[1].toUpperCase(); ); }); -// These attribute exists both in HTML and SVG. -// The attribute name is case-sensitive in SVG so we can't just use -// the React name like we do for attributes that exist only in HTML. -['tabIndex', 'crossOrigin'].forEach(attributeName => { - // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions - properties[attributeName] = new PropertyInfoRecord( - STRING, - attributeName.toLowerCase(), // attributeName - null, // attributeNamespace - false, // sanitizeURL - false, // removeEmptyString - ); -}); - // These attributes accept URLs. These must not allow javascript: URLS. // These will also need to accept Trusted Types object in the future. const xlinkHref = 'xlinkHref'; From d1084cfbc6d1034fa4f68d77afa05754c5aeb625 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 2 Apr 2023 18:08:02 -0400 Subject: [PATCH 08/16] Pass propKey separately from attribute name for error messages --- .../src/client/ReactDOMComponent.js | 493 +++++++++++++++--- 1 file changed, 432 insertions(+), 61 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 11cbc25166bbf..99c597367f305 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -1348,6 +1348,7 @@ function diffHydratedStyles(domElement: Element, value: mixed) { function hydrateAttribute( domElement: Element, + propKey: string, attributeName: string, value: any, extraAttributes: Set, @@ -1373,7 +1374,7 @@ function hydrateAttribute( break; default: { if (__DEV__) { - checkAttributeStringCoercion(value, attributeName); + checkAttributeStringCoercion(value, propKey); } if (serverValue === '' + value) { serverValue = value; @@ -1382,11 +1383,12 @@ function hydrateAttribute( } } } - warnForPropDifference(attributeName, serverValue, value); + warnForPropDifference(propKey, serverValue, value); } function hydrateBooleanishAttribute( domElement: Element, + propKey: string, attributeName: string, value: any, extraAttributes: Set, @@ -1421,7 +1423,7 @@ function hydrateBooleanishAttribute( } } } - warnForPropDifference(attributeName, serverValue, value); + warnForPropDifference(propKey, serverValue, value); } function diffHydratedCustomComponent( @@ -1596,6 +1598,7 @@ function diffHydratedGenericElement( // Lower-case Booleanish String hydrateBooleanishAttribute( domElement, + propKey, 'contenteditable', value, extraAttributes, @@ -1606,6 +1609,7 @@ function diffHydratedGenericElement( // Lower-case Booleanish String hydrateBooleanishAttribute( domElement, + propKey, 'spellcheck', value, extraAttributes, @@ -1618,44 +1622,100 @@ function diffHydratedGenericElement( case 'focusable': case 'preserveAlpha': { // Case-sensitive Booleanish String - hydrateBooleanishAttribute(domElement, propKey, value, extraAttributes); + hydrateBooleanishAttribute( + domElement, + propKey, + propKey, + value, + extraAttributes, + ); continue; } case 'tabIndex': - hydrateAttribute(domElement, 'tabindex', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'tabindex', + value, + extraAttributes, + ); continue; case 'crossOrigin': - hydrateAttribute(domElement, 'crossorigin', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'crossorigin', + value, + extraAttributes, + ); continue; case 'accentHeight': - hydrateAttribute(domElement, 'accent-height', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'accent-height', + value, + extraAttributes, + ); continue; case 'alignmentBaseline': hydrateAttribute( domElement, + propKey, 'alignment-baseline', value, extraAttributes, ); continue; case 'arabicForm': - hydrateAttribute(domElement, 'arabic-form', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'arabic-form', + value, + extraAttributes, + ); continue; case 'baselineShift': - hydrateAttribute(domElement, 'baseline-shift', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'baseline-shift', + value, + extraAttributes, + ); continue; case 'capHeight': - hydrateAttribute(domElement, 'cap-height', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'cap-height', + value, + extraAttributes, + ); continue; case 'clipPath': - hydrateAttribute(domElement, 'clip-path', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'clip-path', + value, + extraAttributes, + ); continue; case 'clipRule': - hydrateAttribute(domElement, 'clip-rule', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'clip-rule', + value, + extraAttributes, + ); continue; case 'colorInterpolation': hydrateAttribute( domElement, + propKey, 'color-interpolation', value, extraAttributes, @@ -1664,20 +1724,34 @@ function diffHydratedGenericElement( case 'colorInterpolationFilters': hydrateAttribute( domElement, + propKey, 'color-interpolation-filters', value, extraAttributes, ); continue; case 'colorProfile': - hydrateAttribute(domElement, 'color-profile', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'color-profile', + value, + extraAttributes, + ); continue; case 'colorRendering': - hydrateAttribute(domElement, 'color-rendering', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'color-rendering', + value, + extraAttributes, + ); continue; case 'dominantBaseline': hydrateAttribute( domElement, + propKey, 'dominant-baseline', value, extraAttributes, @@ -1686,55 +1760,124 @@ function diffHydratedGenericElement( case 'enableBackground': hydrateAttribute( domElement, + propKey, 'enable-background', value, extraAttributes, ); continue; case 'fillOpacity': - hydrateAttribute(domElement, 'fill-opacity', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'fill-opacity', + value, + extraAttributes, + ); continue; case 'fillRule': - hydrateAttribute(domElement, 'fill-rule', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'fill-rule', + value, + extraAttributes, + ); continue; case 'floodColor': - hydrateAttribute(domElement, 'flood-color', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'flood-color', + value, + extraAttributes, + ); continue; case 'floodOpacity': - hydrateAttribute(domElement, 'flood-opacity', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'flood-opacity', + value, + extraAttributes, + ); continue; case 'fontFamily': - hydrateAttribute(domElement, 'font-family', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'font-family', + value, + extraAttributes, + ); continue; case 'fontSize': - hydrateAttribute(domElement, 'font-size', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'font-size', + value, + extraAttributes, + ); continue; case 'fontSizeAdjust': hydrateAttribute( domElement, + propKey, 'font-size-adjust', value, extraAttributes, ); continue; case 'fontStretch': - hydrateAttribute(domElement, 'font-stretch', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'font-stretch', + value, + extraAttributes, + ); continue; case 'fontStyle': - hydrateAttribute(domElement, 'font-style', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'font-style', + value, + extraAttributes, + ); continue; case 'fontVariant': - hydrateAttribute(domElement, 'font-variant', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'font-variant', + value, + extraAttributes, + ); continue; case 'fontWeight': - hydrateAttribute(domElement, 'font-weight', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'font-weight', + value, + extraAttributes, + ); continue; case 'glyphName': - hydrateAttribute(domElement, 'glyph-name', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'glyph-name', + value, + extraAttributes, + ); continue; case 'glyphOrientationHorizontal': hydrateAttribute( domElement, + propKey, 'glyph-orientation-horizontal', value, extraAttributes, @@ -1743,38 +1886,88 @@ function diffHydratedGenericElement( case 'glyphOrientationVertical': hydrateAttribute( domElement, + propKey, 'glyph-orientation-vertical', value, extraAttributes, ); continue; case 'horizAdvX': - hydrateAttribute(domElement, 'horiz-adv-x', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'horiz-adv-x', + value, + extraAttributes, + ); continue; case 'horizOriginX': - hydrateAttribute(domElement, 'horiz-origin-x', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'horiz-origin-x', + value, + extraAttributes, + ); continue; case 'imageRendering': - hydrateAttribute(domElement, 'image-rendering', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'image-rendering', + value, + extraAttributes, + ); continue; case 'letterSpacing': - hydrateAttribute(domElement, 'letter-spacing', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'letter-spacing', + value, + extraAttributes, + ); continue; case 'lightingColor': - hydrateAttribute(domElement, 'lighting-color', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'lighting-color', + value, + extraAttributes, + ); continue; case 'markerEnd': - hydrateAttribute(domElement, 'marker-end', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'marker-end', + value, + extraAttributes, + ); continue; case 'markerMid': - hydrateAttribute(domElement, 'marker-mid', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'marker-mid', + value, + extraAttributes, + ); continue; case 'markerStart': - hydrateAttribute(domElement, 'marker-start', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'marker-start', + value, + extraAttributes, + ); continue; case 'overlinePosition': hydrateAttribute( domElement, + propKey, 'overline-position', value, extraAttributes, @@ -1783,40 +1976,79 @@ function diffHydratedGenericElement( case 'overlineThickness': hydrateAttribute( domElement, + propKey, 'overline-thickness', value, extraAttributes, ); continue; case 'paintOrder': - hydrateAttribute(domElement, 'paint-order', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'paint-order', + value, + extraAttributes, + ); continue; case 'panose-1': - hydrateAttribute(domElement, 'panose-1', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'panose-1', + value, + extraAttributes, + ); continue; case 'pointerEvents': - hydrateAttribute(domElement, 'pointer-events', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'pointer-events', + value, + extraAttributes, + ); continue; case 'renderingIntent': hydrateAttribute( domElement, + propKey, 'rendering-intent', value, extraAttributes, ); continue; case 'shapeRendering': - hydrateAttribute(domElement, 'shape-rendering', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'shape-rendering', + value, + extraAttributes, + ); continue; case 'stopColor': - hydrateAttribute(domElement, 'stop-color', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'stop-color', + value, + extraAttributes, + ); continue; case 'stopOpacity': - hydrateAttribute(domElement, 'stop-opacity', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'stop-opacity', + value, + extraAttributes, + ); continue; case 'strikethroughPosition': hydrateAttribute( domElement, + propKey, 'strikethrough-position', value, extraAttributes, @@ -1825,6 +2057,7 @@ function diffHydratedGenericElement( case 'strikethroughThickness': hydrateAttribute( domElement, + propKey, 'strikethrough-thickness', value, extraAttributes, @@ -1833,6 +2066,7 @@ function diffHydratedGenericElement( case 'strokeDasharray': hydrateAttribute( domElement, + propKey, 'stroke-dasharray', value, extraAttributes, @@ -1841,43 +2075,88 @@ function diffHydratedGenericElement( case 'strokeDashoffset': hydrateAttribute( domElement, + propKey, 'stroke-dashoffset', value, extraAttributes, ); continue; case 'strokeLinecap': - hydrateAttribute(domElement, 'stroke-linecap', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'stroke-linecap', + value, + extraAttributes, + ); continue; case 'strokeLinejoin': - hydrateAttribute(domElement, 'stroke-linejoin', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'stroke-linejoin', + value, + extraAttributes, + ); continue; case 'strokeMiterlimit': hydrateAttribute( domElement, + propKey, 'stroke-miterlimit', value, extraAttributes, ); continue; case 'strokeOpacity': - hydrateAttribute(domElement, 'stroke-opacity', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'stroke-opacity', + value, + extraAttributes, + ); continue; case 'strokeWidth': - hydrateAttribute(domElement, 'stroke-width', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'stroke-width', + value, + extraAttributes, + ); continue; case 'textAnchor': - hydrateAttribute(domElement, 'text-anchor', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'text-anchor', + value, + extraAttributes, + ); continue; case 'textDecoration': - hydrateAttribute(domElement, 'text-decoration', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'text-decoration', + value, + extraAttributes, + ); continue; case 'textRendering': - hydrateAttribute(domElement, 'text-rendering', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'text-rendering', + value, + extraAttributes, + ); continue; case 'transformOrigin': hydrateAttribute( domElement, + propKey, 'transform-origin', value, extraAttributes, @@ -1886,6 +2165,7 @@ function diffHydratedGenericElement( case 'underlinePosition': hydrateAttribute( domElement, + propKey, 'underline-position', value, extraAttributes, @@ -1894,55 +2174,146 @@ function diffHydratedGenericElement( case 'underlineThickness': hydrateAttribute( domElement, + propKey, 'underline-thickness', value, extraAttributes, ); continue; case 'unicodeBidi': - hydrateAttribute(domElement, 'unicode-bidi', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'unicode-bidi', + value, + extraAttributes, + ); continue; case 'unicodeRange': - hydrateAttribute(domElement, 'unicode-range', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'unicode-range', + value, + extraAttributes, + ); continue; case 'unitsPerEm': - hydrateAttribute(domElement, 'units-per-em', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'units-per-em', + value, + extraAttributes, + ); continue; case 'vAlphabetic': - hydrateAttribute(domElement, 'v-alphabetic', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'v-alphabetic', + value, + extraAttributes, + ); continue; case 'vHanging': - hydrateAttribute(domElement, 'v-hanging', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'v-hanging', + value, + extraAttributes, + ); continue; case 'vIdeographic': - hydrateAttribute(domElement, 'v-ideographic', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'v-ideographic', + value, + extraAttributes, + ); continue; case 'vMathematical': - hydrateAttribute(domElement, 'v-mathematical', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'v-mathematical', + value, + extraAttributes, + ); continue; case 'vectorEffect': - hydrateAttribute(domElement, 'vector-effect', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'vector-effect', + value, + extraAttributes, + ); continue; case 'vertAdvY': - hydrateAttribute(domElement, 'vert-adv-y', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'vert-adv-y', + value, + extraAttributes, + ); continue; case 'vertOriginX': - hydrateAttribute(domElement, 'vert-origin-x', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'vert-origin-x', + value, + extraAttributes, + ); continue; case 'vertOriginY': - hydrateAttribute(domElement, 'vert-origin-y', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'vert-origin-y', + value, + extraAttributes, + ); continue; case 'wordSpacing': - hydrateAttribute(domElement, 'word-spacing', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'word-spacing', + value, + extraAttributes, + ); continue; case 'writingMode': - hydrateAttribute(domElement, 'writing-mode', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'writing-mode', + value, + extraAttributes, + ); continue; case 'xmlnsXlink': - hydrateAttribute(domElement, 'xmlns:xlink', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'xmlns:xlink', + value, + extraAttributes, + ); continue; case 'xHeight': - hydrateAttribute(domElement, 'x-height', value, extraAttributes); + hydrateAttribute( + domElement, + propKey, + 'x-height', + value, + extraAttributes, + ); continue; default: { if ( From 740294ec32ad81c5163b49e6a56257ff741316b0 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 2 Apr 2023 18:25:58 -0400 Subject: [PATCH 09/16] More aliases --- .../src/client/ReactDOMComponent.js | 40 +++++++++++++++++++ .../src/server/ReactDOMServerFormatConfig.js | 14 +++++++ .../src/shared/DOMProperty.js | 18 --------- 3 files changed, 54 insertions(+), 18 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 99c597367f305..d21e7d7081577 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -395,6 +395,20 @@ function setProp( } break; } + // A few React string attributes have a different name. + // This is a mapping from React prop names to the attribute names. + case 'acceptCharset': + setValueForAttribute(domElement, 'accept-charset', value); + break; + case 'className': + setValueForAttribute(domElement, 'class', value); + break; + case 'htmlFor': + setValueForAttribute(domElement, 'for', value); + break; + case 'httpEquiv': + setValueForAttribute(domElement, 'http-equiv', value); + break; // HTML and SVG attributes, but the SVG attribute is case sensitive. case 'tabIndex': setValueForAttribute(domElement, 'tabindex', value); @@ -1631,6 +1645,32 @@ function diffHydratedGenericElement( ); continue; } + // A few React string attributes have a different name. + // This is a mapping from React prop names to the attribute names. + case 'acceptCharset': + hydrateAttribute( + domElement, + propKey, + 'accept-charset', + value, + extraAttributes, + ); + continue; + case 'className': + hydrateAttribute(domElement, propKey, 'class', value, extraAttributes); + continue; + case 'htmlFor': + hydrateAttribute(domElement, propKey, 'for', value, extraAttributes); + continue; + case 'httpEquiv': + hydrateAttribute( + domElement, + propKey, + 'http-equiv', + value, + extraAttributes, + ); + continue; case 'tabIndex': hydrateAttribute( domElement, diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index 49f9158277f70..492209c9e29b5 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -691,6 +691,20 @@ function pushAttribute( } return; } + // A few React string attributes have a different name. + // This is a mapping from React prop names to the attribute names. + case 'acceptCharset': + pushStringAttribute(target, 'accept-charset', value); + return; + case 'className': + pushStringAttribute(target, 'class', value); + return; + case 'htmlFor': + pushStringAttribute(target, 'for', value); + return; + case 'httpEquiv': + pushStringAttribute(target, 'http-equiv', value); + return; // HTML and SVG attributes, but the SVG attribute is case sensitive. case 'tabIndex': pushStringAttribute(target, 'tabindex', value); diff --git a/packages/react-dom-bindings/src/shared/DOMProperty.js b/packages/react-dom-bindings/src/shared/DOMProperty.js index bcd02b94f0788..a543c0abebb08 100644 --- a/packages/react-dom-bindings/src/shared/DOMProperty.js +++ b/packages/react-dom-bindings/src/shared/DOMProperty.js @@ -66,24 +66,6 @@ function PropertyInfoRecord( // name warnings. const properties: {[string]: $FlowFixMe} = {}; -// A few React string attributes have a different name. -// This is a mapping from React prop names to the attribute names. -[ - ['acceptCharset', 'accept-charset'], - ['className', 'class'], - ['htmlFor', 'for'], - ['httpEquiv', 'http-equiv'], -].forEach(([name, attributeName]) => { - // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions - properties[name] = new PropertyInfoRecord( - STRING, - attributeName, // attributeName - null, // attributeNamespace - false, // sanitizeURL - false, // removeEmptyString - ); -}); - // These are HTML boolean attributes. [ 'allowFullScreen', From b26affde9412c7afe266fc8cb3b0dd6e23ebcffe Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Sun, 2 Apr 2023 19:06:19 -0400 Subject: [PATCH 10/16] Inline sanitizeURL and removeEmptyString special cases --- .../src/client/DOMPropertyOperations.js | 70 +----- .../src/client/ReactDOMComponent.js | 199 +++++++++++++++++- .../src/server/ReactDOMServerFormatConfig.js | 104 ++++++--- .../src/shared/DOMProperty.js | 51 ----- ...ctDOMServerIntegrationUntrustedURL-test.js | 2 +- 5 files changed, 269 insertions(+), 157 deletions(-) diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js index d89f40037ef2f..619ae8813b379 100644 --- a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js @@ -15,11 +15,9 @@ import { } from '../shared/DOMProperty'; import isAttributeNameSafe from '../shared/isAttributeNameSafe'; -import sanitizeURL from '../shared/sanitizeURL'; import { enableTrustedTypesIntegration, enableCustomElementPropertySupport, - enableFilterEmptyStringAttributesDOM, } from 'shared/ReactFeatureFlags'; import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; import {getFiberCurrentPropsFromNode} from './ReactDOMComponentTree'; @@ -78,31 +76,6 @@ export function getValueForProperty( break; } } - if (enableFilterEmptyStringAttributesDOM) { - if (propertyInfo.removeEmptyString && expected === '') { - 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, - ); - } - } - return expected; - } - } return expected === undefined ? undefined : null; } @@ -165,14 +138,6 @@ export function getValueForProperty( if (__DEV__) { checkAttributeStringCoercion(expected, name); } - if (propertyInfo.sanitizeURL) { - // We have already verified this above. - // eslint-disable-next-line react-internal/safe-string-coercion - if (value === '' + (sanitizeURL(expected): any)) { - return expected; - } - return value; - } // We have already verified this above. // eslint-disable-next-line react-internal/safe-string-coercion if (value === '' + (expected: any)) { @@ -304,32 +269,6 @@ export function setValueForProperty( } } } - 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; - } - } switch (propertyInfo.type) { case BOOLEAN: @@ -380,18 +319,11 @@ export function setValueForProperty( // `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); - } + attributeValue = (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); - } } const attributeNamespace = propertyInfo.attributeNamespace; if (attributeNamespace) { diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index d21e7d7081577..ea2c9a6a336da 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -64,12 +64,15 @@ import possibleStandardNames from '../shared/possibleStandardNames'; import {validateProperties as validateARIAProperties} from '../shared/ReactDOMInvalidARIAHook'; import {validateProperties as validateInputProperties} from '../shared/ReactDOMNullInputValuePropHook'; import {validateProperties as validateUnknownProperties} from '../shared/ReactDOMUnknownPropertyHook'; +import sanitizeURL from '../shared/sanitizeURL'; import { enableCustomElementPropertySupport, enableClientRenderFallbackOnTextMismatch, enableHostSingletons, disableIEWorkarounds, + enableTrustedTypesIntegration, + enableFilterEmptyStringAttributesDOM, } from 'shared/ReactFeatureFlags'; import { mediaEventTypes, @@ -367,6 +370,84 @@ function setProp( // on server rendering (but we *do* want to emit it in SSR). break; } + // These attributes accept URLs. These must not allow javascript: URLS. + case 'src': + case 'href': + case 'action': + if (enableFilterEmptyStringAttributesDOM) { + if (value === '') { + if (__DEV__) { + if (key === '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.', + key, + key, + ); + } 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.', + key, + key, + ); + } + } + domElement.removeAttribute(key); + break; + } + } + // Fall through to the last case which shouldn't remove empty strings. + // eslint-disable-next-line no-fallthrough + case 'formAction': { + if ( + value == null || + typeof value === 'function' || + typeof value === 'symbol' || + typeof value === 'boolean' + ) { + domElement.removeAttribute(key); + break; + } + // `setAttribute` with objects becomes only `[object]` in IE8/9, + // ('' + value) makes it output the correct toString()-value. + if (__DEV__) { + checkAttributeStringCoercion(value, key); + } + const sanitizedValue = (sanitizeURL( + enableTrustedTypesIntegration ? value : '' + (value: any), + ): any); + domElement.setAttribute(key, sanitizedValue); + break; + } + case 'xlinkHref': { + if ( + value == null || + typeof value === 'function' || + typeof value === 'boolean' || + typeof value === 'symbol' + ) { + domElement.removeAttribute('xlink:href'); + break; + } + // `setAttribute` with objects becomes only `[object]` in IE8/9, + // ('' + value) makes it output the correct toString()-value. + if (__DEV__) { + checkAttributeStringCoercion(value, key); + } + const sanitizedValue = (sanitizeURL( + enableTrustedTypesIntegration ? value : '' + (value: any), + ): any); + domElement.setAttributeNS( + 'http://www.w3.org/1999/xlink', + 'xlink:href', + sanitizedValue, + ); + break; + } case 'contentEditable': case 'spellCheck': case 'draggable': @@ -1373,8 +1454,9 @@ function hydrateAttribute( // shouldRemoveAttribute switch (typeof value) { case 'function': - case 'symbol': // eslint-disable-line - serverValue = value; + case 'symbol': + case 'boolean': + return; } serverValue = value === undefined ? undefined : null; } else { @@ -1384,7 +1466,8 @@ function hydrateAttribute( } else { switch (typeof value) { case 'function': - case 'symbol': // eslint-disable-line + case 'symbol': + case 'boolean': break; default: { if (__DEV__) { @@ -1413,8 +1496,8 @@ function hydrateBooleanishAttribute( // shouldRemoveAttribute switch (typeof value) { case 'function': - case 'symbol': // eslint-disable-line - serverValue = value; + case 'symbol': + return; } serverValue = value === undefined ? undefined : null; } else { @@ -1424,7 +1507,7 @@ function hydrateBooleanishAttribute( } else { switch (typeof value) { case 'function': - case 'symbol': // eslint-disable-line + case 'symbol': break; default: { if (__DEV__) { @@ -1440,6 +1523,49 @@ function hydrateBooleanishAttribute( warnForPropDifference(propKey, serverValue, value); } +function hydrateSanitizedAttribute( + domElement: Element, + propKey: string, + attributeName: string, + value: any, + extraAttributes: Set, +): void { + extraAttributes.delete(attributeName); + let serverValue = domElement.getAttribute(attributeName); + if (serverValue === null) { + // shouldRemoveAttribute + switch (typeof value) { + case 'function': + case 'symbol': + case 'boolean': + return; + } + serverValue = value === undefined ? undefined : null; + } else { + if (value == null) { + // We had an attribute but shouldn't have had one, so read it + // for the error message. + } else { + switch (typeof value) { + case 'function': + case 'symbol': + case 'boolean': + break; + default: { + if (__DEV__) { + checkAttributeStringCoercion(value, propKey); + } + const sanitizedValue = sanitizeURL('' + value); + if (serverValue === sanitizedValue) { + serverValue = value; + } + } + } + } + } + warnForPropDifference(propKey, serverValue, value); +} + function diffHydratedCustomComponent( domElement: Element, tag: string, @@ -1608,6 +1734,67 @@ function diffHydratedGenericElement( warnForPropDifference(propKey, serverValue, value); continue; } + case 'src': + case 'href': + case 'action': + if (enableFilterEmptyStringAttributesDOM) { + if (value === '') { + if (__DEV__) { + if (propKey === '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.', + propKey, + propKey, + ); + } 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.', + propKey, + propKey, + ); + } + } + hydrateSanitizedAttribute( + domElement, + propKey, + propKey, + null, + extraAttributes, + ); + continue; + } + } + hydrateSanitizedAttribute( + domElement, + propKey, + propKey, + value, + extraAttributes, + ); + continue; + case 'formAction': + hydrateSanitizedAttribute( + domElement, + propKey, + 'formaction', + value, + extraAttributes, + ); + continue; + case 'xlinkHref': + hydrateSanitizedAttribute( + domElement, + propKey, + 'xlink:href', + value, + extraAttributes, + ); + continue; case 'contentEditable': { // Lower-case Booleanish String hydrateBooleanishAttribute( diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index 492209c9e29b5..f244e99dee41f 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -664,6 +664,80 @@ function pushAttribute( pushBooleanAttribute(target, name.toLowerCase(), value); return; } + case 'src': + case 'href': + case 'action': + if (enableFilterEmptyStringAttributesDOM) { + if (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, + ); + } + } + return; + } + } + // Fall through to the last case which shouldn't remove empty strings. + // eslint-disable-next-line no-fallthrough + case 'formAction': { + if ( + value == null || + typeof value === 'function' || + typeof value === 'symbol' || + typeof value === 'boolean' + ) { + return; + } + if (__DEV__) { + checkAttributeStringCoercion(value, name); + } + const sanitizedValue = sanitizeURL('' + value); + target.push( + attributeSeparator, + stringToChunk(name), + attributeAssign, + stringToChunk(escapeTextForBrowser(sanitizedValue)), + attributeEnd, + ); + return; + } + case 'xlinkHref': { + if ( + value == null || + typeof value === 'function' || + typeof value === 'symbol' || + typeof value === 'boolean' + ) { + return; + } + if (__DEV__) { + checkAttributeStringCoercion(value, name); + } + const sanitizedValue = sanitizeURL('' + value); + target.push( + attributeSeparator, + stringToChunk('xlink:href'), + attributeAssign, + stringToChunk(escapeTextForBrowser(sanitizedValue)), + attributeEnd, + ); + return; + } case 'contentEditable': case 'spellCheck': case 'draggable': @@ -960,31 +1034,6 @@ function pushAttribute( } } } - 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, - ); - } - } - return; - } - } const attributeName = propertyInfo.attributeName; const attributeNameChunk = stringToChunk(attributeName); // TODO: If it's known we can cache the chunk. @@ -1044,11 +1093,6 @@ function pushAttribute( if (__DEV__) { checkAttributeStringCoercion(value, attributeName); } - if (propertyInfo.sanitizeURL) { - // We've already checked above. - // eslint-disable-next-line react-internal/safe-string-coercion - value = sanitizeURL('' + (value: any)); - } target.push( attributeSeparator, attributeNameChunk, diff --git a/packages/react-dom-bindings/src/shared/DOMProperty.js b/packages/react-dom-bindings/src/shared/DOMProperty.js index a543c0abebb08..8bc4ecbfaa7e9 100644 --- a/packages/react-dom-bindings/src/shared/DOMProperty.js +++ b/packages/react-dom-bindings/src/shared/DOMProperty.js @@ -37,8 +37,6 @@ export type PropertyInfo = { +attributeName: string, +attributeNamespace: string | null, +type: PropertyType, - +sanitizeURL: boolean, - +removeEmptyString: boolean, }; export function getPropertyInfo(name: string): PropertyInfo | null { @@ -50,15 +48,11 @@ function PropertyInfoRecord( type: PropertyType, attributeName: string, attributeNamespace: string | null, - sanitizeURL: boolean, - removeEmptyString: boolean, ) { this.acceptsBooleans = type === BOOLEAN || type === OVERLOADED_BOOLEAN; this.attributeName = attributeName; this.attributeNamespace = attributeNamespace; this.type = type; - this.sanitizeURL = sanitizeURL; - this.removeEmptyString = removeEmptyString; } // When adding attributes to this list, be sure to also add them to @@ -97,8 +91,6 @@ const properties: {[string]: $FlowFixMe} = {}; BOOLEAN, name.toLowerCase(), // attributeName null, // attributeNamespace - false, // sanitizeURL - false, // removeEmptyString ); }); @@ -117,8 +109,6 @@ const properties: {[string]: $FlowFixMe} = {}; OVERLOADED_BOOLEAN, name, // attributeName null, // attributeNamespace - false, // sanitizeURL - false, // removeEmptyString ); }); @@ -138,8 +128,6 @@ const properties: {[string]: $FlowFixMe} = {}; POSITIVE_NUMERIC, name, // attributeName null, // attributeNamespace - false, // sanitizeURL - false, // removeEmptyString ); }); @@ -150,8 +138,6 @@ const properties: {[string]: $FlowFixMe} = {}; NUMERIC, name.toLowerCase(), // attributeName null, // attributeNamespace - false, // sanitizeURL - false, // removeEmptyString ); }); @@ -177,8 +163,6 @@ const capitalize = (token: string) => token[1].toUpperCase(); STRING, attributeName, 'http://www.w3.org/1999/xlink', - false, // sanitizeURL - false, // removeEmptyString ); }); @@ -198,40 +182,5 @@ const capitalize = (token: string) => token[1].toUpperCase(); STRING, attributeName, 'http://www.w3.org/XML/1998/namespace', - false, // sanitizeURL - false, // removeEmptyString - ); -}); - -// These attributes accept URLs. These must not allow javascript: URLS. -// These will also need to accept Trusted Types object in the future. -const xlinkHref = 'xlinkHref'; -// $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions -properties[xlinkHref] = new PropertyInfoRecord( - STRING, - 'xlink:href', - 'http://www.w3.org/1999/xlink', - true, // sanitizeURL - false, // removeEmptyString -); - -const formAction = 'formAction'; -// $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions -properties[formAction] = new PropertyInfoRecord( - STRING, - 'formaction', // attributeName - null, // attributeNamespace - true, // sanitizeURL - false, // removeEmptyString -); - -['src', 'href', 'action'].forEach(attributeName => { - // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions - properties[attributeName] = new PropertyInfoRecord( - STRING, - attributeName.toLowerCase(), // attributeName - null, // attributeNamespace - true, // sanitizeURL - true, // removeEmptyString ); }); diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.js index c7da08897ae37..5bdfba529641b 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationUntrustedURL-test.js @@ -339,7 +339,7 @@ describe('ReactDOMServerIntegration - Untrusted URLs - disableJavaScriptURLs', ( // The hydration validation calls it one extra time. // TODO: It would be good if we only called toString once for // consistency but the code structure makes that hard right now. - expectedToStringCalls = 5; + expectedToStringCalls = 4; } else if (__DEV__) { // Checking for string coercion problems results in double the // toString calls in DEV From ba3bbee17c57eb6d476c80fb10b26906a5b9416e Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 3 Apr 2023 01:09:11 -0400 Subject: [PATCH 11/16] Inline special cases for XML namespaces --- .../src/client/DOMPropertyOperations.js | 50 +++--- .../src/client/ReactDOMComponent.js | 163 +++++++++++++++++- .../src/server/ReactDOMServerFormatConfig.js | 38 ++-- .../src/shared/DOMProperty.js | 60 +------ 4 files changed, 215 insertions(+), 96 deletions(-) diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js index 619ae8813b379..95f631e90cd10 100644 --- a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js @@ -311,27 +311,6 @@ export function setValueForProperty( 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) { - attributeValue = (value: any); - } else { - // We have already verified this above. - // eslint-disable-next-line react-internal/safe-string-coercion - attributeValue = '' + (value: any); - } - const attributeNamespace = propertyInfo.attributeNamespace; - if (attributeNamespace) { - node.setAttributeNS(attributeNamespace, attributeName, attributeValue); - } else { - node.setAttribute(attributeName, attributeValue); - } - } } } @@ -371,6 +350,35 @@ export function setValueForAttribute( } } +export function setValueForNamespacedAttribute( + node: Element, + namespace: string, + name: string, + value: mixed, +) { + if (value === null) { + node.removeAttribute(name); + return; + } + switch (typeof value) { + case 'undefined': + case 'function': + case 'symbol': + case 'boolean': { + node.removeAttribute(name); + return; + } + } + if (__DEV__) { + checkAttributeStringCoercion(value, name); + } + node.setAttributeNS( + namespace, + name, + enableTrustedTypesIntegration ? (value: any) : '' + (value: any), + ); +} + export function setValueForPropertyOnCustomComponent( node: Element, name: string, diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index ea2c9a6a336da..cf95c1bb0264d 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -25,6 +25,7 @@ import { setValueForProperty, setValueForPropertyOnCustomComponent, setValueForAttribute, + setValueForNamespacedAttribute, } from './DOMPropertyOperations'; import { initWrapperState as ReactDOMInputInitWrapperState, @@ -266,6 +267,9 @@ export function trapClickOnNonInteractiveElement(node: HTMLElement) { node.onclick = noop; } +const xlinkNamespace = 'http://www.w3.org/1999/xlink'; +const xmlNamespace = 'http://www.w3.org/XML/1998/namespace'; + function setProp( domElement: Element, tag: string, @@ -441,11 +445,7 @@ function setProp( const sanitizedValue = (sanitizeURL( enableTrustedTypesIntegration ? value : '' + (value: any), ): any); - domElement.setAttributeNS( - 'http://www.w3.org/1999/xlink', - 'xlink:href', - sanitizedValue, - ); + domElement.setAttributeNS(xlinkNamespace, 'xlink:href', sanitizedValue); break; } case 'contentEditable': @@ -721,6 +721,78 @@ function setProp( case 'xHeight': setValueForAttribute(domElement, 'x-height', value); break; + case 'xlinkActuate': + setValueForNamespacedAttribute( + domElement, + xlinkNamespace, + 'xlink:actuate', + value, + ); + break; + case 'xlinkArcrole': + setValueForNamespacedAttribute( + domElement, + xlinkNamespace, + 'xlink:arcrole', + value, + ); + break; + case 'xlinkRole': + setValueForNamespacedAttribute( + domElement, + xlinkNamespace, + 'xlink:role', + value, + ); + break; + case 'xlinkShow': + setValueForNamespacedAttribute( + domElement, + xlinkNamespace, + 'xlink:show', + value, + ); + break; + case 'xlinkTitle': + setValueForNamespacedAttribute( + domElement, + xlinkNamespace, + 'xlink:title', + value, + ); + break; + case 'xlinkType': + setValueForNamespacedAttribute( + domElement, + xlinkNamespace, + 'xlink:type', + value, + ); + break; + case 'xmlBase': + setValueForNamespacedAttribute( + domElement, + xmlNamespace, + 'xml:base', + value, + ); + break; + case 'xmlLang': + setValueForNamespacedAttribute( + domElement, + xmlNamespace, + 'xml:lang', + value, + ); + break; + case 'xmlSpace': + setValueForNamespacedAttribute( + domElement, + xmlNamespace, + 'xml:space', + value, + ); + break; // Properties that should not be allowed on custom elements. case 'innerText': case 'textContent': @@ -2542,6 +2614,87 @@ function diffHydratedGenericElement( extraAttributes, ); continue; + case 'xlinkActuate': + hydrateAttribute( + domElement, + propKey, + 'xlink:actuate', + value, + extraAttributes, + ); + continue; + case 'xlinkArcrole': + hydrateAttribute( + domElement, + propKey, + 'xlink:arcrole', + value, + extraAttributes, + ); + continue; + case 'xlinkRole': + hydrateAttribute( + domElement, + propKey, + 'xlink:role', + value, + extraAttributes, + ); + continue; + case 'xlinkShow': + hydrateAttribute( + domElement, + propKey, + 'xlink:show', + value, + extraAttributes, + ); + continue; + case 'xlinkTitle': + hydrateAttribute( + domElement, + propKey, + 'xlink:title', + value, + extraAttributes, + ); + continue; + case 'xlinkType': + hydrateAttribute( + domElement, + propKey, + 'xlink:type', + value, + extraAttributes, + ); + continue; + case 'xmlBase': + hydrateAttribute( + domElement, + propKey, + 'xml:base', + value, + extraAttributes, + ); + continue; + case 'xmlLang': + hydrateAttribute( + domElement, + propKey, + 'xml:lang', + value, + extraAttributes, + ); + continue; + case 'xmlSpace': + hydrateAttribute( + domElement, + propKey, + 'xml:space', + value, + extraAttributes, + ); + 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 f244e99dee41f..efb4fd97c4f95 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -1010,6 +1010,33 @@ function pushAttribute( case 'xHeight': pushStringAttribute(target, 'x-height', value); return; + case 'xlinkActuate': + pushStringAttribute(target, 'xlink:actuate', value); + break; + case 'xlinkArcrole': + pushStringAttribute(target, 'xlink:arcrole', value); + break; + case 'xlinkRole': + pushStringAttribute(target, 'xlink:role', value); + break; + case 'xlinkShow': + pushStringAttribute(target, 'xlink:show', value); + break; + case 'xlinkTitle': + pushStringAttribute(target, 'xlink:title', value); + break; + case 'xlinkType': + pushStringAttribute(target, 'xlink:type', value); + break; + case 'xmlBase': + pushStringAttribute(target, 'xml:base', value); + break; + case 'xmlLang': + pushStringAttribute(target, 'xml:lang', value); + break; + case 'xmlSpace': + pushStringAttribute(target, 'xml:space', value); + break; } if ( // shouldIgnoreAttribute @@ -1089,17 +1116,6 @@ function pushAttribute( ); } break; - default: - if (__DEV__) { - checkAttributeStringCoercion(value, attributeName); - } - target.push( - attributeSeparator, - attributeNameChunk, - attributeAssign, - stringToChunk(escapeTextForBrowser(value)), - attributeEnd, - ); } } else if (isAttributeNameSafe(name)) { // shouldRemoveAttribute diff --git a/packages/react-dom-bindings/src/shared/DOMProperty.js b/packages/react-dom-bindings/src/shared/DOMProperty.js index 8bc4ecbfaa7e9..433363cf398ae 100644 --- a/packages/react-dom-bindings/src/shared/DOMProperty.js +++ b/packages/react-dom-bindings/src/shared/DOMProperty.js @@ -9,10 +9,6 @@ type PropertyType = 0 | 1 | 2 | 3 | 4 | 5 | 6; -// A simple string attribute. -// Attributes that aren't in the filter are presumed to have this type. -export const STRING = 1; - // A real boolean attribute. // When true, it should be present (set either to an empty string or its name). // When false, it should be omitted. @@ -35,7 +31,6 @@ export const POSITIVE_NUMERIC = 6; export type PropertyInfo = { +acceptsBooleans: boolean, +attributeName: string, - +attributeNamespace: string | null, +type: PropertyType, }; @@ -44,14 +39,9 @@ export function getPropertyInfo(name: string): PropertyInfo | null { } // $FlowFixMe[missing-this-annot] -function PropertyInfoRecord( - type: PropertyType, - attributeName: string, - attributeNamespace: string | null, -) { +function PropertyInfoRecord(type: PropertyType, attributeName: string) { this.acceptsBooleans = type === BOOLEAN || type === OVERLOADED_BOOLEAN; this.attributeName = attributeName; - this.attributeNamespace = attributeNamespace; this.type = type; } @@ -90,7 +80,6 @@ const properties: {[string]: $FlowFixMe} = {}; properties[name] = new PropertyInfoRecord( BOOLEAN, name.toLowerCase(), // attributeName - null, // attributeNamespace ); }); @@ -108,7 +97,6 @@ const properties: {[string]: $FlowFixMe} = {}; properties[name] = new PropertyInfoRecord( OVERLOADED_BOOLEAN, name, // attributeName - null, // attributeNamespace ); }); @@ -127,7 +115,6 @@ const properties: {[string]: $FlowFixMe} = {}; properties[name] = new PropertyInfoRecord( POSITIVE_NUMERIC, name, // attributeName - null, // attributeNamespace ); }); @@ -137,50 +124,5 @@ const properties: {[string]: $FlowFixMe} = {}; properties[name] = new PropertyInfoRecord( NUMERIC, name.toLowerCase(), // attributeName - null, // attributeNamespace - ); -}); - -const CAMELIZE = /[\-\:]([a-z])/g; -const capitalize = (token: string) => token[1].toUpperCase(); - -// String SVG attributes with the xlink namespace. -[ - 'xlink:actuate', - 'xlink:arcrole', - 'xlink:role', - 'xlink:show', - 'xlink:title', - 'xlink:type', - - // 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(attributeName => { - const name = attributeName.replace(CAMELIZE, capitalize); - // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions - properties[name] = new PropertyInfoRecord( - STRING, - attributeName, - 'http://www.w3.org/1999/xlink', - ); -}); - -// String SVG attributes with the xml namespace. -[ - 'xml:base', - 'xml:lang', - 'xml:space', - - // 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(attributeName => { - const name = attributeName.replace(CAMELIZE, capitalize); - // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions - properties[name] = new PropertyInfoRecord( - STRING, - attributeName, - 'http://www.w3.org/XML/1998/namespace', ); }); From e09972857b3e6717a91340c45487e5035dada8f8 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 3 Apr 2023 01:19:27 -0400 Subject: [PATCH 12/16] Inline plain booleans --- .../src/client/DOMPropertyOperations.js | 24 ---- .../src/client/ReactDOMComponent.js | 119 ++++++++++++++++-- .../src/server/ReactDOMServerFormatConfig.js | 41 ++++-- .../src/shared/DOMProperty.js | 40 +----- .../src/shared/ReactDOMUnknownPropertyHook.js | 65 +++++++--- 5 files changed, 190 insertions(+), 99 deletions(-) diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js index 95f631e90cd10..8baeb0fbfdebc 100644 --- a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js @@ -8,7 +8,6 @@ */ import { - BOOLEAN, OVERLOADED_BOOLEAN, NUMERIC, POSITIVE_NUMERIC, @@ -51,12 +50,6 @@ export function getValueForProperty( } } switch (propertyInfo.type) { - case BOOLEAN: { - if (!expected) { - return expected; - } - break; - } case OVERLOADED_BOOLEAN: { if (expected === false) { return expected; @@ -98,15 +91,6 @@ export function getValueForProperty( return value; } switch (propertyInfo.type) { - case BOOLEAN: { - if (expected) { - // If this was a boolean, it doesn't matter what the value is - // the fact that we have it is the same as the expected. - // As long as it's positive. - return expected; - } - return value; - } case OVERLOADED_BOOLEAN: { if (value === '') { return true; @@ -271,14 +255,6 @@ export function setValueForProperty( } 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, ''); diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index cf95c1bb0264d..a8c52ae8fbc1d 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -476,6 +476,36 @@ function setProp( } break; } + // Boolean + case 'allowFullScreen': + case 'async': + case 'autoPlay': + case 'controls': + case 'default': + case 'defer': + case 'disabled': + case 'disablePictureInPicture': + case 'disableRemotePlayback': + case 'formNoValidate': + case 'hidden': + case 'loop': + case 'noModule': + case 'noValidate': + case 'open': + case 'playsInline': + case 'readOnly': + case 'required': + case 'reversed': + case 'scoped': + case 'seamless': + case 'itemScope': { + if (value && typeof value !== 'function' && typeof value !== 'symbol') { + domElement.setAttribute(key, ''); + } else { + domElement.removeAttribute(key); + } + break; + } // A few React string attributes have a different name. // This is a mapping from React prop names to the attribute names. case 'acceptCharset': @@ -1521,16 +1551,15 @@ function hydrateAttribute( extraAttributes: Set, ): void { extraAttributes.delete(attributeName); - let serverValue = domElement.getAttribute(attributeName); + const serverValue = domElement.getAttribute(attributeName); if (serverValue === null) { - // shouldRemoveAttribute switch (typeof value) { + case 'undefined': case 'function': case 'symbol': case 'boolean': return; } - serverValue = value === undefined ? undefined : null; } else { if (value == null) { // We had an attribute but shouldn't have had one, so read it @@ -1546,7 +1575,7 @@ function hydrateAttribute( checkAttributeStringCoercion(value, propKey); } if (serverValue === '' + value) { - serverValue = value; + return; } } } @@ -1555,6 +1584,42 @@ function hydrateAttribute( warnForPropDifference(propKey, serverValue, value); } +function hydrateBooleanAttribute( + domElement: Element, + propKey: string, + attributeName: string, + value: any, + extraAttributes: Set, +): void { + extraAttributes.delete(attributeName); + const serverValue = domElement.getAttribute(attributeName); + if (serverValue === null) { + switch (typeof value) { + case 'function': + case 'symbol': + return; + } + if (!value) { + return; + } + } else { + switch (typeof value) { + case 'function': + case 'symbol': + break; + default: { + if (value) { + // If this was a boolean, it doesn't matter what the value is + // the fact that we have it is the same as the expected. + // As long as it's positive. + return; + } + } + } + } + warnForPropDifference(propKey, serverValue, value); +} + function hydrateBooleanishAttribute( domElement: Element, propKey: string, @@ -1563,15 +1628,14 @@ function hydrateBooleanishAttribute( extraAttributes: Set, ): void { extraAttributes.delete(attributeName); - let serverValue = domElement.getAttribute(attributeName); + const serverValue = domElement.getAttribute(attributeName); if (serverValue === null) { - // shouldRemoveAttribute switch (typeof value) { + case 'undefined': case 'function': case 'symbol': return; } - serverValue = value === undefined ? undefined : null; } else { if (value == null) { // We had an attribute but shouldn't have had one, so read it @@ -1586,7 +1650,7 @@ function hydrateBooleanishAttribute( checkAttributeStringCoercion(value, attributeName); } if (serverValue === '' + (value: any)) { - serverValue = value; + return; } } } @@ -1603,16 +1667,15 @@ function hydrateSanitizedAttribute( extraAttributes: Set, ): void { extraAttributes.delete(attributeName); - let serverValue = domElement.getAttribute(attributeName); + const serverValue = domElement.getAttribute(attributeName); if (serverValue === null) { - // shouldRemoveAttribute switch (typeof value) { + case 'undefined': case 'function': case 'symbol': case 'boolean': return; } - serverValue = value === undefined ? undefined : null; } else { if (value == null) { // We had an attribute but shouldn't have had one, so read it @@ -1629,7 +1692,7 @@ function hydrateSanitizedAttribute( } const sanitizedValue = sanitizeURL('' + value); if (serverValue === sanitizedValue) { - serverValue = value; + return; } } } @@ -1904,6 +1967,38 @@ function diffHydratedGenericElement( ); continue; } + case 'allowFullScreen': + case 'async': + case 'autoPlay': + case 'controls': + case 'default': + case 'defer': + case 'disabled': + case 'disablePictureInPicture': + case 'disableRemotePlayback': + case 'formNoValidate': + case 'hidden': + case 'loop': + case 'noModule': + case 'noValidate': + case 'open': + case 'playsInline': + case 'readOnly': + case 'required': + case 'reversed': + case 'scoped': + case 'seamless': + case 'itemScope': { + // Some of these need to be lower case to remove them from the extraAttributes list. + hydrateBooleanAttribute( + domElement, + propKey, + propKey.toLowerCase(), + value, + extraAttributes, + ); + continue; + } // A few React string attributes have a different name. // This is a mapping from React prop names to the attribute names. case 'acceptCharset': diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index efb4fd97c4f95..fcf3f4502eb67 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -41,7 +41,6 @@ import { import isAttributeNameSafe from '../shared/isAttributeNameSafe'; import { getPropertyInfo, - BOOLEAN, OVERLOADED_BOOLEAN, NUMERIC, POSITIVE_NUMERIC, @@ -765,6 +764,37 @@ function pushAttribute( } return; } + case 'allowFullScreen': + case 'async': + case 'autoPlay': + case 'controls': + case 'default': + case 'defer': + case 'disabled': + case 'disablePictureInPicture': + case 'disableRemotePlayback': + case 'formNoValidate': + case 'hidden': + case 'loop': + case 'noModule': + case 'noValidate': + case 'open': + case 'playsInline': + case 'readOnly': + case 'required': + case 'reversed': + case 'scoped': + case 'seamless': + case 'itemScope': { + if (value && typeof value !== 'function' && typeof value !== 'symbol') { + target.push( + attributeSeparator, + stringToChunk(name), + attributeEmptyString, + ); + } + return; + } // A few React string attributes have a different name. // This is a mapping from React prop names to the attribute names. case 'acceptCharset': @@ -1066,15 +1096,6 @@ function pushAttribute( const attributeNameChunk = stringToChunk(attributeName); // TODO: If it's known we can cache the chunk. switch (propertyInfo.type) { - case BOOLEAN: - if (value) { - target.push( - attributeSeparator, - attributeNameChunk, - attributeEmptyString, - ); - } - return; case OVERLOADED_BOOLEAN: if (value === true) { target.push( diff --git a/packages/react-dom-bindings/src/shared/DOMProperty.js b/packages/react-dom-bindings/src/shared/DOMProperty.js index 433363cf398ae..e8b364f747f4f 100644 --- a/packages/react-dom-bindings/src/shared/DOMProperty.js +++ b/packages/react-dom-bindings/src/shared/DOMProperty.js @@ -9,11 +9,6 @@ type PropertyType = 0 | 1 | 2 | 3 | 4 | 5 | 6; -// A real boolean attribute. -// When true, it should be present (set either to an empty string or its name). -// When false, it should be omitted. -export const BOOLEAN = 3; - // An attribute that can be used as a flag as well as with a value. // When true, it should be present (set either to an empty string or its name). // When false, it should be omitted. @@ -40,7 +35,7 @@ export function getPropertyInfo(name: string): PropertyInfo | null { // $FlowFixMe[missing-this-annot] function PropertyInfoRecord(type: PropertyType, attributeName: string) { - this.acceptsBooleans = type === BOOLEAN || type === OVERLOADED_BOOLEAN; + this.acceptsBooleans = type === OVERLOADED_BOOLEAN; this.attributeName = attributeName; this.type = type; } @@ -50,39 +45,6 @@ function PropertyInfoRecord(type: PropertyType, attributeName: string) { // name warnings. const properties: {[string]: $FlowFixMe} = {}; -// These are HTML boolean attributes. -[ - 'allowFullScreen', - 'async', - 'autoPlay', - 'controls', - 'default', - 'defer', - 'disabled', - 'disablePictureInPicture', - 'disableRemotePlayback', - 'formNoValidate', - 'hidden', - 'loop', - 'noModule', - 'noValidate', - 'open', - 'playsInline', - 'readOnly', - 'required', - 'reversed', - 'scoped', - 'seamless', - // Microdata - 'itemScope', -].forEach(name => { - // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions - properties[name] = new PropertyInfoRecord( - BOOLEAN, - name.toLowerCase(), // attributeName - ); -}); - // These are HTML attributes that are "overloaded booleans": they behave like // booleans, but can also accept a string value. [ diff --git a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js index 4722b4ee1b97a..d7d2eb3b77fdb 100644 --- a/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js +++ b/packages/react-dom-bindings/src/shared/ReactDOMUnknownPropertyHook.js @@ -5,7 +5,6 @@ * LICENSE file in the root directory of this source tree. */ -import {BOOLEAN, getPropertyInfo} from './DOMProperty'; import {ATTRIBUTE_NAME_CHAR} from './isAttributeNameSafe'; import isCustomElement from './isCustomElement'; import possibleStandardNames from './possibleStandardNames'; @@ -131,8 +130,6 @@ function validateProperty(tagName, name, value, eventRegistry) { return true; } - const propertyInfo = getPropertyInfo(name); - // Known attributes should match the casing specified in the property config. if (possibleStandardNames.hasOwnProperty(lowerCasedName)) { const standardName = possibleStandardNames[lowerCasedName]; @@ -196,17 +193,37 @@ function validateProperty(tagName, name, value, eventRegistry) { case 'autoReverse': case 'externalResourcesRequired': case 'focusable': - case 'preserveAlpha': { + case 'preserveAlpha': + case 'allowFullScreen': + case 'async': + case 'autoPlay': + case 'controls': + case 'default': + case 'defer': + case 'disabled': + case 'disablePictureInPicture': + case 'disableRemotePlayback': + case 'formNoValidate': + case 'hidden': + case 'loop': + case 'noModule': + case 'noValidate': + case 'open': + case 'playsInline': + case 'readOnly': + case 'required': + case 'reversed': + case 'scoped': + case 'seamless': + case 'itemScope': + case 'capture': + case 'download': { // 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) { + const prefix = name.toLowerCase().slice(0, 5); + if (prefix === 'data-' || prefix === 'aria-') { return true; } if (value) { @@ -253,13 +270,33 @@ function validateProperty(tagName, name, value, eventRegistry) { case 'checked': case 'selected': case 'multiple': - case 'muted': { + case 'muted': + case 'allowFullScreen': + case 'async': + case 'autoPlay': + case 'controls': + case 'default': + case 'defer': + case 'disabled': + case 'disablePictureInPicture': + case 'disableRemotePlayback': + case 'formNoValidate': + case 'hidden': + case 'loop': + case 'noModule': + case 'noValidate': + case 'open': + case 'playsInline': + case 'readOnly': + case 'required': + case 'reversed': + case 'scoped': + case 'seamless': + case 'itemScope': { break; } default: { - if (propertyInfo === null || propertyInfo.type !== BOOLEAN) { - return true; - } + return true; } } console.error( From cb4fa91779d1d970bbe42cf3f2966b38a6c87a93 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 3 Apr 2023 22:11:03 -0400 Subject: [PATCH 13/16] Inline overloaded booleans --- .../src/client/DOMPropertyOperations.js | 45 +--------- .../src/client/ReactDOMComponent.js | 83 +++++++++++++++++++ .../src/server/ReactDOMServerFormatConfig.js | 47 ++++++----- .../src/shared/DOMProperty.js | 25 ------ 4 files changed, 111 insertions(+), 89 deletions(-) diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js index 8baeb0fbfdebc..92c593f196030 100644 --- a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js @@ -7,11 +7,7 @@ * @flow */ -import { - OVERLOADED_BOOLEAN, - NUMERIC, - POSITIVE_NUMERIC, -} from '../shared/DOMProperty'; +import {NUMERIC, POSITIVE_NUMERIC} from '../shared/DOMProperty'; import isAttributeNameSafe from '../shared/isAttributeNameSafe'; import { @@ -44,18 +40,10 @@ export function getValueForProperty( case 'symbol': // eslint-disable-line return expected; case 'boolean': { - if (!propertyInfo.acceptsBooleans) { - return expected; - } + return expected; } } switch (propertyInfo.type) { - case OVERLOADED_BOOLEAN: { - if (expected === false) { - return expected; - } - break; - } case NUMERIC: { if (isNaN(expected)) { return expected; @@ -91,17 +79,6 @@ export function getValueForProperty( return value; } switch (propertyInfo.type) { - case OVERLOADED_BOOLEAN: { - if (value === '') { - return true; - } - if (expected === false) { - // We had an attribute but shouldn't have had one, so read it - // for the error message. - return value; - } - break; - } case NUMERIC: { if (isNaN(expected)) { // We had an attribute but shouldn't have had one, so read it @@ -247,26 +224,12 @@ export function setValueForProperty( node.removeAttribute(attributeName); return; case 'boolean': { - if (!propertyInfo.acceptsBooleans) { - node.removeAttribute(attributeName); - return; - } + node.removeAttribute(attributeName); + return; } } switch (propertyInfo.type) { - 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)); - } - return; case NUMERIC: if (!isNaN(value)) { if (__DEV__) { diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index a8c52ae8fbc1d..f249d780419dc 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -506,6 +506,30 @@ function setProp( } break; } + // Overloaded Boolean + case 'capture': + case 'download': { + // An attribute that can be used as a flag as well as with a value. + // When true, it should be present (set either to an empty string or its name). + // When false, it should be omitted. + // For any other value, should be present with that value. + if (value === true) { + domElement.setAttribute(key, ''); + } else if ( + value !== false && + value != null && + typeof value !== 'function' && + typeof value !== 'symbol' + ) { + if (__DEV__) { + checkAttributeStringCoercion(value, key); + } + domElement.setAttribute(key, (value: any)); + } else { + domElement.removeAttribute(key); + } + break; + } // A few React string attributes have a different name. // This is a mapping from React prop names to the attribute names. case 'acceptCharset': @@ -1620,6 +1644,54 @@ function hydrateBooleanAttribute( warnForPropDifference(propKey, serverValue, value); } +function hydrateOverloadedBooleanAttribute( + domElement: Element, + propKey: string, + attributeName: string, + value: any, + extraAttributes: Set, +): void { + extraAttributes.delete(attributeName); + const serverValue = domElement.getAttribute(attributeName); + if (serverValue === null) { + switch (typeof value) { + case 'undefined': + case 'function': + case 'symbol': + return; + default: + if (value === false) { + return; + } + } + } else { + if (value == null) { + // We had an attribute but shouldn't have had one, so read it + // for the error message. + } else { + switch (typeof value) { + case 'function': + case 'symbol': + break; + case 'boolean': + if (value === true && serverValue === '') { + return; + } + break; + default: { + if (__DEV__) { + checkAttributeStringCoercion(value, propKey); + } + if (serverValue === '' + value) { + return; + } + } + } + } + } + warnForPropDifference(propKey, serverValue, value); +} + function hydrateBooleanishAttribute( domElement: Element, propKey: string, @@ -1999,6 +2071,17 @@ function diffHydratedGenericElement( ); continue; } + case 'capture': + case 'download': { + hydrateOverloadedBooleanAttribute( + domElement, + propKey, + propKey, + value, + extraAttributes, + ); + continue; + } // A few React string attributes have a different name. // This is a mapping from React prop names to the attribute names. case 'acceptCharset': diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index fcf3f4502eb67..19047c97f48b6 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -41,7 +41,6 @@ import { import isAttributeNameSafe from '../shared/isAttributeNameSafe'; import { getPropertyInfo, - OVERLOADED_BOOLEAN, NUMERIC, POSITIVE_NUMERIC, } from '../shared/DOMProperty'; @@ -786,6 +785,7 @@ function pushAttribute( case 'scoped': case 'seamless': case 'itemScope': { + // Boolean if (value && typeof value !== 'function' && typeof value !== 'symbol') { target.push( attributeSeparator, @@ -795,6 +795,28 @@ function pushAttribute( } return; } + case 'capture': + case 'download': { + // Overloaded Boolean + if (value === true) { + target.push( + attributeSeparator, + stringToChunk(name), + attributeEmptyString, + ); + } else if (value === false) { + // Ignored + } else if (typeof value !== 'function' && typeof value !== 'symbol') { + target.push( + attributeSeparator, + stringToChunk(name), + attributeAssign, + stringToChunk(escapeTextForBrowser(value)), + attributeEnd, + ); + } + break; + } // A few React string attributes have a different name. // This is a mapping from React prop names to the attribute names. case 'acceptCharset': @@ -1086,9 +1108,7 @@ function pushAttribute( case 'symbol': // eslint-disable-line return; case 'boolean': { - if (!propertyInfo.acceptsBooleans) { - return; - } + return; } } @@ -1096,25 +1116,6 @@ function pushAttribute( const attributeNameChunk = stringToChunk(attributeName); // TODO: If it's known we can cache the chunk. switch (propertyInfo.type) { - case OVERLOADED_BOOLEAN: - if (value === true) { - target.push( - attributeSeparator, - attributeNameChunk, - attributeEmptyString, - ); - } else if (value === false) { - // Ignored - } else { - target.push( - attributeSeparator, - attributeNameChunk, - attributeAssign, - stringToChunk(escapeTextForBrowser(value)), - attributeEnd, - ); - } - return; case NUMERIC: if (!isNaN(value)) { target.push( diff --git a/packages/react-dom-bindings/src/shared/DOMProperty.js b/packages/react-dom-bindings/src/shared/DOMProperty.js index e8b364f747f4f..04044b0a11726 100644 --- a/packages/react-dom-bindings/src/shared/DOMProperty.js +++ b/packages/react-dom-bindings/src/shared/DOMProperty.js @@ -9,12 +9,6 @@ type PropertyType = 0 | 1 | 2 | 3 | 4 | 5 | 6; -// An attribute that can be used as a flag as well as with a value. -// When true, it should be present (set either to an empty string or its name). -// When false, it should be omitted. -// For any other value, should be present with that value. -export const OVERLOADED_BOOLEAN = 4; - // An attribute that must be numeric or parse as a numeric. // When falsy, it should be removed. export const NUMERIC = 5; @@ -24,7 +18,6 @@ export const NUMERIC = 5; export const POSITIVE_NUMERIC = 6; export type PropertyInfo = { - +acceptsBooleans: boolean, +attributeName: string, +type: PropertyType, }; @@ -35,7 +28,6 @@ export function getPropertyInfo(name: string): PropertyInfo | null { // $FlowFixMe[missing-this-annot] function PropertyInfoRecord(type: PropertyType, attributeName: string) { - this.acceptsBooleans = type === OVERLOADED_BOOLEAN; this.attributeName = attributeName; this.type = type; } @@ -45,23 +37,6 @@ function PropertyInfoRecord(type: PropertyType, attributeName: string) { // name warnings. const properties: {[string]: $FlowFixMe} = {}; -// These are HTML attributes that are "overloaded booleans": they behave like -// booleans, but can also accept a string value. -[ - 'capture', - 'download', - - // 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( - OVERLOADED_BOOLEAN, - name, // attributeName - ); -}); - // These are HTML attributes that must be positive numbers. [ 'cols', From 79f03f5a16e9204ac09bb413d8f272a629aee600 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 3 Apr 2023 22:20:23 -0400 Subject: [PATCH 14/16] Inline numeric properties --- .../src/client/ReactDOMComponent.js | 39 ++++++++++++++++ .../src/server/ReactDOMServerFormatConfig.js | 46 ++++++++++++++++--- 2 files changed, 79 insertions(+), 6 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index f249d780419dc..3bd9be2709631 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -530,6 +530,45 @@ function setProp( } break; } + case 'cols': + case 'rows': + case 'size': + case 'span': { + // These are HTML attributes that must be positive numbers. + if ( + value != null && + typeof value !== 'function' && + typeof value !== 'symbol' && + !isNaN(value) && + (value: any) >= 1 + ) { + if (__DEV__) { + checkAttributeStringCoercion(value, key); + } + domElement.setAttribute(key, (value: any)); + } else { + domElement.removeAttribute(key); + } + break; + } + case 'rowSpan': + case 'start': { + // These are HTML attributes that must be numbers. + if ( + value != null && + typeof value !== 'function' && + typeof value !== 'symbol' && + !isNaN(value) + ) { + if (__DEV__) { + checkAttributeStringCoercion(value, key); + } + domElement.setAttribute(key, (value: any)); + } else { + domElement.removeAttribute(key); + } + break; + } // A few React string attributes have a different name. // This is a mapping from React prop names to the attribute names. case 'acceptCharset': diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index 19047c97f48b6..fd41ae777ed58 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -716,7 +716,6 @@ function pushAttribute( } case 'xlinkHref': { if ( - value == null || typeof value === 'function' || typeof value === 'symbol' || typeof value === 'boolean' @@ -748,11 +747,7 @@ function pushAttribute( // These are "enumerated" attributes that accept "true" and "false". // In React, we let users pass `true` and `false` even though technically // these aren't boolean attributes (they are coerced to strings). - if ( - value != null && - typeof value !== 'function' && - typeof value !== 'symbol' - ) { + if (typeof value !== 'function' && typeof value !== 'symbol') { target.push( attributeSeparator, stringToChunk(name), @@ -817,6 +812,45 @@ function pushAttribute( } break; } + case 'cols': + case 'rows': + case 'size': + case 'span': { + // These are HTML attributes that must be positive numbers. + if ( + typeof value !== 'function' && + typeof value !== 'symbol' && + !isNaN(value) && + (value: any) >= 1 + ) { + target.push( + attributeSeparator, + stringToChunk(name), + attributeAssign, + stringToChunk(escapeTextForBrowser(value)), + attributeEnd, + ); + } + break; + } + case 'rowSpan': + case 'start': { + // These are HTML attributes that must be numbers. + if ( + typeof value !== 'function' && + typeof value !== 'symbol' && + !isNaN(value) + ) { + target.push( + attributeSeparator, + stringToChunk(name), + attributeAssign, + stringToChunk(escapeTextForBrowser(value)), + attributeEnd, + ); + } + break; + } // A few React string attributes have a different name. // This is a mapping from React prop names to the attribute names. case 'acceptCharset': From 37f3fe748edf8e15c13a50f22c967149e7876a8b Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Mon, 3 Apr 2023 22:46:19 -0400 Subject: [PATCH 15/16] Inline numeric properties This is the last of the DOMProperty configs. --- .../src/client/DOMPropertyOperations.js | 149 -------------- .../src/client/ReactDOMComponent.js | 190 ++++++++++++++---- .../src/server/ReactDOMServerFormatConfig.js | 103 +++------- .../src/shared/DOMProperty.js | 65 ------ 4 files changed, 181 insertions(+), 326 deletions(-) delete mode 100644 packages/react-dom-bindings/src/shared/DOMProperty.js diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js index 92c593f196030..4066055e0197f 100644 --- a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js @@ -7,8 +7,6 @@ * @flow */ -import {NUMERIC, POSITIVE_NUMERIC} from '../shared/DOMProperty'; - import isAttributeNameSafe from '../shared/isAttributeNameSafe'; import { enableTrustedTypesIntegration, @@ -17,97 +15,6 @@ import { import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; import {getFiberCurrentPropsFromNode} from './ReactDOMComponentTree'; -import type {PropertyInfo} from '../shared/DOMProperty'; - -/** - * Get the value for a property on a node. Only used in DEV for SSR validation. - * The "expected" argument is used as a hint of what the expected value is. - * Some properties have multiple equivalent values. - */ -export function getValueForProperty( - node: Element, - name: string, - expected: mixed, - propertyInfo: PropertyInfo, -): mixed { - if (__DEV__) { - const attributeName = propertyInfo.attributeName; - - if (!node.hasAttribute(attributeName)) { - // shouldRemoveAttribute - switch (typeof expected) { - case 'function': - case 'symbol': // eslint-disable-line - return expected; - case 'boolean': { - return expected; - } - } - switch (propertyInfo.type) { - case NUMERIC: { - if (isNaN(expected)) { - return expected; - } - break; - } - case POSITIVE_NUMERIC: { - if (isNaN(expected) || (expected: any) < 1) { - return expected; - } - break; - } - } - return expected === undefined ? undefined : null; - } - - // Even if this property uses a namespace we use getAttribute - // because we assume its namespaced name is the same as our config. - // To use getAttributeNS we need the local name which we don't have - // in our config atm. - const value = node.getAttribute(attributeName); - - if (expected == null) { - // We had an attribute but shouldn't have had one, so read it - // for the error message. - return value; - } - - // shouldRemoveAttribute - switch (typeof expected) { - case 'function': - case 'symbol': // eslint-disable-line - return value; - } - switch (propertyInfo.type) { - case NUMERIC: { - if (isNaN(expected)) { - // We had an attribute but shouldn't have had one, so read it - // for the error message. - return value; - } - break; - } - case POSITIVE_NUMERIC: { - if (isNaN(expected) || (expected: any) < 1) { - // We had an attribute but shouldn't have had one, so read it - // for the error message. - return value; - } - break; - } - } - if (__DEV__) { - checkAttributeStringCoercion(expected, name); - } - // We have already verified this above. - // eslint-disable-next-line react-internal/safe-string-coercion - if (value === '' + (expected: any)) { - return expected; - } - return value; - } -} - /** * Get the value for a attribute on a node. Only used in DEV for SSR validation. * The third argument is used as a hint of what the expected value is. Some @@ -197,62 +104,6 @@ export function getValueForAttributeOnCustomComponent( } } -/** - * Sets the value for a property on a node. - * - * @param {DOMElement} node - * @param {string} name - * @param {*} value - */ -export function setValueForProperty( - node: Element, - propertyInfo: PropertyInfo, - value: mixed, -) { - const attributeName = propertyInfo.attributeName; - - if (value === null) { - node.removeAttribute(attributeName); - return; - } - - // shouldRemoveAttribute - switch (typeof value) { - case 'undefined': - case 'function': - case 'symbol': // eslint-disable-line - node.removeAttribute(attributeName); - return; - case 'boolean': { - node.removeAttribute(attributeName); - return; - } - } - - switch (propertyInfo.type) { - case NUMERIC: - if (!isNaN(value)) { - if (__DEV__) { - checkAttributeStringCoercion(value, attributeName); - } - node.setAttribute(attributeName, (value: any)); - } else { - node.removeAttribute(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); - } - break; - } -} - export function setValueForAttribute( node: Element, name: string, diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 3bd9be2709631..e54df227cc858 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -21,8 +21,6 @@ import {checkAttributeStringCoercion} from 'shared/CheckStringCoercion'; import { getValueForAttribute, getValueForAttributeOnCustomComponent, - getValueForProperty, - setValueForProperty, setValueForPropertyOnCustomComponent, setValueForAttribute, setValueForNamespacedAttribute, @@ -59,7 +57,6 @@ import { validateShorthandPropertyCollisionInDev, } from './CSSPropertyOperations'; import {HTML_NAMESPACE, getIntrinsicNamespace} from './DOMNamespaces'; -import {getPropertyInfo} from '../shared/DOMProperty'; import isCustomElement from '../shared/isCustomElement'; import possibleStandardNames from '../shared/possibleStandardNames'; import {validateProperties as validateARIAProperties} from '../shared/ReactDOMInvalidARIAHook'; @@ -908,12 +905,7 @@ function setProp( warnForInvalidEventListener(key, value); } } else { - const propertyInfo = getPropertyInfo(key); - if (propertyInfo !== null) { - setValueForProperty(domElement, propertyInfo, value); - } else { - setValueForAttribute(domElement, key, value); - } + setValueForAttribute(domElement, key, value); } } } @@ -1770,6 +1762,106 @@ function hydrateBooleanishAttribute( warnForPropDifference(propKey, serverValue, value); } +function hydrateNumericAttribute( + domElement: Element, + propKey: string, + attributeName: string, + value: any, + extraAttributes: Set, +): void { + extraAttributes.delete(attributeName); + const serverValue = domElement.getAttribute(attributeName); + if (serverValue === null) { + switch (typeof value) { + case 'undefined': + case 'function': + case 'symbol': + case 'boolean': + return; + default: + if (isNaN(value)) { + return; + } + } + } else { + if (value == null) { + // We had an attribute but shouldn't have had one, so read it + // for the error message. + } else { + switch (typeof value) { + case 'function': + case 'symbol': + case 'boolean': + break; + default: { + if (isNaN(value)) { + // We had an attribute but shouldn't have had one, so read it + // for the error message. + break; + } + if (__DEV__) { + checkAttributeStringCoercion(value, propKey); + } + if (serverValue === '' + value) { + return; + } + } + } + } + } + warnForPropDifference(propKey, serverValue, value); +} + +function hydratePositiveNumericAttribute( + domElement: Element, + propKey: string, + attributeName: string, + value: any, + extraAttributes: Set, +): void { + extraAttributes.delete(attributeName); + const serverValue = domElement.getAttribute(attributeName); + if (serverValue === null) { + switch (typeof value) { + case 'undefined': + case 'function': + case 'symbol': + case 'boolean': + return; + default: + if (isNaN(value) || value < 1) { + return; + } + } + } else { + if (value == null) { + // We had an attribute but shouldn't have had one, so read it + // for the error message. + } else { + switch (typeof value) { + case 'function': + case 'symbol': + case 'boolean': + break; + default: { + if (isNaN(value) || value < 1) { + // We had an attribute but shouldn't have had one, so read it + // for the error message. + break; + } + if (__DEV__) { + checkAttributeStringCoercion(value, propKey); + } + if (serverValue === '' + value) { + return; + } + } + } + } + } + warnForPropDifference(propKey, serverValue, value); +} + function hydrateSanitizedAttribute( domElement: Element, propKey: string, @@ -2121,6 +2213,39 @@ function diffHydratedGenericElement( ); continue; } + case 'cols': + case 'rows': + case 'size': + case 'span': { + hydratePositiveNumericAttribute( + domElement, + propKey, + propKey, + value, + extraAttributes, + ); + continue; + } + case 'rowSpan': { + hydrateNumericAttribute( + domElement, + propKey, + 'rowspan', + value, + extraAttributes, + ); + continue; + } + case 'start': { + hydrateNumericAttribute( + domElement, + propKey, + propKey, + value, + extraAttributes, + ); + continue; + } // A few React string attributes have a different name. // This is a mapping from React prop names to the attribute names. case 'acceptCharset': @@ -2922,40 +3047,27 @@ function diffHydratedGenericElement( ) { continue; } - const propertyInfo = getPropertyInfo(propKey); let isMismatchDueToBadCasing = false; - let serverValue; - if (propertyInfo !== null) { - extraAttributes.delete(propertyInfo.attributeName); - serverValue = getValueForProperty( - domElement, - propKey, - value, - propertyInfo, - ); + let ownNamespaceDev = parentNamespaceDev; + if (ownNamespaceDev === HTML_NAMESPACE) { + ownNamespaceDev = getIntrinsicNamespace(tag); + } + if (ownNamespaceDev === HTML_NAMESPACE) { + extraAttributes.delete(propKey.toLowerCase()); } else { - let ownNamespaceDev = parentNamespaceDev; - if (ownNamespaceDev === HTML_NAMESPACE) { - ownNamespaceDev = getIntrinsicNamespace(tag); - } - if (ownNamespaceDev === HTML_NAMESPACE) { - extraAttributes.delete(propKey.toLowerCase()); - } else { - const standardName = getPossibleStandardName(propKey); - if (standardName !== null && standardName !== propKey) { - // If an SVG prop is supplied with bad casing, it will - // be successfully parsed from HTML, but will produce a mismatch - // (and would be incorrectly rendered on the client). - // However, we already warn about bad casing elsewhere. - // So we'll skip the misleading extra mismatch warning in this case. - isMismatchDueToBadCasing = true; - extraAttributes.delete(standardName); - } - extraAttributes.delete(propKey); + const standardName = getPossibleStandardName(propKey); + if (standardName !== null && standardName !== propKey) { + // If an SVG prop is supplied with bad casing, it will + // be successfully parsed from HTML, but will produce a mismatch + // (and would be incorrectly rendered on the client). + // However, we already warn about bad casing elsewhere. + // So we'll skip the misleading extra mismatch warning in this case. + isMismatchDueToBadCasing = true; + extraAttributes.delete(standardName); } - serverValue = getValueForAttribute(domElement, propKey, value); + extraAttributes.delete(propKey); } - + const serverValue = getValueForAttribute(domElement, propKey, value); if (!isMismatchDueToBadCasing) { warnForPropDifference(propKey, serverValue, value); } diff --git a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js index fd41ae777ed58..74f6676ac740a 100644 --- a/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js +++ b/packages/react-dom-bindings/src/server/ReactDOMServerFormatConfig.js @@ -39,11 +39,6 @@ import { } from 'react-server/src/ReactServerStreamConfig'; import isAttributeNameSafe from '../shared/isAttributeNameSafe'; -import { - getPropertyInfo, - NUMERIC, - POSITIVE_NUMERIC, -} from '../shared/DOMProperty'; import isUnitlessNumber from '../shared/isUnitlessNumber'; import {checkControlledValueProps} from '../shared/ReactControlledValuePropTypes'; @@ -810,7 +805,7 @@ function pushAttribute( attributeEnd, ); } - break; + return; } case 'cols': case 'rows': @@ -831,7 +826,7 @@ function pushAttribute( attributeEnd, ); } - break; + return; } case 'rowSpan': case 'start': { @@ -849,7 +844,7 @@ function pushAttribute( attributeEnd, ); } - break; + return; } // A few React string attributes have a different name. // This is a mapping from React prop names to the attribute names. @@ -1123,76 +1118,38 @@ function pushAttribute( case 'xmlSpace': pushStringAttribute(target, 'xml:space', value); break; - } - if ( - // shouldIgnoreAttribute - // We have already filtered out null/undefined and reserved words. - name.length > 2 && - (name[0] === 'o' || name[0] === 'O') && - (name[1] === 'n' || name[1] === 'N') - ) { - return; - } - - const propertyInfo = getPropertyInfo(name); - if (propertyInfo !== null) { - // shouldRemoveAttribute - switch (typeof value) { - case 'function': - case 'symbol': // eslint-disable-line - return; - case 'boolean': { + default: + if ( + // shouldIgnoreAttribute + // We have already filtered out null/undefined and reserved words. + name.length > 2 && + (name[0] === 'o' || name[0] === 'O') && + (name[1] === 'n' || name[1] === 'N') + ) { return; } - } - const attributeName = propertyInfo.attributeName; - const attributeNameChunk = stringToChunk(attributeName); // TODO: If it's known we can cache the chunk. - - switch (propertyInfo.type) { - case NUMERIC: - if (!isNaN(value)) { - target.push( - attributeSeparator, - attributeNameChunk, - attributeAssign, - stringToChunk(escapeTextForBrowser(value)), - attributeEnd, - ); - } - break; - case POSITIVE_NUMERIC: - if (!isNaN(value) && (value: any) >= 1) { - target.push( - attributeSeparator, - attributeNameChunk, - attributeAssign, - stringToChunk(escapeTextForBrowser(value)), - attributeEnd, - ); - } - break; - } - } else if (isAttributeNameSafe(name)) { - // shouldRemoveAttribute - switch (typeof value) { - case 'function': - case 'symbol': // eslint-disable-line - return; - case 'boolean': { - const prefix = name.toLowerCase().slice(0, 5); - if (prefix !== 'data-' && prefix !== 'aria-') { - return; + if (isAttributeNameSafe(name)) { + // shouldRemoveAttribute + switch (typeof value) { + case 'function': + case 'symbol': // eslint-disable-line + return; + case 'boolean': { + const prefix = name.toLowerCase().slice(0, 5); + if (prefix !== 'data-' && prefix !== 'aria-') { + return; + } + } } + target.push( + attributeSeparator, + stringToChunk(name), + attributeAssign, + stringToChunk(escapeTextForBrowser(value)), + attributeEnd, + ); } - } - target.push( - attributeSeparator, - stringToChunk(name), - attributeAssign, - stringToChunk(escapeTextForBrowser(value)), - attributeEnd, - ); } } diff --git a/packages/react-dom-bindings/src/shared/DOMProperty.js b/packages/react-dom-bindings/src/shared/DOMProperty.js deleted file mode 100644 index 04044b0a11726..0000000000000 --- a/packages/react-dom-bindings/src/shared/DOMProperty.js +++ /dev/null @@ -1,65 +0,0 @@ -/** - * 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 - */ - -type PropertyType = 0 | 1 | 2 | 3 | 4 | 5 | 6; - -// An attribute that must be numeric or parse as a numeric. -// When falsy, it should be removed. -export const NUMERIC = 5; - -// An attribute that must be positive numeric or parse as a positive numeric. -// When falsy, it should be removed. -export const POSITIVE_NUMERIC = 6; - -export type PropertyInfo = { - +attributeName: string, - +type: PropertyType, -}; - -export function getPropertyInfo(name: string): PropertyInfo | null { - return properties.hasOwnProperty(name) ? properties[name] : null; -} - -// $FlowFixMe[missing-this-annot] -function PropertyInfoRecord(type: PropertyType, attributeName: string) { - this.attributeName = attributeName; - this.type = type; -} - -// When adding attributes to this list, be sure to also add them to -// the `possibleStandardNames` module to ensure casing and incorrect -// name warnings. -const properties: {[string]: $FlowFixMe} = {}; - -// These are HTML attributes that must be positive numbers. -[ - 'cols', - 'rows', - 'size', - 'span', - - // 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( - POSITIVE_NUMERIC, - name, // attributeName - ); -}); - -// These are HTML attributes that must be numbers. -['rowSpan', 'start'].forEach(name => { - // $FlowFixMe[invalid-constructor] Flow no longer supports calling new on functions - properties[name] = new PropertyInfoRecord( - NUMERIC, - name.toLowerCase(), // attributeName - ); -}); From 69fa0d888c0542c4f4b7c7d74a759a7df13e748d Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 4 Apr 2023 10:53:43 -0400 Subject: [PATCH 16/16] Rm copypasta comment --- packages/react-dom-bindings/src/client/ReactDOMComponent.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index e54df227cc858..60776996e537a 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -276,7 +276,6 @@ function setProp( ): void { switch (key) { case 'style': { - // Relies on `updateStylesByID` not mutating `styleUpdates`. setValueForStyles(domElement, value); break; } @@ -920,7 +919,6 @@ function setPropOnCustomElement( ): void { switch (key) { case 'style': { - // Relies on `updateStylesByID` not mutating `styleUpdates`. setValueForStyles(domElement, value); break; }