WICG / webcomponents

Web Components specifications
Other
4.36k stars 370 forks source link

Custom attribute names conflicting with built-in attribute names #654

Open treshugart opened 7 years ago

treshugart commented 7 years ago

Say I have a custom element that responds to the hidden attribute. This custom element may hide itself in a particular way, or have a particular semantic meaning for "hidden" in this context that is different to "the browser should hide this element".

class MyElement extends HTMLElement {
  static observedAttributes = ['hidden']
  attributeChangedCallback (name, oldValue, newValue) {
    // go do some hidy stuff
  }
}

Now let's say this element is in production, correctly responding to the semantics of its custom hidden attribute. Then along comes a new, standardised, attribute with of the same name in what normally would be an innocuous change. The new attribute takes the element and hides it immediately. All of our custom elements that implement our own semantics for this attribute are now broken.

This example is somewhat contrived in the context of HTMLElement because hidden is fairly concrete behaviour. This would be more likely to happen with customised built-ins. That said, this is still possible at the HTMLElement level with more abstract use cases.

One could say that this isn't limited to custom elements as people have been doing this with existing component systems for a long time (jQuery plugins, Moo Tools, Angular, etc.). Previously, people have been encouraged to use data prefixed attributes (maybe not with certain component models such as Angular which have seen similar issues). With the arrival of custom elements, people have been encouraged to use non-prefixed attribute names.

I'd really appreciate a discussion on how we can prevent this from happening at the standards level so custom element authors don't have to worry about this.

The origination of this issue was a discussion started by @sebmarkbage. I don't have any prior art to offer, but maybe others do.

domenic commented 7 years ago

With the arrival of custom elements, people have been encouraged to use non-prefixed attribute names.

Why do you think that is the case? We've loosened the validation rules, but I don't believe we've encouraged using such attribute names.

treshugart commented 7 years ago

Why do you think that is the case? We've loosened the validation rules, but I don't believe we've encouraged using such attribute names.

My language could have been better. People haven't been discouraged from using non-prefixed names, and I'm not pointing fingers at the working group or any group in particular. Every example I've ever seen (and that's quite a bit) has always used un-prefixed names (unless using dashes to separate words).

Validation rules aren't the issue here. The issue is if a conflicting name is added to the spec. It could unknowingly break an incalculable number of custom elements in the wild.

annevk commented 7 years ago

Yeah, folks shouldn't add custom attributes without using data-*. Loosening that up is https://github.com/whatwg/html/issues/2271. I don't think we need to track this here.

sebmarkbage commented 7 years ago

I think that's commonly understood for existing tags. This is about custom elements. I don't think that's the common understanding nor what people do in practice for custom elements.

The spec itself includes an example for adding the "country" attribute.

treshugart commented 7 years ago

I like the proposal for custom attributes, but this feels a bit different. Attributes are declarative properties that can only accept strings. Many times these are linked (even if one-way) to a property with a very similar - if not same - name. Props are the imperative API (unless you have a declarative abstraction on top of them).

Props behave differently to attributes in the fact that you can override them and callback to the parent if needed (super.someProp = value). Props are resilient to the described problem because of this. Adding a conflicting prop to one that a custom element defines would not break a custom element (unless it was defined as non-configurable, of course).

Attributes could benefit in the same way if for observedAttributes you were required to call super.attributeChangedCallback() in order to get the overridden behaviour. Unfortunately this requires this be implemented at the HTMLElement level and this probably would be difficult to implement at the browser level. I'm probably not thinking of something else, but hopefully that makes sense.

If we really shouldn't be using non-prefixed attributes, there needs to be something that discourages this.

annevk commented 7 years ago

@sebmarkbage that example seems like a bug. The problem with allowing those kind of attributes is that they'll clash with (super)global attributes (both existing and new). E.g., for shadow trees we've added slot and we'll add part too at some point. They need some kind of differentiation.

annevk commented 7 years ago

@treshugart well, the validator discourages it, no?

treshugart commented 7 years ago

@annevk https://validator.w3.org says this is valid:

<!DOCTYPE html>
<html>
  <head>
    <title>test</title>
  </head>
  <body>
    <x-element country="au"></x-element>
  </body>
</html>
annevk commented 7 years ago

Oh, I guess that is allowed then (and @domenic indeed says as much above). But yeah, that seems likely to cause problems. I suspect the rationale was that we always have to do some research which attribute names are still available when minting a new global attribute name as we're not allowed to break non-conforming content (at least not when it's widespread) either. Backwards compatibility is not limited to the good stuff.

treshugart commented 7 years ago

I suspect the rationale was that we always have to do some research which attribute names are still available when minting a new global attribute name as we're not allowed to break non-conforming content (at least not when it's widespread) either. Backwards compatibility is not limited to the good stuff.

That makes sense now. I wonder if there is some way to allow attributes to behave more like properties or methods as opposed to going down the road of namespacing and discouragement.

sebmarkbage commented 7 years ago

I'm more concerned when subclassing more specific elements becomes common place. Currently implementations are not widespread. Specific elements are more likely to need new attributes.

Normally, when new top level APIs are added, they are shadowed by the page's global variables. When properties are added to prototypes, they're shadowed by subclasses. That's not the case for attributes.

This is also a new scale of the problem. Currently there is some non-conformance as can be expected but not to the extent that custom-elements are going to be used. I had a hard time finding a single component on https://www.webcomponents.org/ that didn't use a non-hyphenated attribute name. At the very least, there's a messaging problem here.

sorvell commented 7 years ago

Adding global attributes to the platform is pretty rare, but it must now be done with the understanding that custom elements exist and should not be broken or overly burdened. Namespacing attributes was not part of the core design of custom elements (as, for example, was naming them with a dash) and therefore is not required or encouraged.

There are a variety of ways to add global functionality like attributes that does not conflict with existing custom elements: (1) it can be added above HTMLElement or a cutout for custom elements can be made, (2) it can be customized/overridden via custom elements features (e.g. attributeChangedCallback, observedAttributes, etc.), and of course (3) additional features (v2 spec) can/should be added to custom elements to allow deeper integration with the platform as it evolves.

strongholdmedia commented 7 years ago

Now let's say this element is in production, correctly responding to the semantics of its custom hidden attribute. Then along comes a new, standardised, attribute with of the same name in what normally would be an innocuous change. The new attribute takes the element and hides it immediately. All of our custom elements that implement our own semantics for this attribute are now broken.

Basically, this is exactly why using the same rules for attribute names as for tag names (as per custom naming) is the worst idea ever, and also why one should not just invent attribute names they deem appropriate.

effulgentsia commented 6 years ago

With the arrival of custom elements, people have been encouraged to use non-prefixed attribute names.

Why do you think that is the case? We've loosened the validation rules, but I don't believe we've encouraged using such attribute names.

folks shouldn't add custom attributes without using data-*.

In addition to what was said in earlier comments about how non-prefixed custom element attributes are already in the wild and valid, I also want to point out that https://html.spec.whatwg.org/multipage/custom-elements.html#custom-elements-core-concepts literally says:

Autonomous custom elements have the following element definition:
...
Content attributes:
Global attributes, except the is attribute
Any other attribute that has no namespace (see prose).

And the prose below that says:

Any namespace-less attribute that is relevant to the element's functioning, as determined by 
the element's author, may be specified on an autonomous custom element, so long as the 
attribute name is XML-compatible and contains no ASCII upper alphas. The exception is the is 
attribute, which must not be specified on an autonomous custom element (and which will have 
no effect if it is).

I'm more concerned when subclassing more specific elements becomes common place. Currently implementations are not widespread. Specific elements are more likely to need new attributes.

There, we're ok, since that same spec says:

Customized built-in elements follow the normal requirements for attributes, based on the 
elements they extend. To add custom attribute-based behavior, use data-* attributes.

Adding global attributes to the platform is pretty rare, but it must now be done with the understanding that custom elements exist and should not be broken or overly burdened.

Does that mean every specifications proposal that proposes adding a global attribute will be constrained by this? For example, https://dvcs.w3.org/hg/undomanager/raw-file/tip/undomanager.html, which proposes an undoscope global attribute?

robdodson commented 6 years ago

Does that mean every specifications proposal that proposes adding a global attribute will be constrained by this?

I think this is already the case as Anne mentioned:

I suspect the rationale was that we always have to do some research which attribute names are still available when minting a new global attribute name as we're not allowed to break non-conforming content (at least not when it's widespread) either. Backwards compatibility is not limited to the good stuff.

For example a popular jQuery plugin or framework could add its own attributes.

robdodson commented 6 years ago

@rniwa I was wondering if you have any thoughts on this discussion?

rniwa commented 6 years ago

Like I've stated in the past, it would be good to standardize what could be considered as a custom attribute and what should be reserved for future global attributes.

robdodson commented 6 years ago

@rniwa sorry didn't realize you'd gone over this before 😁 Do you have a link to that previous discussion? I'm putting together a doc to kick around some ideas of how to resolve this and would love to incorporate any feedback you have.

rniwa commented 6 years ago

I'm sure it's in minutes somewhere but basically we're supportive of coming up with a recommendation on what names are safe to use in custom elements. Others opposed stating that it would have no benefit since anyone could ignore that rule and start using whatever name they like to use.

robdodson commented 6 years ago

Maybe a bad idea, but one option would be to just encourage developers to prefix/suffix their custom attributes with a signifier. I think you can do all of the following without needing to patch setAttribute():

:baz=
baz:=
_baz=
baz_=
baz-=
baz.=
treshugart commented 6 years ago

Noting here that a dash prefix (-test) was proposed in https://github.com/whatwg/html/issues/2271 but would require relaxing the attribute name rules.

rniwa commented 6 years ago

Again, we reached no consensus on F2F in Tokyo.

arnog commented 5 years ago

Since (custom) attributes are not supposed to include upper alphas, a possible solution would be to require that future global attributes do contain at least an upper alpha. For example, a future "undoscope" global attribute would be "undoScope" (or "UndoScope"?). That seems easier to police than retroactively imposing a restriction on the naming of custom attributes of custom elements.

annevk commented 5 years ago

The HTML parser lowercases all attributes and that's not something we can really change at this point.