Skip to content

Commit

Permalink
Limit the meaning of "custom element" to not include is (#26524)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
sebmarkbage committed Mar 30, 2023
1 parent 1308e49 commit 43a70a6
Show file tree
Hide file tree
Showing 8 changed files with 185 additions and 253 deletions.
48 changes: 8 additions & 40 deletions packages/react-dom-bindings/src/client/DOMPropertyOperations.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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);

Expand All @@ -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);
}
Loading

0 comments on commit 43a70a6

Please sign in to comment.