Always enqueue an element regardless of callback

[annevk]

Tests: ...

Fixes WICG/webcomponents#760.

---

/acknowledgements.html ( diff )
/custom-elements.html ( diff )

[annevk]

Is this correct? This is still an early return... An alternative would be to set a flag here to only skip the next step, which is a lil ugly, but okay.

[rniwa]

WebKit's implementation always enqueues the element regardless of which attribute is observed. But perhaps that's not as bad as connected/disconnected case since the observedAttributes is the only thing that controls whether an attribute change is observed or not.

[annevk]

Yeah okay, so this should not be observable?

[rniwa]

It IS observable. When mutating an attribute that's not being observed would invoke custom elements reactions at that point if the mutation was done inside a connected callback of another element, for example.

[annevk]

Oh, right, sigh. Okay, I think in that case we should always enqueue, and add tests for both scenarios.

[rniwa]

So this would incur quite a bit of performance cost because then every DOM attribute mutation even ones that are not observed would result in the element to be inserted into a custom element reaction queue. I don't think we want to do that.

[annevk]

Right, I was thinking that too. However, it's hard to square that with sometimes making the order of callbacks predictable and sometimes not. Shouldn't we then maintain the status quo of the standard?

[rniwa]

I'm fine with that as long as other browser vendors are okay with it.

[tkent-google]

I'm fine with this change.
I think the performance penalty by this isn't large.

[annevk]

cc @rniwa

[annevk]

I've now made it so that it always enqueues and added a slightly modified version of @tomalec's example.

@rniwa are there tests for this already?

cc @tkent-google

[chrisdavidmills]

Docs needs recorded on MDN content roadmap — https://trello.com/c/j2MktaWx/133-custom-elements-api

[domenic]

LGTM as an implementation of the agreed-upon semantics. Some editorial tweaks suggested.

I'd like @rniwa to confirm that this spec matches WebKit's semantics. It's a bit hard to imagine a bug that switches from one algorithm to another that's quite different (e.g. with a new skip variable).

[annevk]

So per #4127 (review) we're in a bit of a pickle. Options:

1. Maintain the status quo. Likely the most performant due to early exits, but execution order weirdly depends on what callbacks you care about.
2. Make a partial change. Likely still somewhat performant and execution order only partially depends on what callbacks you care about. This doesn't seem acceptable.
3. Make the change as the PR stands today. Unclear how bad this is for performance. It has some support from Chrome, but no support from Apple.

It seems 3 would require some kind of measurement to be done to make everyone comfortable it's okay. I hope everyone agrees 2 isn't worth it.

[rniwa]

?? I just said that I'm fine with the change as long as other browser vendors are fine.

[annevk]

I see, I thought you said you were fine with maintaining the status quo. 😊

Okay, great, if everyone is onboard the main blocker here is tests.

[rniwa]

Hm... I'm having second thoughts about this. If we made this change, presumably, all new reaction callback we would add should do the same. But that would mean that we would enqueue a custom element into the reaction queue at timings we currently don't do. That seems like a serious forward compatibility issue to me.