-
Notifications
You must be signed in to change notification settings - Fork 378
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
connectedCallback timing when the document parser creates custom elements #551
Comments
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. |
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. |
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 |
In constructor, yes, but in |
Ah yeah they could. But that should not be required by the custom element. |
I'am the author of the blink bug,
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. |
When the node is created and inserted, it doesn't have any children. |
So... how do you work with custom elements that have expectations about their children? |
Mutation observers. |
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:
No concerns here, just trying to work it out in my head. |
If you set up the mutation observer at construction time, that first |
I meant it can cause do stuff to be scheduled. Mutation observers run end-of-task so not immediately. |
An alternative is to add |
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.
Why not if all initial children are returned in one call. |
The whole idea of initial children is flawed. Children are dynamic. |
Indeed. In general the multi-tag designs of HTML (e.g. |
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 |
Fixes WICG/webcomponents#551 by ensuring that insertions into the DOM trigger connectedCallback immediately, instead of putting the callback reaction on the the backup element queue and letting it get triggered at the next microtask checkpoint. This means connectedCallback will generally be invoked when the element has zero children, as expected, instead of a random number depending on when the next custom element is seen.
FWIW, I would object to adding something like |
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 ? |
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.
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, that's a good idea.
Ok, thanks, it's bizarre but i trust you :) |
Nah. When do you think browsers update rendering for their elements? (The answer is: every animation frame callback.) |
Only if there is something to update though. |
Fixes WICG/webcomponents#551 by ensuring that insertions into the DOM trigger connectedCallback immediately, instead of putting the callback reaction on the the backup element queue and letting it get triggered at the next microtask checkpoint. This means connectedCallback will generally be invoked when the element has zero children, as expected, instead of a random number depending on when the next custom element is seen.
Sorry to come back but i was checking the SkateJS library and i saw that in the todolist example on the homepage:
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. |
Yeah, this might be worth someone doing a blog post on or similar. |
A library could provide that hook for you, or conversely a small function could give you the same behavior: connectedCallback() {
onChildren(this, children => {
});
} |
Right ... little variation that uses a const HTMLParsedElement = (() => {
const DCL = 'DOMContentLoaded';
const init = new WeakMap;
const isParsed = el => {
do {
if (el.nextSibling)
return true;
} while (el = el.parentNode);
return false;
};
const cleanUp = (el, observer, ownerDocument, onDCL) => {
observer.disconnect();
ownerDocument.removeEventListener(DCL, onDCL);
init.set(el, true);
parsedCallback(el);
};
const parsedCallback = el => el.parsedCallback();
return class HTMLParsedElement extends HTMLElement {
connectedCallback() {
if ('parsedCallback' in this && !init.has(this)) {
const self = this;
const {ownerDocument} = self;
init.set(self, false);
if (ownerDocument.readyState === 'complete' || isParsed(self))
Promise.resolve(self).then(parsedCallback);
else {
const onDCL = () => cleanUp(self, observer, ownerDocument, onDCL);
ownerDocument.addEventListener(DCL, onDCL);
const observer = new MutationObserver(changes => {
if (isParsed(self)) {
cleanUp(self, observer, ownerDocument, onDCL);
return true;
}
});
observer.observe(self.parentNode, {childList: true, subtree: true});
}
}
}
get parsed() {
return init.get(this) === true;
}
};
})(); I think I'll publish this one to npm. |
Comments on your comments:
What is still open with this solution is to adjust it for use in customized-built-ins that have expectations about their children. Btw, I was also considering publishing this on npm, but probably it'll gain more traction if you do it. |
@WebReflection I saw you're quick: https://github.com/WebReflection/html-parsed-element I wouldn't reject attribution, neither would @irhadkul I assume :) |
well, if any of you want I can include attributions but just to be clear, this is what HyperHTMLElement does since about ever, using a timeout instead of The I will still link to this ticket in there so again, I don't mind adding anyone in here as contributor 👋 |
also @franktopel ...
My approach solves every issue. When
You don't . You disconnect the observer too so that's not your intent and also you want to Again, the missing bit that is essential is to know when it's safe to handle children or even inject nodes/html. Once we have that, everything else is trivial. |
Attribution is definitely welcome. While none of us has done anything even remotely comparable to your work, we surely contributed to the birth of html-parsed-element with the last 10 days' work. |
IIR you can test this page and it should work in IE9 too compatibility is probably for something not fully transpilable but I don't remember what. Feel free to file a PR for the attribution so I'm sure it's done properly/as you expect. |
Btw, how does |
@franktopel the Differently from |
P.S. @franktopel attributions are live |
@WebReflection So in that case we (our team) have the exact same problem with the |
@franktopel it's useful for empty nodes with shadow dom, and not much else indeed. With Meanwhile, if you react to const attributeChanged = new WeakMap;
class MyEl extends HTMLParsedElement {
parsedCallback() {
// setup the node, you have access to all its content/attributes
// then ...
const changes = attributeChanged.get(this);
if (changes) {
attributeChanged.delete(this);
changes.forEach(args => this.attributeChangedCallback(...args));
}
}
attributeChangedCallback(...args) {
if (!this.parsed) {
const changes = attributeChanged.get(this) || [];
if (changes.push(args) === 1)
attributeChanged.set(this, changes);
return;
}
// the rest of the code
}
connectedCallback() {
// here you can safely add listeners
// or set own component properties
this.live = true;
}
disconnectedCallback() {
// here you can safely remove listeners
this.live = false;
}
} |
Well, we're not using shadow DOM at all. We just need to replicate Swing controls for the web, for a huge migration project. And currently it needs to support IE 11 mainly. And it has to work for the next 10 to 15 years, without huge update efforts, like you'd have with using a framework like Angular or React. That's why the company decided to use native web components. So you're saying the whole spec has been designed around a very limited use edge case? |
Also, we have been using |
Wouldn't it be an even better approach to dispatch a We could even have all components register at a central service in their constructor on creation, and have that same service emit a global |
You can dispatch any event you want from parsedCallback ...having hybrid callbacks and events feels inconsistent with the API , imo |
Fixes WICG/webcomponents#551 by ensuring that insertions into the DOM trigger connectedCallback immediately, instead of putting the callback reaction on the the backup element queue and letting it get triggered at the next microtask checkpoint. This means connectedCallback will generally be invoked when the element has zero children, as expected, instead of a random number depending on when the next custom element is seen.
@franktopel and @WebReflection, thank you again for making a full implementation of this, I think it will help out a ton! I love how Web Components work, and using this in addition with Constructable Stylesheets will make everything so much more seamless, now being able to parse |
Noob suggestion here, why not just link your scripts with deferred or <script type="module" src="/static/..../component.min.js>
|
That will not work for nodes that are dynamically inserted, after initial setup. |
Solution for this problem is pretty simple, all one has to do it wrap I am pretty sure this is faster then any other proposed solution, as More in my answer here |
@dux not correct apparently, read thread from here: #809 (comment) |
From a blink bug, an author reported that
connectedCallback
is called too early in Blink.or a sample code in jsfiddle.
When current Blink impl runs this code,
this.children.length
is 1 inconnectedCallback
forx-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:
x-x
as Any other start tag, it insert an HTML element.Note that this "insert" is not a link, but I assume this is insert a node?
connectedCallback
.div
. In its create an element for a token, definition is null, so will execute script is false. Thisdiv
is then inserted.y-y
. In its create an element for a token, definition is non-null, so will execute script is true.connectedCallback
runs here.Am I reading the spec correctly? If so, is this what we should expect?
@domenic @dominiccooney @rniwa
The text was updated successfully, but these errors were encountered: