[w3c/webcomponents] CE reaction queue's not-really-sync behavior doesn't provide strong enough guarantees about when reactions occur (#616)

The custom elements reaction queue is difficult to reason about because nested queue flushing can cause callbacks to be run even when the information those callbacks are meant to convey is no longer valid.

[This gist](https://gist.github.com/bicknellr/567661eff48bb130add3bb1be86e37e2) shows a situation with two elements, one the parent of the other, where the parent sets an attribute on the child during its `connectedCallback` but the child removes that same attribute during its `connectedCallback`. The inner element's `attributeChangedCallback` is called with incorrect values because a pending `connectedCallback` reaction in the queue causes calls to functions with `[CEReactions]` to be 'delayed' until subsequent. [This gist](https://gist.github.com/bicknellr/a964b91fcbae51df5361085b5fd666e2) is the same as the last but has lots of extra logging. Here's the output:

```
 1 CALL fragment = document.importNode(template.content, true)
 2 EXIT fragment = document.importNode(template.content, true)
 3 CALL document.body.appendChild(fragment)
 4 | START <x-outer> connectedCallback
 5 | <x-outer>.children[0].getAttribute('a'): null
 6 | CALL <x-outer>.children[0].setAttribute('a', 'foo')
 7 | | START <x-inner> connectedCallback
 8 | | <x-inner>.getAttribute('a'): foo
 9 | | CALL <x-inner>.removeAttribute('a')
10 | | | START <x-inner> attributeChangedCallback ( name: a , old: null , new: foo )
11 | | | <x-inner>.getAttribute('a'): null
12 | | | END <x-inner> attributeChangedCallback
13 | | | START <x-inner> attributeChangedCallback ( name: a , old: foo , new: null )
14 | | | <x-inner>.getAttribute('a'): null
15 | | `-END <x-inner> attributeChangedCallback
16 | | EXIT <x-inner>.removeAttribute('a')
17 | | <x-inner>.getAttribute('a'): null
18 | `-END <x-inner> connectedCallback
19 | EXIT <x-outer>.children[0].setAttribute('a', 'foo')
20 | <x-outer>.children[0].getAttribute('a'): null
21 `-END <x-outer> connectedCallback
22 EXIT document.body.appendChild(fragment)
```

The most obviously confusing (ha) thing going on here is what happens starting from the parent's call to `setAttribute` of the child (L6 above). When this call is made, an `attributeChangedCallback` reaction is enqueued but the child's queue already contains a pending `connectedCallback` reaction and causes that reaction to be called first. Then, as mentioned earlier, the child removes the attribute `a` from itself during its `connectedCallback`. However, when the child calls `removeAttribute` (L9; has `[CEReactions]`), the `attributeChangedCallback` reaction setting `a` to `'foo'` that was added shallower in the stack is already in the queue and is called next (L10). So, the child's `attributeChangedCallback` ends up being called with arguments `['a', null, 'foo']` even though `this.getAttribute('a')` inside that callback returns `null` because the last thing to actually happen to attribute `a` was that it was removed!

What this means is code running in `attributeChangedCallback` can't assume that the values provided to it are in any way representative the state of that element when the callback is called. If we continue assuming the specced behavior to be 'correct', for the moment, then custom elements' `setAttribute` inherently becomes an asynchronous operation. A response to this might be, "No, the `attributeChangedCallback` for any given `setAttribute` or `removeAttribute` call will occur synchronously before that call has completed.", and this would be correct.

However, the real problem arises when implementing `attributeChangedCallback`. If `attributeChangedCallback` isn't guaranteed to be called synchronously, then, to avoid doing incorrect work in response to a callback that may have already been invalidated, the element must wait. Additionally, there is no way to know how long to wait: waiting until `this.getAttribute(name) === newValue` isn't necessarily sufficient because there may be other attribute changes pending (or soon to be triggered) that eventually cycle `name`'s value back to `newValue` but occur after something that would change how the element responds to `name`'s value becoming `newValue`. This means the earliest safe time for the element to act is after there are no more frames of author JS on the stack, which can only be best approximated with a microtask. So, because the element can't safely act synchronously *because* can't know that its state has stabilized until at least the next microtask, `setAttribute` has effectively become async.

IMHO, this makes working with custom elements' attributes unusably cumbersome because every custom element must now supply some asynchronous signal to indicate that the element has finished acting in response to a given call to `setAttribute` or `removeAttribute`. Also, every user of a custom element must know and use these async signals whenever changing attributes of a custom element or initially interacting with a custom element, if that element was created by the parser. I feel confident saying that these signals will differ between custom element authors and even amongst the same author's elements.

**This problem is not limited to `attributeChangedCallback`.** [This gist](https://gist.github.com/bicknellr/2f5e1c0bda51f4c63579aa771d75af72) shows a very similar situation where `connectedCallback` of an element is called when the element is disconnected.

I think the justification for having the custom elements reactions queue rather than making these actions synchronous needs to be reconsidered. Even with a large number of elements to be assembled, is natively testing a boolean when building the tree ("Is this a custom element?") really that expensive? And, as far as potentially exposing intermediate state goes, it seems like this problem is actually a *result* of the queue. For example, if I wanted to build this tree:
```html
<x-outer a="1">
  <x-inner b="2"></x-inner>
</x-inner>
```
I could do this:
```javascript
var outer = document.createElement('x-outer');
outer.setAttribute('a', '1');

var inner = document.createElement('x-inner');
inner.setAttribute('b', '2');
outer.appendChild(inner);

document.body.appendChild(outer);
```
... and the callbacks would be called in a sane order and the state of the tree also makes sense: `inner` would have already responded to `b` changing to `'2'` before it is appended to the tree, when `outer` might potentially call `inner.setAttribute('b', somethingElse)`. Why should cloning a template be any different?


-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/w3c/webcomponents/issues/616