Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
treshugart opened this issue Aug 8, 2017 · 24 comments

Comments

@treshugart
Copy link

commented Aug 8, 2017

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

This comment has been minimized.

Copy link
Contributor

commented Aug 8, 2017

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

This comment has been minimized.

Copy link
Author

commented Aug 8, 2017

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

This comment has been minimized.

Copy link
Member

commented Aug 8, 2017

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

@sebmarkbage

This comment has been minimized.

Copy link

commented Aug 8, 2017

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

This comment has been minimized.

Copy link
Author

commented Aug 8, 2017

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

This comment has been minimized.

Copy link
Member

commented Aug 8, 2017

@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

This comment has been minimized.

Copy link
Member

commented Aug 8, 2017

@treshugart well, the validator discourages it, no?

@treshugart

This comment has been minimized.

Copy link
Author

commented Aug 8, 2017

@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

This comment has been minimized.

Copy link
Member

commented Aug 8, 2017

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

This comment has been minimized.

Copy link
Author

commented Aug 8, 2017

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

This comment has been minimized.

Copy link

commented Aug 8, 2017

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

This comment has been minimized.

Copy link

commented Aug 8, 2017

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

This comment has been minimized.

Copy link

commented Aug 16, 2017

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

This comment has been minimized.

Copy link

commented Oct 25, 2017

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

This comment has been minimized.

Copy link

commented Oct 25, 2017

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

This comment has been minimized.

Copy link

commented Nov 3, 2017

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

@rniwa

This comment has been minimized.

Copy link
Contributor

commented Nov 3, 2017

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

This comment has been minimized.

Copy link

commented Nov 3, 2017

@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

This comment has been minimized.

Copy link
Contributor

commented Nov 3, 2017

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

This comment has been minimized.

Copy link

commented Mar 5, 2018

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

This comment has been minimized.

Copy link
Author

commented Mar 5, 2018

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

@rniwa

This comment has been minimized.

Copy link
Contributor

commented Mar 5, 2018

Again, we reached no consensus on F2F in Tokyo.

@arnog

This comment has been minimized.

Copy link

commented Jun 28, 2019

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

This comment has been minimized.

Copy link
Member

commented Jun 28, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
You can’t perform that action at this time.