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 `<input is="my-input">`
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
This commit is contained in:
Sebastian Markbåge
2023-03-30 18:38:50 -04:00
committed by GitHub
parent 1308e49a69
commit 43a70a610d
8 changed files with 185 additions and 253 deletions
@@ -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);
}
+39 -178
View File
@@ -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 <dialog>. Let people use it.
dialog: true,
// Electron ships a custom <webview> 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 = '<script><' + '/script>'; // eslint-disable-line
// This is guaranteed to yield a script element.
const firstChild = ((div.firstChild: any): HTMLScriptElement);
const element = div.removeChild(firstChild);
return element;
}
export function createSelectElement(
props: Object,
ownerDocument: Document,
): Element {
let element;
if (typeof props.is === 'string') {
element = ownerDocument.createElement('select', {is: props.is});
} else {
// Separate else branch instead of using `props.is || undefined` above because of a Firefox bug.
// See discussion in https://github.com/facebook/react/pull/6896
// and discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1276240
element = ownerDocument.createElement('select');
}
if (props.multiple) {
element.multiple = true;
} else if (props.size) {
// Setting a size greater than 1 causes a select to behave like `multiple=true`, where
// it is possible that no option is selected.
//
// This is only necessary when a select in "single selection mode".
element.size = props.size;
}
return element;
}
// Creates elements in the HTML namesapce
export function createHTMLElement(
type: string,
props: Object,
ownerDocument: Document,
): Element {
if (__DEV__) {
switch (type) {
case 'script':
case 'select':
console.error(
'createHTMLElement was called with a "%s" type. This type has special creation logic in React and should use the create function implemented specifically for it. This is a bug in React.',
type,
);
break;
case 'svg':
case 'math':
console.error(
'createHTMLElement was called with a "%s" type. This type must be created with Document.createElementNS which this method does not implement. This is a bug in React.',
type,
);
}
}
let isCustomComponentTag;
let element: Element;
if (__DEV__) {
isCustomComponentTag = isCustomComponent(type, props);
// Should this check be gated by parent namespace? Not sure we want to
// allow <SVG> or <mATH>.
if (!isCustomComponentTag && type !== type.toLowerCase()) {
console.error(
'<%s /> is using incorrect casing. ' +
'Use PascalCase for React components, ' +
'or lowercase for HTML elements.',
type,
);
}
}
if (typeof props.is === 'string') {
element = ownerDocument.createElement(type, {is: props.is});
} else {
// Separate else branch instead of using `props.is || undefined` above because of a Firefox bug.
// See discussion in https://github.com/facebook/react/pull/6896
// and discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1276240
element = ownerDocument.createElement(type);
}
if (__DEV__) {
if (
!isCustomComponentTag &&
// $FlowFixMe[method-unbinding]
Object.prototype.toString.call(element) ===
'[object HTMLUnknownElement]' &&
!hasOwnProperty.call(warnedUnknownTags, type)
) {
warnedUnknownTags[type] = true;
console.error(
'The tag <%s> is unrecognized in this browser. ' +
'If you meant to render a React component, start its name with ' +
'an uppercase letter.',
type,
);
}
}
return element;
}
export function createTextNode(
text: string,
rootContainerElement: Element | Document | DocumentFragment,
): Text {
return getOwnerDocumentFromRootContainer(rootContainerElement).createTextNode(
text,
);
}
export function setInitialProperties(
domElement: Element,
tag: string,
@@ -807,7 +657,6 @@ export function setInitialProperties(
}
// defaultChecked and defaultValue are ignored by setProp
default: {
// TODO: If the `is` prop is specified, this should go through the isCustomComponentTag flow.
setProp(domElement, tag, propKey, propValue, false, props);
}
}
@@ -816,7 +665,7 @@ export function setInitialProperties(
}
}
const isCustomComponentTag = isCustomComponent(tag, props);
const isCustomElementTag = isCustomElement(tag, props);
for (const propKey in props) {
if (!props.hasOwnProperty(propKey)) {
continue;
@@ -825,7 +674,7 @@ export function setInitialProperties(
if (propValue == null) {
continue;
}
setProp(domElement, tag, propKey, propValue, isCustomComponentTag, props);
setProp(domElement, tag, propKey, propValue, isCustomElementTag, props);
}
}
@@ -920,6 +769,13 @@ export function diffProperties(
}
break;
}
case 'is':
if (__DEV__) {
console.error(
'Cannot update the "is" prop after it has been initialized.',
);
}
// eslint-disable-next-line no-fallthrough
default: {
(updatePayload = updatePayload || []).push(propKey, nextProp);
}
@@ -1095,7 +951,6 @@ export function updateProperties(
}
// defaultChecked and defaultValue are ignored by setProp
default: {
// TODO: If the `is` prop is specified, this should go through the isCustomComponentTag flow.
setProp(domElement, tag, propKey, propValue, false, nextProps);
}
}
@@ -1104,21 +959,12 @@ export function updateProperties(
}
}
// TODO: Handle wasCustomComponentTag. Changing "is" isn't valid.
// const wasCustomComponentTag = isCustomComponent(tag, lastProps);
const isCustomComponentTag = isCustomComponent(tag, nextProps);
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,
isCustomComponentTag,
nextProps,
);
setProp(domElement, tag, propKey, propValue, isCustomElementTag, nextProps);
}
}
@@ -1218,6 +1064,21 @@ function diffHydratedCustomComponent(
continue;
}
// eslint-disable-next-line no-fallthrough
case 'className':
if (enableCustomElementPropertySupport) {
// className is a special cased property on the server to render as an attribute.
extraAttributeNames.delete('class');
const serverValue = getValueForAttributeOnCustomComponent(
domElement,
'class',
nextProp,
);
if (nextProp !== serverValue) {
warnForPropDifference('className', serverValue, nextProp);
}
continue;
}
// eslint-disable-next-line no-fallthrough
default: {
let ownNamespaceDev = parentNamespaceDev;
if (ownNamespaceDev === HTML_NAMESPACE) {
@@ -1504,7 +1365,7 @@ export function diffHydratedProperties(
extraAttributeNames.add(attributes[i].name);
}
}
if (isCustomComponent(tag, props)) {
if (isCustomElement(tag, props)) {
diffHydratedCustomComponent(
domElement,
tag,
+113 -15
View File
@@ -44,10 +44,6 @@ import {
export {detachDeletedInstance};
import {hasRole} from './DOMAccessibilityRoles';
import {
createHTMLElement,
createPotentiallyInlineScriptElement,
createSelectElement,
createTextNode,
setInitialProperties,
diffProperties,
updateProperties,
@@ -59,7 +55,6 @@ import {
warnForDeletedHydratableText,
warnForInsertedHydratedElement,
warnForInsertedHydratedText,
getOwnerDocumentFromRootContainer,
} from './ReactDOMComponent';
import {getSelectionInformation, restoreSelection} from './ReactInputSelection';
import setTextContent from './setTextContent';
@@ -90,6 +85,7 @@ import {
enableScopeAPI,
enableFloat,
enableHostSingletons,
enableTrustedTypesIntegration,
} from 'shared/ReactFeatureFlags';
import {
HostComponent,
@@ -119,6 +115,9 @@ export type Props = {
left?: null | number,
right?: null | number,
top?: null | number,
is?: string,
size?: number,
multiple?: boolean,
...
};
type RawProps = {
@@ -182,6 +181,14 @@ let selectionInformation: null | SelectionInformation = null;
export * from 'react-reconciler/src/ReactFiberHostConfigWithNoPersistence';
function getOwnerDocumentFromRootContainer(
rootContainerElement: Element | Document | DocumentFragment,
): Document {
return rootContainerElement.nodeType === DOCUMENT_NODE
? (rootContainerElement: any)
: rootContainerElement.ownerDocument;
}
export function getRootHostContext(
rootContainerInstance: Container,
): HostContext {
@@ -285,7 +292,8 @@ export function createHoistableInstance(
const ownerDocument = getOwnerDocumentFromRootContainer(
rootContainerInstance,
);
const domElement: Instance = createHTMLElement(type, props, ownerDocument);
const domElement: Instance = ownerDocument.createElement(type);
precacheFiberNode(internalInstanceHandle, domElement);
updateFiberProps(domElement, props);
setInitialProperties(domElement, type, props);
@@ -293,6 +301,20 @@ export function createHoistableInstance(
return domElement;
}
let didWarnScriptTags = false;
const warnedUnknownTags: {
[key: string]: boolean,
} = {
// There are working polyfills for <dialog>. Let people use it.
dialog: true,
// Electron ships a custom <webview> 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,
};
export function createInstance(
type: string,
props: Props,
@@ -334,20 +356,94 @@ export function createInstance(
break;
default:
switch (type) {
case 'svg':
case 'svg': {
domElement = ownerDocument.createElementNS(SVG_NAMESPACE, type);
break;
case 'math':
}
case 'math': {
domElement = ownerDocument.createElementNS(MATH_NAMESPACE, type);
break;
case 'script':
domElement = createPotentiallyInlineScriptElement(ownerDocument);
}
case 'script': {
// 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 = '<script><' + '/script>'; // eslint-disable-line
// This is guaranteed to yield a script element.
const firstChild = ((div.firstChild: any): HTMLScriptElement);
domElement = div.removeChild(firstChild);
break;
case 'select':
domElement = createSelectElement(props, ownerDocument);
}
case 'select': {
if (typeof props.is === 'string') {
domElement = ownerDocument.createElement('select', {is: props.is});
} else {
// Separate else branch instead of using `props.is || undefined` above because of a Firefox bug.
// See discussion in https://github.com/facebook/react/pull/6896
// and discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1276240
domElement = ownerDocument.createElement('select');
}
if (props.multiple) {
domElement.multiple = true;
} else if (props.size) {
// Setting a size greater than 1 causes a select to behave like `multiple=true`, where
// it is possible that no option is selected.
//
// This is only necessary when a select in "single selection mode".
domElement.size = props.size;
}
break;
default:
domElement = createHTMLElement(type, props, ownerDocument);
}
default: {
if (typeof props.is === 'string') {
domElement = ownerDocument.createElement(type, {is: props.is});
} else {
// Separate else branch instead of using `props.is || undefined` above because of a Firefox bug.
// See discussion in https://github.com/facebook/react/pull/6896
// and discussion in https://bugzilla.mozilla.org/show_bug.cgi?id=1276240
domElement = ownerDocument.createElement(type);
}
if (__DEV__) {
if (type.indexOf('-') === -1) {
// We're not SVG/MathML and we don't have a dash, so we're not a custom element
// Even if you use `is`, these should be of known type and lower case.
if (type !== type.toLowerCase()) {
console.error(
'<%s /> is using incorrect casing. ' +
'Use PascalCase for React components, ' +
'or lowercase for HTML elements.',
type,
);
}
if (
// $FlowFixMe[method-unbinding]
Object.prototype.toString.call(domElement) ===
'[object HTMLUnknownElement]' &&
!hasOwnProperty.call(warnedUnknownTags, type)
) {
warnedUnknownTags[type] = true;
console.error(
'The tag <%s> is unrecognized in this browser. ' +
'If you meant to render a React component, start its name with ' +
'an uppercase letter.',
type,
);
}
}
}
}
}
}
precacheFiberNode(internalInstanceHandle, domElement);
@@ -429,7 +525,9 @@ export function createTextInstance(
const hostContextDev = ((hostContext: any): HostContextDev);
validateDOMNesting(null, text, hostContextDev.ancestorInfo);
}
const textNode: TextInstance = createTextNode(text, rootContainerInstance);
const textNode: TextInstance = getOwnerDocumentFromRootContainer(
rootContainerInstance,
).createTextNode(text);
precacheFiberNode(internalInstanceHandle, textNode);
return textNode;
}
@@ -35,7 +35,7 @@ import {
processDispatchQueue,
accumulateTwoPhaseListeners,
} from '../DOMPluginEventSystem';
import isCustomComponent from '../../shared/isCustomComponent';
import isCustomElement from '../../shared/isCustomElement';
function registerEvents() {
registerTwoPhaseEvent('onChange', [
@@ -312,7 +312,7 @@ function extractEvents(
} else if (
enableCustomElementPropertySupport &&
targetInst &&
isCustomComponent(targetInst.elementType, targetInst.memoizedProps)
isCustomElement(targetInst.elementType, targetInst.memoizedProps)
) {
getTargetInstFunc = getTargetInstForChangeEvent;
}
@@ -2483,11 +2483,7 @@ export function pushStartInstance(
formatContext.insertionMode !== SVG_MODE &&
formatContext.insertionMode !== MATHML_MODE
) {
if (
type.indexOf('-') === -1 &&
typeof props.is !== 'string' &&
type.toLowerCase() !== type
) {
if (type.indexOf('-') === -1 && type.toLowerCase() !== type) {
console.error(
'<%s /> is using incorrect casing. ' +
'Use PascalCase for React components, ' +
@@ -2608,7 +2604,7 @@ export function pushStartInstance(
);
}
default: {
if (type.indexOf('-') === -1 && typeof props.is !== 'string') {
if (type.indexOf('-') === -1) {
// Generic element
return pushStartGenericElement(target, props, type);
} else {
@@ -7,7 +7,7 @@
import {BOOLEAN, getPropertyInfo} from './DOMProperty';
import {ATTRIBUTE_NAME_CHAR} from './isAttributeNameSafe';
import isCustomComponent from './isCustomComponent';
import isCustomElement from './isCustomElement';
import possibleStandardNames from './possibleStandardNames';
import hasOwnProperty from 'shared/hasOwnProperty';
import {enableCustomElementPropertySupport} from 'shared/ReactFeatureFlags';
@@ -308,7 +308,7 @@ function warnUnknownProperties(type, props, eventRegistry) {
}
export function validateProperties(type, props, eventRegistry) {
if (isCustomComponent(type, props)) {
if (isCustomElement(type, props) || typeof props.is === 'string') {
return;
}
warnUnknownProperties(type, props, eventRegistry);
@@ -7,9 +7,9 @@
* @flow
*/
function isCustomComponent(tagName: string, props: Object): boolean {
function isCustomElement(tagName: string, props: Object): boolean {
if (tagName.indexOf('-') === -1) {
return typeof props.is === 'string';
return false;
}
switch (tagName) {
// These are reserved SVG and MathML elements.
@@ -30,4 +30,4 @@ function isCustomComponent(tagName: string, props: Object): boolean {
}
}
export default isCustomComponent;
export default isCustomElement;
@@ -34,7 +34,7 @@ function initModules() {
};
}
const {resetModules, itRenders, clientCleanRender, clientRenderOnServerString} =
const {resetModules, itRenders, clientCleanRender} =
ReactDOMServerIntegrationUtils(initModules);
describe('ReactDOMServerIntegration', () => {
@@ -655,22 +655,31 @@ describe('ReactDOMServerIntegration', () => {
expect(e.getAttribute('class')).toBe('test');
});
itRenders('className for is elements', async render => {
const e = await render(<div is="custom-element" className="test" />, 0);
expect(e.getAttribute('className')).toBe(null);
expect(e.getAttribute('class')).toBe('test');
});
itRenders('className for custom elements', async render => {
const e = await render(<custom-element className="test" />, 0);
if (ReactFeatureFlags.enableCustomElementPropertySupport) {
const e = await render(
<div is="custom-element" className="test" />,
render === clientRenderOnServerString ? 1 : 0,
);
expect(e.getAttribute('className')).toBe(null);
expect(e.getAttribute('class')).toBe('test');
} else {
const e = await render(<div is="custom-element" className="test" />, 0);
expect(e.getAttribute('className')).toBe('test');
expect(e.getAttribute('class')).toBe(null);
}
});
itRenders('htmlFor attribute on custom elements', async render => {
itRenders('htmlFor property on is elements', async render => {
const e = await render(<div is="custom-element" htmlFor="test" />);
expect(e.getAttribute('htmlFor')).toBe(null);
expect(e.getAttribute('for')).toBe('test');
});
itRenders('htmlFor attribute on custom elements', async render => {
const e = await render(<custom-element htmlFor="test" />);
expect(e.getAttribute('htmlFor')).toBe('test');
expect(e.getAttribute('for')).toBe(null);
});