[IDEA] deferred upgrade for Custom Elements. #559

trusktr · 2016-08-29T20:24:33Z

I'm not sure if upgrade is async in the new spec, but in v0 in Chrome it appears not to be (at least that is my guess based on the hacks I've described in #558.

Here I propose that Custom Element class registration should not cause immediate upgrade, and should wait for the next turn, at which point the engine can upgrade elements in depth-first order with some guarantee that all the necessary classes have already been registered.

I believe that this would prevent me from requiring the hacks described in #558.

trusktr · 2016-08-29T20:32:15Z

Does the issue make sense? Let me know if I need to explain it differently. Essentially, _I never have any problem if my elements don't have to be upgraded_ (i.e. the classes are registered before the elements are ever created). The problems arise when the elements are created before the classes are registered, in which case they need to be upgraded and that means _causes some of my library code to execute before all the classes have been registered and therefore the code can fail because the custom element classes are minimally coupled and depend on things existing on all the elements, where those things are defined in multiple custom element classes_.

trusktr · 2016-08-29T20:59:19Z

Here's the problem explained as simply as I can, and with possibly incorrect assumptions as I'm not completely familiar with Chrome's v0 implementation:

Suppose I wish to register two element classes, SomeElement and OtherElement. Then, suppose the pre-existing markup exists:

<some-el>
  <other-el>
  </other-el>
</some-el>

Then, the following JS is executed:

document.registerElement('some-el', SomeElement)
document.registerElement('other-el', OtherElement)

In an attempt to describe the problem I see from observing Chrome's v0 behavior, it seems that the first call to document.registerElement('some-el', SomeElement) causes some-el elements to be upgraded and their attachedCallback methods to be fired before the registration of OtherElement, so the children of the some-el will not be of the correct type when attachedCallback is called. By deferring logic with the setTimeout hack shown in #558, I can run logic after the children have also been upgraded to be of type OtherElement (after the execution of document.registerElement('other-el', OtherElement)).

I don't have a simple reproduction, but will post back if I do.

rniwa · 2016-08-29T21:00:33Z

We've visited this idea before and the problem is that we'll delay the painting of a custom element during parsing until all of its children are parsed, which is not acceptable.

trusktr · 2016-08-29T21:13:49Z

@rniwa Although I run document.registerElement calls in the <head> of the <html> document and while the body contains

<some-el>
  <other-el>
  </other-el>
</some-el>

then when the attachedCallback on some-el is called, it will be able to see the non-yet-upgraded child other-el, which leads me to believe that when attachedCallback of some-el is fired that the tree has already been parsed, otherwise how can the children of some-el be visible inside it's attachedCallback? It also seems that the upgrades happen in pre-order, so some-el is upgraded and it's attachedCallback fired, then other-el is upgraded and it's attachedCallback fired.

However, if the elements are attached into DOM after the class registrations, then the attachedCallbacks will fire in post-order, in which case everything works fine without hacks.

So, it seems there just needs to be some type of mechanism that can guarantee that attachedCallbacks are always fired in post-order (depth-first), and it seems that for this to happen that the upgrades should be delayed until all classes are registered.

But, I'm not even sure why if I run my document.registerElement calls in the <head> why the upgrades happen in pre-order. Is this because of the parsing, as in

<some-el> <-- when the parser reaches here, it upgrades `some-el`?
  <other-el> <-- then when it reaches here it *first* calls attachedCallback on some-el, *then* upgrades other-el?
  </other-el>
</some-el>

domenic · 2016-08-29T21:15:24Z

I would really urge you to stop making assumptions based on old implementations, and actually read the spec. There are so many unfounded assumptions here in these (very long) posts, that I don't have time to walk you through where they are wrong.

trusktr · 2016-08-29T23:00:20Z

@domenic I will strive to do that.

Here is simple reproduction of the problem on jsfiddle (tested in Chrome): https://jsfiddle.net/dgbqdL33 (look at console output).

In TEST CASE 1, how would we wait for the child elements to be upgraded when the logic in the parent will depend on the child being instanceof the upgraded type? In TEST CASE 2, it works fine because the elements are added to DOM after the class registration.

Is this fixed in v1?

domenic · 2016-08-29T23:01:47Z

I'd suggest you trying it in Chrome Canary with v1 syntax and so on. Or read the spec.

trusktr · 2016-08-29T23:18:08Z

@domenic The same problem persists using v1 API in Chrome Canary (see console output): https://jsfiddle.net/wkwmzLwL.

This is problematic because a parent element may need to have branching logic depending on which type of custom-element children are detected. f.e.

class SomeElement extends HTMLElement {
    connectedCallback() {
        for (let child of this.children)
            if (child instanceof SomeElement)
                // ...
            else if (child instanceof OtherElement)
                // ...
            else if (child instanceof AnotherElement)
                // ...
    }
}

This can be solved using a timeout hack, so the code would become

class SomeElement extends HTMLElement {
    connectedCallback() {
        setTimeout(() => {
            for (let child of this.children)
                if (child instanceof SomeElement)
                    // ...
                else if (child instanceof OtherElement)
                    // ...
                else if (child instanceof AnotherElement)
                    // ...
        }, 0)
    }
}

Here's a fiddle that shows that the hack works (see output): https://jsfiddle.net/aqp4uaja

rniwa · 2016-08-29T23:28:57Z

I think this problem also goes away if we add back childrenChangedCallback.

rniwa · 2016-08-29T23:32:01Z

For now, you can work around it by notifying parent element from a child element's connectedCallback and disconnectedCallback.

trusktr · 2016-08-30T06:26:25Z

@rniwa

I think this problem also goes away if we add back childrenChangedCallback.

True, if that callback would be fired when an element is upgraded, at which point we can run an instanceof check successfully.

For now, you can work around it by notifying parent element from a child element's connectedCallback and disconnectedCallback.

But as I mentioned earlier, that (child-to-parent notification/observation) doesn't work with distribution into closed shadow trees, which is why I've changed my API to be parent-to-child which should always work, just that I've had to do some hacks like with setTimeout above. Even with a closed tree, a parent can look into a <slot> and notify children, but it doesn't work the other way around, so I've settled with only parent-to-child notification/observation.

rniwa · 2016-08-31T07:03:03Z

Perhaps what we need is an event that fires when a node gets assigned to some slot? Sort of the counterpart of slotchange event which fires on the assigned node instead of the slot to which the node was assigned.

trusktr · 2016-08-31T23:34:47Z

@rniwa I think that would be handy! That is similar to the idea I had with slottedCallback. Maybe having both would be optimal, because slottedCallback can only be taken advantage of with Custom Elements, while something like a slotted event can be observed on any element without having to write a custom element.

trusktr · 2016-08-31T23:58:44Z

@rniwa

I think this problem also goes away if we add back childrenChangedCallback.

Which criteria would cause a childrenChangedCallback to fire? f.e. when a child is connected, disconnected, upgraded. Anything else? An attribute change could be considered a change. Maybe it is better to make specific callbacks for specific changes. f.e. childUpgradedCallback, childConnectedCallback, childDisconnectedCallback, childAttributeChangedCallback, etc.

annevk · 2018-02-17T12:42:27Z

@trusktr if you still think you need such events, could you explain them clearly in a new issue? This one is rather convoluted and hard to digest. Thanks.

trusktr · 2018-02-18T02:24:32Z

@annevk Sure, made a new issue here: #737

trusktr changed the title ~~[IDEA] Asynchronous upgrade.~~ [IDEA] deferred upgrade. Aug 29, 2016

trusktr mentioned this issue Aug 29, 2016

How to wait for will-be custom elements to be upgraded? #558

Closed

trusktr changed the title ~~[IDEA] deferred upgrade.~~ [IDEA] deferred upgrade for Custom Elements. Aug 29, 2016

rniwa added custom-elements v1 labels Aug 29, 2016

domenic added question and removed v1 labels Aug 29, 2016

trusktr mentioned this issue Sep 1, 2016

[idea] childConnectedCallback and childDisconnectedCallback #550

Closed

annevk closed this as completed Feb 17, 2018

trusktr mentioned this issue Feb 18, 2018

Some sort of guarantee on custom element lifecycle order of events #737

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IDEA] deferred upgrade for Custom Elements. #559

[IDEA] deferred upgrade for Custom Elements. #559

trusktr commented Aug 29, 2016 •

edited

Loading

trusktr commented Aug 29, 2016 •

edited

Loading

trusktr commented Aug 29, 2016 •

edited

Loading

rniwa commented Aug 29, 2016

trusktr commented Aug 29, 2016

domenic commented Aug 29, 2016

trusktr commented Aug 29, 2016 •

edited

Loading

domenic commented Aug 29, 2016 •

edited

Loading

trusktr commented Aug 29, 2016 •

edited

Loading

rniwa commented Aug 29, 2016

rniwa commented Aug 29, 2016

trusktr commented Aug 30, 2016 •

edited

Loading

rniwa commented Aug 31, 2016 •

edited

Loading

trusktr commented Aug 31, 2016 •

edited

Loading

trusktr commented Aug 31, 2016 •

edited

Loading

annevk commented Feb 17, 2018

trusktr commented Feb 18, 2018

[IDEA] deferred upgrade for Custom Elements. #559

[IDEA] deferred upgrade for Custom Elements. #559

Comments

trusktr commented Aug 29, 2016 • edited Loading

trusktr commented Aug 29, 2016 • edited Loading

trusktr commented Aug 29, 2016 • edited Loading

rniwa commented Aug 29, 2016

trusktr commented Aug 29, 2016

domenic commented Aug 29, 2016

trusktr commented Aug 29, 2016 • edited Loading

domenic commented Aug 29, 2016 • edited Loading

trusktr commented Aug 29, 2016 • edited Loading

rniwa commented Aug 29, 2016

rniwa commented Aug 29, 2016

trusktr commented Aug 30, 2016 • edited Loading

rniwa commented Aug 31, 2016 • edited Loading

trusktr commented Aug 31, 2016 • edited Loading

trusktr commented Aug 31, 2016 • edited Loading

annevk commented Feb 17, 2018

trusktr commented Feb 18, 2018

trusktr commented Aug 29, 2016 •

edited

Loading

trusktr commented Aug 29, 2016 •

edited

Loading

trusktr commented Aug 29, 2016 •

edited

Loading

trusktr commented Aug 29, 2016 •

edited

Loading

domenic commented Aug 29, 2016 •

edited

Loading

trusktr commented Aug 29, 2016 •

edited

Loading

trusktr commented Aug 30, 2016 •

edited

Loading

rniwa commented Aug 31, 2016 •

edited

Loading

trusktr commented Aug 31, 2016 •

edited

Loading

trusktr commented Aug 31, 2016 •

edited

Loading