Webcomponents: [idea] Make use of `observedAttributes` optional

[trusktr]

I think it might be nice that if the observedAttributes static property is not present on the constructor, then for attributeChangedCallback to behave as in v0, where it is called for all attribute changes.

Unless I misread the spec, there is only a single algorithm that uses observedAttributes in the spec, and that algorithm doesn't specify what happens when it is undefined, it only specifies what happens when it is "not undefined".

Furthermore, Section 2.6 says

When any of [a custom element's] attributes are changed, appended, removed, or replaced, [the custom element's] attributeChangedCallback is run.

noting the keyword "any" in that sentence.

Based on the above two facts, then there is nothing that states what the behavior should be if attributeChangedCallback is defined while observedAttributes is not. In my opinion, it'd be nice for behavior to remain the same as in v0 where attributeChangedCallback is called for every attribute that is changed, so that migration from v0 to v1 is easy. Plus, it's easy to opt in to the new behavior anyways by simply defining the observedAttributes property.

Related (reason why I opened this issue): https://github.com/WebReflection/document-register-element/issues/76

[matthewp]

Isn't the reason for observedAttributes performance?

[JanMiksovsky]

While many things changed from Custom Elements v0 to v1, in upgrading elements to v1, I've been surprised that that dealing with observedAttributes has been, by far, the trickiest issue I've grappled with. It can be tedious and error-prone to keep observedAttributes up to date. I've hit numerous bugs that I've eventually traced to the failure to remember to add a new property to observedAttributes. It also complicates the creation of component mixins or subclasses, which need to dynamically insert the attribute they're defining into a list of attributes observed by the base class.

As @matthewp indicates, the motivation behind observedAttributes was performance. Specifically, it was observed that many apps set the style attribute programmatically with multiple invocations:

this.style.width = newWidth;
this.style.height = newHeight;
this.style.display = newDisplay;
...

Each of those style updates would invoke attributeChangedCallback. To avoid this problem, observedAttributes was introduced.

@trusktr's proposal above would help, but I think it'd fall down in the case, e.g., where a mixin/subclass wants to observe an attribute. If a mixin/subclass defined observedAttributes with an attribute it was interested in, this would suddenly break base class behavior that had depended on no observedAttributes being defined in order to get the default behavior proposed above.

Counter-proposal: all attributes _except standard attributes_ always trigger attributeChangedCallback.

• For custom attributes, it feels like the default behavior should be to invoke attributeChangedCallback.
• It's rare for a component to want to react to a change in a standard attribute (like style), so forcing declaration of that seems acceptable. The use of observedAttributes could be reserved for observing standard attributes.
• The name of the property could change to observedStandardAttributes to reflect the refined semantics. If that member included custom attribute names, those would be ignored.

@domenic If I recall, you were the driving force behind the introduction of observedAttributes. Given the challenges we're having using this member, do you think it'd be possible to refine how it works?

[domenic]

No, I don't think it's possible to change this at this stage.

[JanMiksovsky]

Drat. Okay, thanks.

[trusktr]

On the upside, observedAttributes makes it possible to define a feature in a base class which reflects the defined attributes as properties on the element instance in an official way (as in there is now an official way to specify which attributes an element will have).

Will be playing around with it and seeing how it goes...

[domenic]

Given how observedAttributes was designed, and how the consensus was explicitly to make it mandatory, I'll close this issue. Happy to continue discussing in the closed thread if people are unable to find the meeting notes and GitHub issues documenting that decision on their own.

[rniwa]

I agree, changing the semantics of observedAttribute at this stage would be contentious. Google wanted to special case style and other animated attributes for performance reasons and Mozilla didn't want to special case a single attribute. And then there was an issue of attributeChangedCallback being spammy when there is a lot of attributes or some attribute you don't care keep changing its value. So the compromise was made to always specify a list of attribute to observe.

Defining the list of _standard attributes_ is hard because the list is not finalized. As more attributes are added to the standard, components which relied that those attributes to be not standard will fail to observe the attribute value changes even if the attributes only had a meaning in some builtin elements. As such, this approach poses a serious forward compatibility concern.

[WebReflection]

I think having observedAttributes either an Array or a RegExp would solve pretty much every issue raised in here. "catch-all" would be as easy as /.*/ and it's possible to filter out stuff like /^(?!data-)/ or /^(?!style$)/ to rule out style only.

If the attribute name matches the RegExp then the method is triggered.

The current Array could be also represented as RegExp:

["my-attr", "my-other"]
/^(?:my-attr|my-other)$/

For performance concerns, if any faster, developers can still use the Array and let browsers optimise it as possible.

My 2 cents

[rniwa]

I don't think we want to be evaluating regular expressions against attribute names whenever its value changes. That would be unacceptably expensive.

[WebReflection]

Agreed, and I wasn't suggesting that. Implementors can whitelist attributes by name once, instead of each time, so that you'd evaluate only first time it matches the RegExp and from that time on use the list.

In better words: the RegExp filters once the list of attributes.
It'd be like "JIT" so a bit slower, if a RegExp is used instead of an Array, but "_fast as_" after first test.

On specs level it should probably be simpler, but AFAIK specs don't care much about optimisation details.

[rniwa]

I'm not sure what you mean by whitelist. Basically, anytime an attribute is added, we have to evaluate the regular expression. The alternative is to cache the list of attributes we've matched in the past, which can increase the memory footprint. I don't think either approach is acceptable.

[WebReflection]

Basically, anytime an attribute is added, we have to evaluate the regular expression

Only the first time. Meta-speaking

if (!_whiteList.contains(newAttribute)) {
  if (!_blackList.contains(newAttribute)) {
    if (observedAttributes.test(newAttribute)) {
      _whiteList.push(newAttribute);
    } else {
      _blackList.push(newAttribute);
    }
  }
}

After the first time, you'll do that also only if the name is unknown/different which is usually not common.

Of course performance would be penalised compared to just first check but like I've said, we could keep the current Array when performance are a concern.

Mine was just an idea, it's surely more work for vendors and specs author, but it'd probably make developers happy giving them the ability to intercept whatever they want if/when they want.

I personally wouldn't mind keeping as it is, but I understand other developers needs.

[Mr0grog]

As a web developer, @WebReflection's idea seems like a pretty neat and useful way to split the difference between performance and flexibility here.

I definitely appreciate that using an expression of some sort (whether a regex or something else) certainly has a little overhead, but am curious as to whether the memory cost of caching the results is really that high—since the list is specific to the custom element _type_, not the instance, it doesn’t _seem_ like that’d be a really huge cost. And wouldn’t the cache effectively be the same as just prepending a matched attribute name to the list anyway? Would it be any more expensive (memory-wise) than an author providing a list of 10 strings rather than a list of two expressions? (I _do_ get than this means using mutable, non-fixed length structures, but I have to imagine there are lots of opportunities for smart optimizations, too.)

And if we can expect people to use a similar set of attributes on most instances of an element (again, this _seems_ like a reasonable assumption to me), wouldn’t we be likely to arrive at a relatively fixed list of strings that pretty much always matches before falling back to one of the provided expressions pretty quickly?

[trusktr]

I think having observedAttributes either an Array or a RegExp would solve pretty much every issue raised in here. "catch-all" would be as easy as /.*/ and it's possible to filter out stuff like /^(?!data-)/ or /^(?!style$)/ to rule out style only.

With caching, I think this could be useful, and adding it to spec later would remain backwards-compatible. It would also be opt-in, so if an array is provided then no extra memory footprint for the cache list.

Although I like the idea of using RegExp for observedAttributes, extending a super class's observedAttributes then becomes tricky (imagine super observedAttributes is a list, and the child class wants to have a RegExp).

Would it be any more expensive (memory-wise) than an author providing a list of 10 strings rather than a list of two expressions?

The phrase "list of two expressions" makes me think: maybe observedAttributes can be a list as now, but can also contain RegExps as well (with caching as mentioned). In this case, extending super isn't so bad.

Defining the list of standard attributes is hard because the list is not finalized. As more attributes are added to the standard, components which relied that those attributes to be not standard will fail to observe the attribute value changes even if the attributes only had a meaning in some builtin elements. As such, this approach poses a serious forward compatibility concern.

From a design-pattern perspective, would it be more web-manifesto-friendly for "global" attributes to be observedAttributes on a base class that all elements extend from, like HTMLElement? That would be a breaking change after v1, but I feel like it makes sense in order for things to be explainable rather than just magic. The downside is that calling super would be required for all custom elements or the custom element would no longer observe things like style:

class MyElement extends HTMLElement {
  static get observedAttributes() {
    return super.observedAttributes().concat(...)
  }
}

[trusktr]

After reading your responses on the original topic, I am okay with current behavior. I should also be less nit picky. :}

[WebReflection]

extending a super class's observedAttributes then becomes tricky

if really needed, there are ways.
Following a very basic example

class MyElement extends HTMLElement {
  static get observedAttributes() {
    return new RegExp("^my-el-|" + super.observedAttributes().source);
  }
}

[trusktr]

@WebReflection But in that example we're assuming that super.observedAttributes() is a RegExp, but it may also be an array.

[trusktr]

So I think requiring observedAttributes to be an array -- possibly containing RegExp instances -- would make assumptions easier. Then the pattern can just always be:.

class MyElement extends HTMLElement {
  static get observedAttributes() {
   return super.observedAttributes().concat(/* your new stuff here */)
  }
}

[WebReflection]

like I've said, I'm OK with current status and extendability through .concat but you know what you extend so that if super has no super.observedAttributes you'll have an error trying to concat nothing.

In few words, we are making up inexistent problems because reality would be like the following:

• if super is Array and you want RegExp you can new RegExp("your-thing|" + super.stuff.join('|')) (simplification)
• if super is Array and you want extend you use .concat
• if super is RegExp and you want extend either copy super.observables and enrich it without even calling super or use new RegExp
• if super is RegExp and you want Array, you still new RegExp(['your', 'cases'].join('|') + super.stuff)

Since people will abuse these things and it's easy to get it wrong, I guess we'll stick with current status and that's it, it's like good old Object.prototype.watch where either you know upfront or you cannot spy on everything like a proxy would do.

[Mr0grog]

FWIW, I don’t feel particularly concerned about the extendability issue—I think the status quo for extension is fine. That said, I think making observedAttributes always a list (of strings and/or expressions) would be better than a list-of-strings-or-one-expression for two reasons:

• Extending an existing regular expression isn’t necessarily simple. For example, what if a super class uses u flag on the expression but you don’t want to on the subclass or vice-versa? Adding another expression onto a list avoids subtle errors people could have here.
• Extension in general, even if just adding more strings, is straightforward. Instead of handling four possible cases as above, there’s only one: concat() onto the list.

I think supporting some kind of expression would also be good in so far as it “explains the platform” for data-* attributes / the dataset property, which I don’t think could be handled through custom elements as-is right now (please correct me if I’m wrong on that).

[treshugart]

I think if we wanted to maximise potential, making it a callback that returns true if it is to be observed, might be a good idea.

[rniwa]

We definitely don't want to turn this into a callback. The whole point of this property is to avoid invoking JS whenever possible in the engine code.

[allenhwkim]

For my case, I ended up with building my own instead of using observedAttributes and attributeChangedCallback() because;

1. I wanted to observe all attribute changes instead of few.
2. and, the list of attributes to observe are not limited, but dynamic. (e.g. google maps attribute.)

What I do is to simply call this from connectedCallback

function observeAttrChange(el, callback) {
  var observer = new MutationObserver(mutations => {
    mutations.forEach(mutation => {
      if (mutation.type === 'attributes') {
        let newVal = mutation.target.getAttribute(mutation.attributeName);
        callback(mutation.attributeName, newVal);
      }
    });
  });
  observer.observe(el, {attributes: true});
  return observer;
}

connectedCallback() {
    observeAttrChange(this, ....);
}

[trusktr]

@rniwa, your previous thought of avoiding invoking JS is not really solving performance. Instead of the end user's conditional statements running logic for certain attributes, we've merely moved that logic into the HTML engine, so now the HTML engine has to conditionally check observedAttributes in order to fire the end user's attributeChangedCallback. This practically does nothing to improve performance, because regardless of whether the HTML engine checks observedAttributes or not, the user will still have some code like follows:

attributeChangedCallback (a, o, n) {
  if (a == 'foo') { ... }
  else if (a == 'bar') { ... }
  else if (a == 'baz') { ... }
  // etc...
}

So the only thing being saved is some extra no-op function calls that will do nothing because most people will already be checking conditionally for certain attributes.

Is there really a significant gain from observedAttributes that outweighs the burden of having to remember to manually list all observed attributes? Are there benchmarks somewhere?

[annevk]

Yes there is. Going from C++ to JavaScript is expensive.

[rniwa]

Yeah, calling out to JS at all is extremely expensive. The fact we have to remember the old attribute value and queue things up when multiple attributes are modified, etc... all add up to something like 3x overhead the last time I measured.

[trusktr]

What if someone wants to observe any attributes without knowing what they are in advance? Would you prefer for them to reimplement attribute observation like @allenhwkim had to above?

[rniwa]

What are use cases in which you want to observe changes to arbitrary set of attributes?

[allenhwkim]

@rniwa Common use cases are building an element that wraps an visual object; map, chart, drawing, etc.

Assuming your are building a <google-map> custom element, and you are accepting all map properties; more than 30 properties, through attributes; e.g., center, zoom, etc, Then, you also want to monitor the attribute change and update the map when it is changed. It would be ideal to handle it dynamically rather than listing 30+ attributes(https://developers.google.com/maps/documentation/javascript/reference#MapOptions).

It's not exactly related to this issue, but I have tried to write a google map on this article. https://medium.com/allenhwkim/back-to-element-c4aecf3c6b64

[rniwa]

Assuming your are building a <google-map> custom element, and you are accepting all map properties; more than 30 properties, through attributes; e.g., center, zoom, etc, Then, you also want to monitor the attribute change and update the map when it is changed.

That doesn't involve observing all content attributes. What you need is simply a dynamic filtering of a pre-determined set of content attributes. You can do that using observedAttributes and then applying any filtering mechanism.

[allenhwkim]

@rniwa Is there a way to use observedAttributes without listing 30+ attributes within the getter?

[rniwa]

@rniwa Is there a way to use observedAttributes without listing 30+ attributes within the getter?

No, you should list those attributes. That would be still much cheaper than having to serialize the style attribute whenever the inline style is changed.