Re: [w3c/webcomponents] [feature request] change when upgrade/connected happens during parsing, so that it is the first event in the following microtask (#787)

Alright, I thought about it, a workaround for using the `MutationObserver` approach and also being able to clean up on `disconnectedCallback` and re-observe on `connectedCallback`,  is to:

- create the observer in the `constructor` so that we get the benefit of it during parsing,
- disconnect and dispose of it in `disconnectedCallback`,
- and in `connectedCallback` create a new observer if one doesn't exist. So `connectedCallback` will handle cases of imperative manipulation of the elements (suppose they are cached and will be re-used by React's (or other lib's) internals), while the `constructor` will handle the parsing case.

A **very big downside** of this approach is that an observer is created even if the node is outside of a tree, and like I mentioned above in the previous comment, I'd rather make it behave the way `connectedCallback` and `disconnectedCallback` do with respect to a those callback being called on a tree only when the tree is inserted into a document.

This is bad because:

- for a tree outside of a document,
- `childConnectedCallback` will be called on parents, while `connectedCallback` will not be called on children.
- This will cause drastically different behavior from nodes that are in a document, and will definitely be a source of bugs that require extra complexity to solve.

The `CustomEvent` approach has the advantage that because it is implemented in `connectedCallback` and `disconnectedCallback`, it happens only when the tree is inserted into a document, which is 👍, so:

- The `connectedCallback` of a child **always** corresponds one-to-one with a `childConnectedCallback` call in the parent.
- Same with `disconnectedCallback` and `childDisconnectedCallback`
- this prevents the above problem.

So taking @rniwa's example, here's an updated version showing the `constructor` + `connectedCallback` approach. Look in the console to see the problem I just described, and you'll see that for the element which are not being placed into the document, the execution of code is imbalanced:

```html
<!DOCTYPE html>
<html>
 <body>
  <pre id="log"></pre>
  <script>
   let instanceCount = 0

   customElements.define(
    'test-element',
    class TestElement extends HTMLElement {
     constructor() {
      super()
      this.instanceNumber = ++instanceCount
      this.changeCount = 0
      this.createObserver()
     }

     connectedCallback() {
      console.log('PARENT CONNECTED CALLBACK')
      this.style = 'display: block'
      this.createObserver()
     }

     disconnectedCallback() {
      this.destroyObserver()
     }

     childConnectedCallback(child) {
      console.log('CHILD CONNECTED CALLBACK')
      this.changeCount++
      this.appendChild(
       document.createTextNode(
        `child changed! Change count: ${
         this.changeCount
        }, Child tag: <${child.tagName.toLowerCase()}>, Child number: ${
         child.instanceNumber
        }`,
       ),
      )
     }

     createObserver() {
      if (this._observer) return
      this._observer = new MutationObserver(changes => {
       for (const change of changes) {
        change.type === 'childList' &&
         change.addedNodes &&
         Array.from(change.addedNodes).forEach(child => {
          if (!(child instanceof Element)) return
          this.childConnectedCallback(child)
         })
       }
      })
      this._observer.observe(this, { childList: true })
     }

     destroyObserver() {
      if (!this._observer) return
      this._observer.disconnect()
      this._observer = null
     }
    },
   )

   setInterval(() => {
    document
     .querySelector('test-element')
     .appendChild(document.createElement('test-element'))

    console.log(' --- Test tree outside of the document:')
    // shows that the MutationObserver still operates on Nodes that are not in a document. :(
    const one = document.createElement('test-element')
    const two = document.createElement('test-element')
    const three = document.createElement('test-element')
    one.appendChild(two)
    two.appendChild(three)
   }, 1000)
  </script>
  <test-element>
   Test1
   <test-element>Test2</test-element>
   <test-element>Test3</test-element>
  </test-element>
 </body>
</html>
```

That's not a nice problem to have. The both APIs should be symmetrical!

-- 
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/787#issuecomment-459065552

Received on Wednesday, 30 January 2019 18:58:46 UTC