attributes/properties transformation & custom elements defaulting to attributes #373
Conversation
| else if (isCE && !isProp && !isChildProp) node[toPropertyName(prop)] = value; | ||
| else node[propAlias || prop] = value; | ||
| else if (isCE && !isProp && !isChildProp) { | ||
| node.setAttribute(prop, value); |
There was a problem hiding this comment.
This is nice, unlike React's which forces a string coercion of value before passing it to setAttribute. The native DOM setAttribute method already will do that coercion, but! a custom element class can override the setAttribute method to also handle non-string values and for example map them directly to JS properties, which is not possible with React at time of this writing.
This means that, even if Solid JSX calls el.setAttribute, a custom element can itself be designed to pass the value directly to JS and skip the native setAttribute, therefore achieving the same thing as prop: without the user having to use prop: syntax. The A-Frame library, for example, overrides setAttribute like this for all its elements. I'm contemplating adding this to Lume Element. In my case, I could map JS value types to the JSX attribute types. But other people's CE libraries don't have to, they can map only string types for non-prop: attributes for example.
There was a problem hiding this comment.
In fact, had React at least removed that stupid string coercion, they'd have largely eliminated the need for React 19 CE support because all CEd would have had a way to accept non-string values.
But they didn't listen.
There was a problem hiding this comment.
Overriding setAttribute is interesting and something I hadn't thought of. I'm sure it has other implications/inconsistency but it definitely is interesting.
| ) { | ||
| if (forceProp) { | ||
| prop = prop.slice(5); | ||
| isProp = true; | ||
| } else if (isHydrating(node)) return value; | ||
| if (prop === "class" || prop === "className") className(node, value); | ||
| else if (isCE && !isProp && !isChildProp) node[toPropertyName(prop)] = value; | ||
| else node[propAlias || prop] = value; | ||
| else if (isCE && !isProp && !isChildProp) { |
There was a problem hiding this comment.
Seems like we wouldn't need isCE anymore. The logic would be for any element (doesn't matter if it is CE or not) set the attribute (except for the special cases).
There was a problem hiding this comment.
Yeah that is the benefit here. Not treating CE's special. I think that is directionally where we go. I sort of over compensated because I wanted WC development to be pleasant but we shouldn't really be trying to pave over the awkward parts. They are the way they are and its up to the specs to handle that.
…butes for custom elements
titoBouzout
force-pushed
the
attrnotransform
branch
from
126dae8
to
ec18f11
Compare
|
Just as a reminder, it may be a need to also update |
It includes two changes:
propertiesin custom elements. It defaults toattributes.Rationality: Seems like numerous people expect custom elements to just receive attributes 1, 2, 3, 4, and they will use
prop:when they want to set a property. It seems strange wanting to useprop:, but it looks to me, they are used to dealing with this sort of problem. Kind of makes sense as attributes can only receive strings.Avoiding transformation of case and dashes, makes the issue easier to deal with it. I do not think this will cause any problem, as one won't mix up case when using attr/props, so normalizing it is unnecessary and could cause some other issues
If I do not miss anything, this only affects custom-elements. I suggest merging it whenever possible, as this is something people seem to expect to behave this way. I also would like to recommend merging it as
v1.10.0to not break and alienate people using the previous behaviour.