WICG / webcomponents

Web Components specifications
Other
4.34k stars 373 forks source link

connectedCallback timing when the document parser creates custom elements #551

Closed kojiishi closed 7 years ago

kojiishi commented 7 years ago

From a blink bug, an author reported that connectedCallback is called too early in Blink.

<script>customElements.define('x-x', ...);
<script>customElements.define('y-y', ...);
<x-x>
  <div></div>
  <y-y></y-y>
</x-x>

or a sample code in jsfiddle.

When current Blink impl runs this code, this.children.length is 1 in connectedCallback for x-x. This looks like matching to the spec to me, but I appreciate discussion here to confirm if my understanding is correct, and also to confirm if this is what we should expect.

My reading goes this way:

  1. The parser is in the "in body" insertion mode.
  2. It sees x-x as Any other start tag, it insert an HTML element.
  3. insert a foreign element says to create an element for a token, then insert. Note that this "insert" is not a link, but I assume this is insert a node?
  4. In insert a node, step 5.2.1 enqueues connectedCallback.
  5. In enqueue a custom element callback reaction, it adds to the backup element queue.
  6. The parser then reads div. In its create an element for a token, definition is null, so will execute script is false. This div is then inserted.
  7. The parser then reads y-y. In its create an element for a token, definition is non-null, so will execute script is true.
  8. Step 6.2 perform a microtask checkpoint if will execute script is true, so the connectedCallback runs here.

Am I reading the spec correctly? If so, is this what we should expect?

@domenic @dominiccooney @rniwa

domenic commented 7 years ago

In enqueue a custom element callback reaction, it adds to the backup element queue.

I think you are correct that the spec says this, but this also seems wrong. The backup element queue should be avoidable when we are doing parsing; it should only be used for things like editing. We should be able to just use the element queue created for that element.

That would make connectedCallback be called with zero children, right? That sounds much better.

I am not sure what the best spec fix is for this---it is getting late over here---but maybe it would suffice to just push an element queue in "insert a foreign element", then pop it and run reactions? Better suggestions appreciated.

kojiishi commented 7 years ago

Yeah, 0 sounds much better than 1, and "insert a foreign element" looks the right place.

I also understand web developers might expect 2 instead of 0, but that's harder, wondering whether the benefit is worth the complexity or not.

annevk commented 7 years ago

No. We absolutely want zero children in as many places as possible. The whole point is that you should not depend on children, since you cannot depend on them with createElement() either. Web developers should create robust custom element implementations, not half-baked ones.

kojiishi commented 7 years ago

In constructor, yes, but in connectedCallback, authors can add children before adding the element to a document, no?

annevk commented 7 years ago

Ah yeah they could. But that should not be required by the custom element.

ju-lien commented 7 years ago

I'am the author of the blink bug,

Ah yeah they could. But that should not be required by the custom element.

Could you explain why ? connectedCallback() is called when the node is inserted in a DOM, so i think it should be aware of its own children ?

If not, how/when a CE can work with its children ?

Also it's very bizare that connectedCallback() doesn't have the same capability when the node is created from the parser or upgraded.

annevk commented 7 years ago

When the node is created and inserted, it doesn't have any children.

matthewp commented 7 years ago

So... how do you work with custom elements that have expectations about their children?

annevk commented 7 years ago

Mutation observers.

matthewp commented 7 years ago

So connectedCallback is called with 0 children in the case of upgrades, but if I am to do:

var myEl = document.createElement('my-element');
myEl.appendChild(document.createElement('span'));

var frag = document.createDocumentFragment();
frag.appendChild(myEl);

document.body.appendChild(frag);

Then "my-element"'s connectedCallback is called when the fragment is inserted into the body and it will have 1 children in this case.

So a custom element with expectations on children will need to:

  1. Check if has children in connectedCallback, do stuff
  2. Set up a mutation observer and do stuff when nodes are added (which will happen in the case of upgrades).
  3. Disconnect the mutation observer in disconnectedCallback

No concerns here, just trying to work it out in my head.

annevk commented 7 years ago

If you set up the mutation observer at construction time, that first appendChild() can trigger do stuff too, but if you only want to observe while connected something like that could work.

annevk commented 7 years ago

I meant it can cause do stuff to be scheduled. Mutation observers run end-of-task so not immediately.

rniwa commented 7 years ago

An alternative is to add childrenChangedCallback: https://github.com/w3c/webcomponents/issues/550

ju-lien commented 7 years ago

Mutation observers.

For now in chrome canary ( i don't know if it matches the spec or not), if i try to get all children with an observer even in constructor(), the observer is fired 2 times as you can see in this jsfiddle. So if i just want to have a list of initial children, i don't know when i have to disconnect the observer and do the work?

An alternative is to add childrenChangedCallback: #550

Why not if all initial children are returned in one call.

annevk commented 7 years ago

The whole idea of initial children is flawed. Children are dynamic.

domenic commented 7 years ago

Indeed. In general the multi-tag designs of HTML (e.g. <select> + <option>, <ul> + <li>, <picture> + <source> + <img>, etc.) are very fragile; the amount of spec machinery involved in making them work correctly is extensive and in some cases still buggy in the current spec. (We don't even have a correct design for <li> yet, this many years later: https://github.com/whatwg/html/issues/1617.) Web devs should be very cautious when trying to set up such designs for their custom elements; you need to be aware of how children change dynamically, and not just use the children as some kind of setup data.

domenic commented 7 years ago

It's worth noting that in the past we've talked about adding a new callback for this sort of thing, I think someone called it closeTagReachedCallback or something. That would be useful for emulating <script> behavior, I think.

rniwa commented 7 years ago

FWIW, I would object to adding something like closeTagReachedCallback because they're specific to HTML parser behavior. I'd much rather add something like childrenChangedCallback, which can be used either during DOM mutations or during parsers.

ju-lien commented 7 years ago

Thanks domenic for this complete answer.

I thought indeed to this type of case, an element that sort its children or compute a layout.

For example to create an element that compute a sexy layout, its important to access to an initial state of children to avoid re-compute the layout many times(each time a new CE children is parsed) for just the first render...

So i understand we should avoid to use the children as some kind of setup data, but what is the main technical reason to not to do that ?

domenic commented 7 years ago

For example to create an element that compute a sexy layout, its important to access to an initial state of children to avoid re-compute the layout many times(each time a new CE children is parsed) for just the first render...

What I'd suggest doing is just resorting or re-rendering your children on every requestAnimationFrame. That way you will not only get the post-parsing batching behavior you desire, but you will also synchronize your work with when it's displayed to users.

So i understand we should avoid to use the children as some kind of setup data, but what is the main technical reason to not to do that ?

I thought I explained why that was above, talking about how they are dynamic and can change any time, and this leads to complicated behavior.

ju-lien commented 7 years ago

What I'd suggest doing is just resorting or re-rendering your children on every requestAnimationFrame. That way you will not only get the post-parsing batching behavior you desire, but you will also synchronize your work with when it's displayed to users.

Ok, that's a good idea. Even it's seems a - little - hacky for a brand new API.

I thought I explained why that was above, talking about how they are dynamic and can change any time, and this leads to complicated behavior.

Ok, thanks, it's bizarre but i trust you :)

domenic commented 7 years ago

Ok, that's a good idea. Even it's seems a - little - hacky for a brand new API.

Nah. When do you think browsers update rendering for their elements? (The answer is: every animation frame callback.)

annevk commented 7 years ago

Only if there is something to update though.

ju-lien commented 7 years ago

Sorry to come back but i was checking the SkateJS library and i saw that in the todolist example on the homepage:

...
attached(elem) {
    // Setup the initial list of items from the current children.
    elem.items = elem.children;
  },
...

And attached() seems to rely on connectedCallback().

So i think it show the need to really clarify the situation - or - provide a real solution even requestAnimationFrame() could do the trick.

domenic commented 7 years ago

Yeah, this might be worth someone doing a blog post on or similar.

matthewp commented 7 years ago

A library could provide that hook for you, or conversely a small function could give you the same behavior:

connectedCallback() {
  onChildren(this, children => {

  });
}
treshugart commented 7 years ago

Fwiw you can probably use the slotchange event on slot elements for most things. The skatejs-named-slots polyfill exposes that and makes native v0 behave like v1.

On Thu, 25 Aug 2016, 08:58 Matthew Phillips notifications@github.com wrote:

A library could provide that hook for you, or conversely a small function could give you the same behavior:

connectedCallback() { onChildren(this, children => {

}); }

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/w3c/webcomponents/issues/551#issuecomment-242234361, or mute the thread https://github.com/notifications/unsubscribe-auth/AAIVbAr4qQnVroGn_kDRefmMkbIcRdfuks5qjMyTgaJpZM4Jqj2v .

justinfagnani commented 7 years ago

Not being able to see children in elements at connectedCallback() seems like a pretty big hassle when dealing with wrappers that are needed because of lack of support for customized built-in elements.

For example, where before we could write an extension to customize template behavior:

<template is="fancy-template">fancy stuff</template>

Now we have to write a wrapper:

<fancy-template><template>fancy stuff</template></fancy-template>

And in this wrapper we need to either create a MutationObserver, or wait a microtask, etc. (in addition to any MutationObserve that might have existed on the template's contents). Being able to see children in connectedCallback() makes these wrappers that are now required much simpler to write, and still work with createElement().

domenic commented 7 years ago

It sounds like what you're asking for is not connectedCallback, but a separate feature (like childrenChangedCallback or finishedParsingEndTagCallback). connectedCallback simply fires when the element is inserted into the DOM, and at that time, there are no children.

WebReflection commented 7 years ago

connectedCallback simply fires when the element is inserted into the DOM, and at that time, there are no children.

<my-el>
  <p>child 0</p>
  <p>child 1</p>
</my-el>
<script>
customElements.define('my-el', class extends HTMLElement {
  connectedCallback() {
    console.log(this.children);
  }
});
</script>

If I understand correctly the current status, it's hard to explain it, with or without a blog post, that there are no children in above situation.

domenic commented 7 years ago

Right, that is an upgrade, and happens way way after the element has been inserted into the DOM.

It's these kind of inconsistencies between upgrades and non-upgrades that had people arguing for removing upgrades from the spec entirely. We did not go that route, but perhaps that perspective can be helpful in appreciating that they are rather different things. I agree it is hard to explain, but I think it's still worth keeping upgrades.

(Many other things are different between upgrades and non-upgrades, e.g. in the constructor there are children during upgrades.)

WebReflection commented 7 years ago

e.g. in the constructor there are children during upgrades.

which is why I am personally promoting a different approach such:

class HTMLCustomElement extends HTMLElement {
  constructor() {
    const self = super();
    self.init();
    return self;
  }
  init() {}
}
// so that everyone can extend it via ...
class MyEl extends HTMLCustomElement {
  init() {
    // any logic previously used as
    // createdCallback();
  }
}

Afaik that logic should have children and delegate to connectedCallback / disconnectedCallback the only addEventListener / removeEventListener or direct self node related logic.

Yet this means developers expectations might need a fix for standards they use, which is usually undesired.

WebReflection commented 7 years ago

If I might, and without sarcasm meant, the more I think about this upgrade part of the specs, the more I think it feels like "quantum physics" where it's not clear to consumers (users, developers) what a custom element is, or what it'll become.

The DOM so far never worked this way, I'm not sure an intermediate state is a good addiction to these specs.

Maybe it's easier for implementation purpose so that libraries on top are needed.

Just my 2 cents.

matthewp commented 7 years ago

You can't rely on children in the constructor, that will only be the case for upgrades. Better to have your framework/library implement a hook for when children are ready. A childrenChangedCallback would be nice too, maybe we should take the discussion over there. https://github.com/w3c/webcomponents/issues/550

Mr0grog commented 7 years ago

I apologize for continuing a thread that's closed, but I'm worried I might be confused -- shouldn't there be 2 children (instead of no children) in the connectedCallback in @WebReflection's first example (because it's an upgrade, where the children have already been appended to the element)?

In an upgrade:

  1. (There is an HTMLUnknownElement with children)
  2. Enqueue attribute callbacks
  3. Enqueue the connected callback
  4. Run the constructor (with children)
  5. (some time later) Run the callbacks (attributes and connected) (with children)

vs. when parsing:

  1. Parser hits start tag (so it hasn't seen children yet)
  2. Create an element
    1. Find custom element definition
      1. Run microtasks (which runs any waiting reaction callbacks)
      2. Run the constructor (no children here)
    2. Otherwise make a normal HTML element
  3. Append attributes (and enqueue attribute callbacks if custom element)
  4. Run callbacks (just attributes at this point) (no children yet)
  5. Insert the element
    1. Actually insert, set up parentNode, etc.
    2. Enqueue connected callback
    3. Run any callbacks (e.g. the connected callback) (new in https://github.com/whatwg/html/pull/1707, which solves the initial issue here) (still no children yet)
  6. Parser continues on and hits the start tag for the first child (go back to step 1)

Do I have that right?

In this case, I don't think @WebReflection's second example would help anything or guarantee children are present, right?

As an everyday web developer, this generally makes sense and seems fairly straightforward to me. When parsing, the start tag is the thing that represents the element (not the end tag) and is what triggers construction, attributes, and insertion (in that order), whether an element is custom or not. The end tag is merely a marker that tells the parser where to do the insertion (that is, it's a parser instruction and not really anything to do with the DOM).

Upgrades are the special magic that allows me to attach my behavior to an element that already exists and has been fully set up. (And thank you @domenic and everyone who fought to keep upgrades in the spec; they are incredibly useful to have even when they complicate things.)

I will echo that my naive first reaction is to ask for a finishedParsingEndTagCallback or something, but I ultimately think @annevk's comment that "the whole idea of initial children is flawed" is probably right. While I thirst for a way to unifying parsing and upgrading, having a way to get the "initial children" while parsing still doesn't unify them with the live reality of an element that can have children appended to it later. It's probably better to unify all three cases with ideas like childListChanged callbacks.

WebReflection commented 7 years ago

I don't think @WebReflection's second example would help anything or guarantee children are present, right?

I think I forgot to delay the init invocation so that it should be requestAnimationFrame(this.init.bind(this)) in the constructor.

Although it's easy to solve on library side, I don't think we need an extra method for that.

If knowing about children is mandatory for the component, a MutationObserver added in the connectedCallback should be enough, right?

tnikolai2 commented 7 years ago

As a developer i expect, that all dom structure parsed and accessible in connectedCallback. CEReaction for element must starts only after parent element finished his connectedCallback event. Because parent can fully change his content. connectedCallback must behave as connectedAndParsedAllChildrenCallback.

If component has child nodes but they inaccessible, connectedCallback is absolutelly useless for developers without children content. Now all implementations connectedCallback behave as connectedAndParsedAllChildrenCallback. I dont't understand why new specification modify it.

aaronshaf commented 7 years ago

How I minimize flash of non-upgraded children:

export default class TimeagoElement extends HTMLElement {
  connectedCallback() {
    if (this.querySelector('time')) {
      this.init()
    } else {
      window.requestAnimationFrame(() => {
        this.init()
      })
    }
  }
  ...
}
franktopel commented 5 years ago

As Firefox (Gecko/Quantum) seems to have implemented a different behaviour (children being available in the connectedCallback outright), and as the polyfilled browsers (document-register-element) seem to also grant access to a custom element's children in the connectedCallback, would this be the way to implement it cross-browser with minimal performance impact?

class MyElement extends HTMLElement {
    connectedCallback() {
        this.init();
    }

    init() {
        if (this.children.length) { 
            this.childrenAvailableCallback() 
        } else { 
            setTimeout(this.childrenAvailableCallback.bind(this), 0) 
        }
    }

    childrenAvailableCallback() {
        /* access to child elements available here */
    }
}
WebReflection commented 5 years ago

@franktopel watch out attributeChangedCallback might trigger before that and with available children too.

The upgrading mechanism of Custom Elements is easily a footgun for expectations.

If you define the Custom Element before it's found in the body or after, you have already two different behaviors and different possibility to access their content.

In the former case, it'll break until the whole element has been parsed and all its children known too, in the latter case it'll work without problems right away.

Then you have the procedural way to create a custom element with a class ... and that might land on the DOM very late (or even never) so that any recursive interval/animation frame might leak forever if the node gets trashed before it's appended.

Using MutationObserver also might not work because if the custom element is known and it's defined later, no mutation will happen once upgraded.

Example:

customElements.define(
  'my-early-definition',
  class extends HTMLElement {
    constructor() {
      console.log('early', super().children.length);
      new MutationObserver((records, observer) => {
        observer.disconnect(this);
        console.log('early', this.children.length);
      }).observe(this, {childList: true});
    }
  }
);

document.body.innerHTML = `<div>
  <my-early-definition>
    <p>early</p>
  </my-early-definition>
  <my-lazy-definition>
    <p>lazy</p>
  </my-lazy-definition>
</div>`.replace(/\n\s*/g, '');

Copy and paste above code in a bank page. Read early 0 and early 1.

Now copy and paste the following:

customElements.define(
  'my-lazy-definition',
  class extends HTMLElement {
    constructor() {
      console.log('lazy', super().children.length);
      new MutationObserver((records, observer) => {
        observer.disconnect(this);
        console.log('lazy', this.children.length);
      }).observe(this, {childList: true});
    }
  }
);

Read lazy 1 and that's it.

Play around putting connectedCallback and attributeChangedCallback in the mix and see what happens with observed attributes already defined in the element.

Tl;DR it's complicated

WebReflection commented 5 years ago

P.S. with empty custom elements that might never be a way to understand if these have been initialized or not, 'cause checking childNodes.length won't be enough.

WebReflection commented 5 years ago

P.S.2 .. the early 0 happens only in Safari, not Chrome.

franktopel commented 5 years ago

What a mess. I can't wait to see CustomElements v2 fix this mess and give developers a reliable lifecycle api that meets developer needs and expectations. Tbh I still don't get what Chrome is achieving with the current implementation behaviour, even after two days of messing around with this topic. Why does Gecko/Quantum allow reliable child element access in the connectedCallback, where Blink doesn't? How does the current document-register-element polyfill work in this regard? Where is this going?

WebReflection commented 5 years ago

How does the current document-register-element polyfill work in this regard?

with anything that makes sense, without granting any specific behavior since no browser is behaving the same of others.

The sad story short is that Custom Elements are a mess to setup unless you use shadow DOM which works in the constructor too, but shadow DOM cannot be reliably poly filled and whatever poly around weights already too much and is full of hidden caveats.

... but somebody still wonders how come WebComponents didn't take off as expected 🤷‍♂️

franktopel commented 5 years ago

If I am not mistaken, then including the web components with type="module" fixes the missing children in the connectedCallback in Chrome. ?!?!?? totallyconfusednow

Does type="module" imply a similar parsing behaviour/timing as defer?

rniwa commented 5 years ago

First off, it's literally impossible to have children or attributes before an element is constructed because we can't add children to an element yet to exist. Just think about the way a JS code would construct an element:

const someElement = document.createElement('some-element');
someElement.setAttribute('title', 'This is some element!');
someElement.appendChild(document.createElement('other-element'));

In the case of upgrading, however, the element already exists in the tree, and we're iterating over them to instantiate. In particular, if a custom element definition comes in a deferred script or an async script, then the custom elements that already exist in the document needs to be upgraded.

We could have gone our way to delete attributes & detach from children, but that would have caused serious performance issues in addition to flush of content.

franktopel commented 5 years ago

So what approach is supposed to be used with loads of components in a page of which many rely on their children to setup? I mean, transforming their content which comes as plain HTML from the serverside to something different from my perspective is one of the main use cases for autonomous custom elements, as well as for customized built-ins which usually serve as containers for child elements.

What would be the downsides of the approach suggested in https://github.com/w3c/webcomponents/issues/551#issuecomment-429242035 (regarding components that rely on their children to setup)?

rniwa commented 5 years ago

@franktopel : The problem then is that the element won't get connectedCallback until all children are parsed. For example, if the entire document was a single custom element, that custom element would never receive connectedCallback until the entire document is fetched & parsed even though the element is really in the document. That would be bad.

Alternatively, we could add something like finishedParsingChildrenCallback but then again, any JS could insert more child nodes after the parse had finished inserting children.

In general, the recommended approach is to either use MutationObserver to observe children being added or removed, or to have a child element notify the parent as it gets inserted. That's the only way a custom element would be able to behave correctly when JS, HTML parser, etc... inserts more children or remove some.

domenic commented 5 years ago

FWIW a lot of this was discussed upthread, with some good discussion worth (re-)reading. I am particularly fond of my response from two years ago: https://github.com/w3c/webcomponents/issues/551#issuecomment-241840803 :)

franktopel commented 5 years ago

I did read and try to understand the thread, and some others, too. At the end of the day I'm just a developer who has to take care that the components I develop work reliably for the customer; and at this point in time, I feel unable to achieve that.

One of the concrete use-cases we have (just an example) is a <data-table> component that holds a regular HTML <table> element which needs to be datatableized with a very complex configuration. On top of that, we have the situation that we get asynchronous updates for that table in the form of, again, a <data-table> that has a regular child HTML table which we need to plug in to the page using morphdom. And this is where it fails: The custom element is already registered and defined, and by replacing the current <data-table> using morphdom the connectedCallback is called again, and doesn't find the slotted table inside.