From 43a70a610dd2cc298ab5592deebfbf8f7bbac127 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Thu, 30 Mar 2023 18:38:50 -0400 Subject: [PATCH] Limit the meaning of "custom element" to not include `is` (#26524) This PR has a bunch of surrounding refactoring. See individual commits. The main change is that we no longer special case `typeof is === 'string'` as a special case according to the `enableCustomElementPropertySupport` flag. Effectively this means that you can't use custom properties/events, other than the ones React knows about on `` extensions. This is unfortunate but there's too many paths that are forked in inconsistent ways since we fork based on tag name. I think __the solution is to let all React elements set unknown properties/events in the same way as this flag__ but that's a bigger change than this flag implies. Since `is` is not universally supported yet anyway, this doesn't seem like a huge loss. Attributes still work. We still support passing the `is` prop and turn that into the appropriate createElement call. @josepharhar --- .../src/client/DOMPropertyOperations.js | 48 +--- .../src/client/ReactDOMComponent.js | 217 ++++-------------- .../src/client/ReactDOMHostConfig.js | 128 +++++++++-- .../src/events/plugins/ChangeEventPlugin.js | 4 +- .../src/server/ReactDOMServerFormatConfig.js | 8 +- .../src/shared/ReactDOMUnknownPropertyHook.js | 4 +- ...sCustomComponent.js => isCustomElement.js} | 6 +- ...eactDOMServerIntegrationAttributes-test.js | 23 +- 8 files changed, 185 insertions(+), 253 deletions(-) rename packages/react-dom-bindings/src/shared/{isCustomComponent.js => isCustomElement.js} (85%) diff --git a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js index f0649b97522b4..d89f40037ef2f 100644 --- a/packages/react-dom-bindings/src/client/DOMPropertyOperations.js +++ b/packages/react-dom-bindings/src/client/DOMPropertyOperations.js @@ -253,10 +253,6 @@ export function getValueForAttributeOnCustomComponent( } return expected === undefined ? undefined : null; } - if (enableCustomElementPropertySupport && name === 'className') { - // className is a special cased property on the server to render as an attribute. - name = 'class'; - } const value = node.getAttribute(name); if (enableCustomElementPropertySupport) { @@ -448,11 +444,7 @@ export function setValueForPropertyOnCustomComponent( name: string, value: mixed, ) { - if ( - enableCustomElementPropertySupport && - name[0] === 'o' && - name[1] === 'n' - ) { + if (name[0] === 'o' && name[1] === 'n') { const useCapture = name.endsWith('Capture'); const eventName = name.substr(2, useCapture ? name.length - 9 : undefined); @@ -477,40 +469,16 @@ export function setValueForPropertyOnCustomComponent( } } - if (enableCustomElementPropertySupport && name in (node: any)) { + if (name in (node: any)) { (node: any)[name] = value; return; } - if (isAttributeNameSafe(name)) { - // shouldRemoveAttribute - if (value === null) { - node.removeAttribute(name); - return; - } - switch (typeof value) { - case 'undefined': - case 'function': - case 'symbol': // eslint-disable-line - node.removeAttribute(name); - return; - case 'boolean': { - if (enableCustomElementPropertySupport) { - if (value === true) { - node.setAttribute(name, ''); - return; - } - node.removeAttribute(name); - return; - } - } - } - if (__DEV__) { - checkAttributeStringCoercion(value, name); - } - node.setAttribute( - name, - enableTrustedTypesIntegration ? (value: any) : '' + (value: any), - ); + if (value === true) { + node.setAttribute(name, ''); + return; } + + // From here, it's the same as any attribute + setValueForAttribute(node, name, value); } diff --git a/packages/react-dom-bindings/src/client/ReactDOMComponent.js b/packages/react-dom-bindings/src/client/ReactDOMComponent.js index 93686a54b8b7e..e587d84707189 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMComponent.js +++ b/packages/react-dom-bindings/src/client/ReactDOMComponent.js @@ -15,7 +15,6 @@ import { } from '../events/EventRegistry'; import {canUseDOM} from 'shared/ExecutionEnvironment'; -import hasOwnProperty from 'shared/hasOwnProperty'; import {checkHtmlStringCoercion} from 'shared/CheckStringCoercion'; import { @@ -59,15 +58,13 @@ import { } from './CSSPropertyOperations'; import {HTML_NAMESPACE, getIntrinsicNamespace} from './DOMNamespaces'; import {getPropertyInfo} from '../shared/DOMProperty'; -import {DOCUMENT_NODE} from './HTMLNodeType'; -import isCustomComponent from '../shared/isCustomComponent'; +import isCustomElement from '../shared/isCustomElement'; 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 { - enableTrustedTypesIntegration, enableCustomElementPropertySupport, enableClientRenderFallbackOnTextMismatch, enableHostSingletons, @@ -79,25 +76,8 @@ import { } from '../events/DOMPluginEventSystem'; let didWarnInvalidHydration = false; -let didWarnScriptTags = false; - -let warnedUnknownTags: { - [key: string]: boolean, -}; let canDiffStyleForHydrationWarning; - if (__DEV__) { - warnedUnknownTags = { - // There are working polyfills for . Let people use it. - dialog: true, - // Electron ships a custom tag to display external web content in - // an isolated frame and process. - // This tag is not present in non Electron environments such as JSDom which - // is often used for testing purposes. - // @see https://electronjs.org/docs/api/webview-tag - webview: true, - }; - // IE 11 parses & normalizes the style attribute as opposed to other // browsers. It adds spaces and sorts the properties in some // non-alphabetical order. Handling that would require sorting CSS @@ -264,14 +244,6 @@ export function checkForUnmatchedText( } } -export function getOwnerDocumentFromRootContainer( - rootContainerElement: Element | Document | DocumentFragment, -): Document { - return rootContainerElement.nodeType === DOCUMENT_NODE - ? (rootContainerElement: any) - : rootContainerElement.ownerDocument; -} - function noop() {} export function trapClickOnNonInteractiveElement(node: HTMLElement) { @@ -292,7 +264,7 @@ function setProp( tag: string, key: string, value: mixed, - isCustomComponentTag: boolean, + isCustomElementTag: boolean, props: any, ): void { switch (key) { @@ -418,8 +390,16 @@ function setProp( warnForInvalidEventListener(key, value); } } else { - if (isCustomComponentTag) { - setValueForPropertyOnCustomComponent(domElement, key, value); + 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); + } } else { if ( // shouldIgnoreAttribute @@ -443,136 +423,6 @@ function setProp( } } -// creates a script element that won't execute -export function createPotentiallyInlineScriptElement( - ownerDocument: Document, -): Element { - // Create the script via .innerHTML so its "parser-inserted" flag is - // set to true and it does not execute - const div = ownerDocument.createElement('div'); - if (__DEV__) { - if (enableTrustedTypesIntegration && !didWarnScriptTags) { - console.error( - 'Encountered a script tag while rendering React component. ' + - 'Scripts inside React components are never executed when rendering ' + - 'on the client. Consider using template tag instead ' + - '(https://developer.mozilla.org/en-US/docs/Web/HTML/Element/template).', - ); - didWarnScriptTags = true; - } - } - div.innerHTML = '