className prop not working with custom DOM elements in 0.14.0-rc1

[hellosmithy]

The className prop does not appear to get mapped correctly when applied to custom DOM elements in 0.14.0-rc1.

JSFiddle: https://jsfiddle.net/hellosmithy/5pdujnfq/1/

[syranide]

It works if you set class instead.

cc @jimfb perhaps? (#3067 is related I assume)

[hellosmithy]

@syranide I think you're right that the #3067 commit introduced the issue

https://github.com/facebook/react/blob/v0.14.0-rc1/src/renderers/dom/shared/ReactDOMComponent.js#L637-L641

Any custom element will match the isCustomComponent check and skip all the DOM property operations.

I'm struggling to understand the use-case for #3067. Surely attributes should work the same for custom elements and standard DOM elements?

[syranide]

I'm struggling to understand the use-case for #3067. Surely attributes should work the same for custom elements and standard DOM elements?

I'm not very familiar with all this, but there's a difference between custom tags <customtag> and web components <web-component>. Web components are much like React components and aren't themselves visible nodes, so the list of DOM attributes don't inherently apply to them (again, just like it makes little sense passing DOM properties to React components), whereas they do for custom tags which just translate to unstyled DOM nodes.

I assume <custom-tag> was previously valid too, but as of the introduction of web components that's no longer true? This is my limited understanding of all this anyway, so don't take my word for it!

[zpao]

cc @jimfb

[jimfb]

@syranide is correct, there is a difference between DOM nodes and webcomponents (which are much more like React components). For standard DOM nodes, we know a priori that there is no className attribute, so we can commandeer the namespace for our own purposes. For web components, there is no such guarantee, so we can't steal the namespace. In fact, web component authors may prefer className over class for the same reasons we stole it for standard dom components, and to remain compatible, we need to pass it through unmolested.

Another way of thinking about this: For standard DOM nodes, we conceptually have a react-component defined for each node, that happens to take className, just as we happen to take defaultValue for uncontrolled form elements and happen to use camel case for attributes. We happen to follow some consistent naming conventions in our framework-provided components. The fact that we do most of our markup transformations at the framework level instead of inside react-component definitions is an implementation detail. Web component authors are free to follow those same conventions (or not), but we need to pass them through without transformations or we'll confuse the web components.

TLDR: Custom elements DO work in React. You must use class instead of className because the custom element spec requires that we allow users to specify a className attribute and we need to preserve that functionality for custom elements.

[hellosmithy]

I agree there's a distinction between custom elements and web components. Custom elements are one of the building blocks of web components but they are a spec in their own right. But there's a use case for creating semantically meaningful custom elements which don't extend default DOM behaviour.

Wouldn't a more consistent approach be to treat all elements the same within React but allow either users to escape attributes somehow or alternatively they have the option to dangerouslySetInnerHTML if they are indeed using web components that need to operate outside the React scope?

[jimfb]

@hellosmithy Using a component with a dash is the escape. As per the specification, a node with a dash in it may have arbitrary attributes defined.

[hellosmithy]

@jimfb I'm not sure that's a safe assumption to make. My understanding is that a custom element must have a dash in it. Custom elements are used in web components. But an element with a dash in it is not necessarily a web component.

https://developer.mozilla.org/en-US/docs/Web/Web_Components/Custom_Elements
"They are part of Web Components but they can also be used by themselves."

[jimfb]

Semantics. Custom element is a subset of the web component specification. We use the term "web component" loosely, but I believe the statement holds if you replace "web component" with "custom element"; namely that the custom element may define arbitrary attributes and process those attributes using arbitrary rules.

[hellosmithy]

@jimfb I think this is a little more than semantics. Either way this change prevents custom elements from being interoperable with core dom elements, when surely there are other options such as escaping special case attributes (className etc) for cases where people need to and thus not create a breaking change. Unless there is another way to use custom elements as purely custom elements and still use React props?

[jimfb]

You'd have to special case/escape a whole ton of attributes including every possible camelcase attribute that could ever (past, present, or future) occur on a native DOM component, not to mention dealing with event handlers and other complexities. It would not be as simple as escaping a couple special-cased attributes; the rules would be complex and confusing (certainly more so than the current attribute whitelisting) - I can't see a way to do such a thing in a way that's better than what we currently do (pass attributes straight through).

[hellosmithy]

I'm suggesting there should be a discussion on the best way to do it, escaping may not be it.

One way would be an escape pattern convention e.g. myPropEscaped (just a straw man example). But I'm also wondering, if you want to create web components that are outside of React's scope surely that is a case for using dangerouslySetInnerHTML, I would have thought that the default behaviour should be interoperability otherwise there's no way to apply React attributes such as className to custom elements, which seems an odd and inconsistent restriction. On the other hand there are other ways to pass through web component attributes in those cases where you might want to, and in those cases surely you should be explicitly saying "I don't want React to do the default behaviour in this instance", rather than it just automatically making that assumption for you.

Looking at the JSX code:

<div className="myClass" />
<div is="custom-elem" className="myClass" />
<custom-elem className="myClass" />

It seems right to expect those to behave consistently.

[hellosmithy]

@jimfb @zpao has there been any more thought on this?

Having read the thread at #140 I can see why this direction was taken, but I think it raises inconsistencies and will make code harder to reason about. I really do think escaping is the more consistent and flexible option, and forces code to be explicit if it's breaking React convention. Would be interested to hear what your thoughts are.

[jimfb]

@hellosmithy I can't see how to make it work in a way that's better than what we currently have. If you have an escaping solution that you'd like to propose, we'd be happy to take a look.

The constraints are:

• Must allow people to specify all possible attributes in a case sensitive way. Users must be able to specify any combination of class and className (zero, one, or both). Users must also be able to specify any combination of onclick and onClick (zero, one, or both). This is because custom elements can impose arbitrary processing rules on their attributes, and case sensitivity matters.
• Must be intuitive for users. Ideally, the first thing people try should work. If it doesn't work, users must have an obvious "next step" to try so they can fix their code (ie. what happens when an attribute they wanted to use gets rewritten? do they get a useful error message? how do they intuitively know what to try next?).
• Must allow us to do basic sanity checks (like preventing/mitigating XSS attacks) in a way that minimizes the impact on performance.
• Should not require modifications to the JSX syntax, which is already becoming a defacto standard as other languages/editors/IDEs/parsers understand and implement the syntax, so any changes would break all of them. You can read the specification here: https://facebook.github.io/jsx/
• Should play nice with Allow custom (nonstandard) attributes. #140 and futureproof us from maintaining an ever-growing whitelist as browsers add new elements/attributes.