You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Thanks for the issue @onEXHovia! I think your idea has some merit, but I am not sure the prefix is totally redundant for custom elements. Custom Elements will inherit attributes from the element they inherit, which includes Aria attributes, the Global attributes like lang and title, as well as newer global attributes like slot. If we don’t prefix with data- we run the risk of creating web compatibility issues, for example:
^ If this element was defined prior to slot existing (and @attr wasn’t data- prefixed), then slot might not have existed at all, as introducing it could have created conflicts.
The problem gets more complex when we talk about extending built in elements (which is something Catalyst doesn’t do yet, but may do in the future). The potential for clobbering attributes gets a lot higher:
Part of the purpose of Catalyst is to guide web component authors to the right path. Having @attr be data- prefixed feels right to us because we believe that’s the right path for setting attributes unless you’re mimicking semantics of an existing attribute (e.g. <img loading=… />). I think we could maybe have a better story for mimicking existing attributes… hopefully something we’ll address in the future.
However I do think there’s something to pick at here… the ergonomics of @attr - especially in relation to attributeChangedCallback - can be a little confusing. The data- prefix is hidden from you until you actually start to rely on it. We hope to address this in 2.0 which will be released soon!
Thank you for such a quick answer, I partly agree with you, but all the examples that have been described are not a problem of catalyst, but authors web components. It seems to me such problems can be described in the documentation. Still, the configuration of this behavior would be useful.
It kind of becomes a problem of catalyst, as it is expressly designed to guide authors to the right choices.
In the case of data-, it is usually the right choice because of the aforementioned issues. The times where it's not are seldom, and I think someone with enough experience on when not to use data- prefixes is likely to have enough knowledge on how to drop out of @attr and write the attribute hookup manually.
I think there are a few of things I want to pull out of this though:
We should find a way to minimise how often someone has to write data-...
@attr ergonomics with attributeChangedCallback are sort of painful, and we should address that.
Built in attribute names come with baked in semantics. We should deliver a way to apply those baked in semantics in a way that makes them easy to opt in to use (eg img's loading attribute)
I'd like to work hard to avoid configuration, and so having an opt out of data- would need to have really compelling reasons. I'd sooner drop it altogether but I don't think we can in good conscience.
Reading any documentation on custom elements, the examples almost always setting attributes html without the data prefix, thereby showing developers that this is a standard approach to developing web components. The same can be said about vue, react... the same approach is everywhere.
Since Catalyst extends the capabilities of web components, it was strange for me to see recommendations for setting attributes via data, it looks a bit "dirty". These are just my thoughts on the current design around attributes, but im ok with current implementation.
Upon reflecting on this, and looking at the list of existing HTML attribute names, and to see if we could find I happy medium, I think I’m more on board with dropping the data- prefix but adding a restriction that @attr names must be at least 2 words (and so the HTML attribute will have a dash), the justification being:
It provides a nice understandable symmetry with custom element names.
Most built in HTML attribute names are lower-case all-one-word which seems to be a continuing trend (e.g. datetime, contenteditable, enterkeyhint, formaction). The exceptions here are accep-charset and http-equiv - neither of which are global - and data-* and aria-* which are both pseudo namespaces.
Perhaps the same point as above, but enforcing a - means less likelihood of collision with new global attributes if they ever get added.
Perhaps we should not try to solve this problem for the developer, but inform him or throw an exception if he tries to define a property from the prototype. I'm not sure how much this is possible, but hasOwnProperty is suitable for this case.
Appreciate the feedback, and I think you've listed two great examples of attributes which have specific built in semantics, which we'd like Catalyst components to respect, and so we're working to make "Abilities" (mixin decorators) which provide mechanisms that align tightly to the existing DOM specs. These abilities can then be dropped in to Controller, so these might look something like:
/*The forable mixin: - adds `htmlFor` property (mapping to `for` attribute) - watches for `for` attribute changes. - calls `forElementChangedCallback` with the new for element*/import{forable}from'@github/catalyst'/*The loadable mixin: - adds `src` and `loading` property - watches for `src` and `loading` attribute changes. - calls `load()` when `src` changes (if connected) - calls load on `connectedCallback` if `loading=eager` - calls load when visible if `loading=lazy` - `load()` calls get debounced to microtask - emits `loadstart, `load`, `loadend`, and `error` events, where appropriate*/import{loadable}from'@github/catalyst'
@forable
@loadable
@controllerclassAutoCompleteElementextendsHTMLElement{forElementChangedCallback(newForElement: Element){this.htmlFor// returns '#for' }load(request: Request){}}
This way we get the best of both worlds: full flexibility with @attr without clobbering existing attributes, while also being able to pull in semantics for attributes that match the existing Web Platform featureset.
Attribute path not exist in DOM specs, for this reason calling an element <route-page path="link">text</route-page> it looks logical than if property was made up 2-words <route-page path-link="link">text</route-page> or with prefix <route-page data-path="link">text</route-page>
I think with a little creative naming one should be able to work around the limitation, for example in this case pathName might be suitable - as it matches the pathName property in URL. Failing any creativity dataPath would work equally well. Folks manage to work around the same limitation of CustomElements, so while I think we could find examples that deviate from the rule, I still think it’s a good compromise between the danger of clobbering global attributes and the ability to express attributes cleanly without the data- prefix.
I agree that overriding global or native properties should be avoided, can this be worked around by simply throwing an error (with a helpfully specific message) if a property with the attr's name already exists on the component? It would then be up to the library consumer to resolve the conflict. Warnings about potential naming conflicts could also be added to the documentation, possibly including links to cases like the above Jira incident, to educate developers about the risks.
can this be worked around by simply throwing an error (with a helpfully specific message) if a property with the attr's name already exists on the component?
The problem with that is that we don't know what new attributes are to be introduced. In the case of popup, it was a new attribute that doesn't exist in the ecosystem right now.
Hmm, good point. My thought was a runtime check to see if a property exists on the DOM element, but that wouldn't cover possible attributes. And there doesn't seem to be a way to get a list of possible attribute names for an element or element type, short of looking at the spec. Sigh.
Activity
keithamus commentedon Apr 30, 2022
Thanks for the issue @onEXHovia! I think your idea has some merit, but I am not sure the prefix is totally redundant for custom elements. Custom Elements will inherit attributes from the element they inherit, which includes Aria attributes, the Global attributes like
langandtitle, as well as newer global attributes likeslot. If we don’t prefix withdata-we run the risk of creating web compatibility issues, for example:^ If this element was defined prior to
slotexisting (and@attrwasn’tdata-prefixed), thenslotmight not have existed at all, as introducing it could have created conflicts.The problem gets more complex when we talk about extending built in elements (which is something Catalyst doesn’t do yet, but may do in the future). The potential for clobbering attributes gets a lot higher:
Part of the purpose of Catalyst is to guide web component authors to the right path. Having
@attrbedata-prefixed feels right to us because we believe that’s the right path for setting attributes unless you’re mimicking semantics of an existing attribute (e.g.<img loading=… />). I think we could maybe have a better story for mimicking existing attributes… hopefully something we’ll address in the future.However I do think there’s something to pick at here… the ergonomics of
@attr- especially in relation toattributeChangedCallback- can be a little confusing. Thedata-prefix is hidden from you until you actually start to rely on it. We hope to address this in 2.0 which will be released soon!onEXHovia commentedon Apr 30, 2022
Thank you for such a quick answer, I partly agree with you, but all the examples that have been described are not a problem of catalyst, but authors web components. It seems to me such problems can be described in the documentation. Still, the configuration of this behavior would be useful.
keithamus commentedon Apr 30, 2022
It kind of becomes a problem of catalyst, as it is expressly designed to guide authors to the right choices.
In the case of
data-, it is usually the right choice because of the aforementioned issues. The times where it's not are seldom, and I think someone with enough experience on when not to usedata-prefixes is likely to have enough knowledge on how to drop out of@attrand write the attribute hookup manually.I think there are a few of things I want to pull out of this though:
data-...@attrergonomics withattributeChangedCallbackare sort of painful, and we should address that.loadingattribute)I'd like to work hard to avoid configuration, and so having an opt out of
data-would need to have really compelling reasons. I'd sooner drop it altogether but I don't think we can in good conscience.onEXHovia commentedon Apr 30, 2022
Reading any documentation on custom elements, the examples almost always setting attributes html without the
dataprefix, thereby showing developers that this is a standard approach to developing web components. The same can be said about vue, react... the same approach is everywhere.Since Catalyst extends the capabilities of web components, it was strange for me to see recommendations for setting attributes via
data, it looks a bit "dirty". These are just my thoughts on the current design around attributes, but im ok with current implementation.keithamus commentedon May 5, 2022
Upon reflecting on this, and looking at the list of existing HTML attribute names, and to see if we could find I happy medium, I think I’m more on board with dropping the
data-prefix but adding a restriction that@attrnames must be at least 2 words (and so the HTML attribute will have a dash), the justification being:datetime,contenteditable,enterkeyhint,formaction). The exceptions here areaccep-charsetandhttp-equiv- neither of which are global - anddata-*andaria-*which are both pseudo namespaces.-means less likelihood of collision with new global attributes if they ever get added.onEXHovia commentedon May 6, 2022
Using the example of
Autocomplete Element, it is difficult to come up with a 2-word propertry name.Perhaps we should not try to solve this problem for the developer, but inform him or throw an exception if he tries to define a property from the prototype. I'm not sure how much this is possible, but
hasOwnPropertyis suitable for this case.keithamus commentedon May 6, 2022
Appreciate the feedback, and I think you've listed two great examples of attributes which have specific built in semantics, which we'd like Catalyst components to respect, and so we're working to make "Abilities" (mixin decorators) which provide mechanisms that align tightly to the existing DOM specs. These abilities can then be dropped in to Controller, so these might look something like:
This way we get the best of both worlds: full flexibility with
@attrwithout clobbering existing attributes, while also being able to pull in semantics for attributes that match the existing Web Platform featureset.onEXHovia commentedon May 6, 2022
@keithamus looks great, its would be useful. Wanted continue the discussion on the name properties of 2 words with an example
Attribute
pathnot exist in DOM specs, for this reason calling an element<route-page path="link">text</route-page>it looks logical than if property was made up 2-words<route-page path-link="link">text</route-page>or with prefix<route-page data-path="link">text</route-page>keithamus commentedon May 6, 2022
At the risk of pedantry
pathexists on svg elements, but I totally get your point there.I think with a little creative naming one should be able to work around the limitation, for example in this case
pathNamemight be suitable - as it matches thepathNameproperty inURL. Failing any creativitydataPathwould work equally well. Folks manage to work around the same limitation of CustomElements, so while I think we could find examples that deviate from the rule, I still think it’s a good compromise between the danger of clobbering global attributes and the ability to express attributes cleanly without thedata-prefix.keithamus commentedon Jul 12, 2022
An example of webcompat issues with the addition of new global attributes that cause issues is the new popup attribute, which caused webcompat issues with Jira which used the property. This is good motivation for us to avoid clobbering global properties.
chilimangoes commentedon Oct 13, 2022
I agree that overriding global or native properties should be avoided, can this be worked around by simply throwing an error (with a helpfully specific message) if a property with the attr's name already exists on the component? It would then be up to the library consumer to resolve the conflict. Warnings about potential naming conflicts could also be added to the documentation, possibly including links to cases like the above Jira incident, to educate developers about the risks.
keithamus commentedon Oct 13, 2022
The problem with that is that we don't know what new attributes are to be introduced. In the case of popup, it was a new attribute that doesn't exist in the ecosystem right now.
chilimangoes commentedon Oct 13, 2022
Hmm, good point. My thought was a runtime check to see if a property exists on the DOM element, but that wouldn't cover possible attributes. And there doesn't seem to be a way to get a list of possible attribute names for an element or element type, short of looking at the spec. Sigh.