Feature: attrs without data- prefix

[onEXHovia]

👋 It would be great if it was in the kernel, data prefix is redundant for custom elements.

[keithamus]

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:

@controller
class FooBar extends HTMLElement {
  @attr slot = 1
}

^ 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:

@controller
class FooBar extends HTMLInputElement {
  @attr autocomplete = ‘foo’ // Oops! Accidentally clobbered input[autocomplete] semantics!
}

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!

[onEXHovia]

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]

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.

[onEXHovia]

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.

[keithamus]

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.

[onEXHovia]

Using the example of Autocomplete Element, it is difficult to come up with a 2-word propertry name.

@controller
class AutocompleteElement extends HTMLElement {
  @attr src: string;
  @attr for: string;
}

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.

[keithamus]

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
@controller
class AutoCompleteElement extends HTMLElement {

  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.

[onEXHovia]

@keithamus looks great, its would be useful. Wanted continue the discussion on the name properties of 2 words with an example

document.addEventListener('click', (event: Event) => {
  ....
  const target = event.target.closest('route-page');
  if (target instanceof RoutePageElement) {
    target.forward();
  }
});

@controller
class RoutePageElement extends HTMLElement {
  @attr path: string;

  forward(): void {
    window.location.href = this.path;
  }
}

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>

[keithamus]

At the risk of pedantry path exists 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 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.

[keithamus]

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]

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]

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.

[chilimangoes]

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.