Skip to content
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

A way to detect if a custom element was constructed during customElements.define call? #790

Closed
trusktr opened this issue Feb 6, 2019 · 8 comments

Comments

@trusktr
Copy link
Contributor

trusktr commented Feb 6, 2019

F.e.,

class MyEl extends HTMLElement {
 constructor() {
  if (/* constructedDuringCustomElementsDotDefineCall? */) {
    console.log('created during customElements.define call')
  }
 }
 connectedCallback() {
  if (/* runningInTheCustomElementsDotDefineCall? */) {
    console.log('connected during customElements.define call')
  }
 }
}

I think one way I can do this is to monkey patch customElements.define and set a var.

The reason is, because I'm finding that if I define elements after they have already been parsed, then creating a MutationObserver in the constructor does not observe childList because the nodes that are being upgraded already have the children (no DOM mutation happened).

I'd like to fire childConnectedCallback in this case, but the idea from #789 only works during parsing.

@trusktr
Copy link
Contributor Author

trusktr commented Feb 6, 2019

I made a module to detect it:

export var isCustomElementsDefineCall = false

const oldDefine = customElements.define

customElements.define = function(tagName, Constructor) {
    isCustomElementsDefineCall = true
    oldDefine.call(this, tagName, Constructor)
    isCustomElementsDefineCall = false
}

So then,

import { isCustomElementsDefineCall } from './util/isCustomElementsDefineCall'

class MyEl extends HTMLElement {
 constructor() {
  if (isCustomElementsDefineCall) {
    console.log('created during customElements.define call')
  }
 }
}

now when I detect this case, I can schedule an observation of children.

@andyearnshaw
Copy link

andyearnshaw commented Feb 6, 2019

The reason is, because I'm finding that if I define elements after they have already been parsed, then creating a MutationObserver in the constructor does not observe childList because the nodes that are being upgraded already have the children (no DOM mutation happened).

An upgrade can happen with children in other scenarios, too. Why not just check to see if the element already has children in the constructor?

class MyEl extends HTMLElement {
  constructor() {
    super();
    if (this.children.length) {
      console.log('has children when upgraded');
    }
  }
}

These kinds of questions are probably better suited to somewhere like Stack Overflow rather than this issue tracker. You'll probably get answers quicker there anyway.

@trusktr
Copy link
Contributor Author

trusktr commented Feb 6, 2019

    if (this.children.length) {

About that, that isn't enough. At that point if a child isn't upgraded yet, we can not see upgraded props/methods on the children. That's why I opened #789. So in that case, I create a MutationObserver, and it does fire for the children after they are upgraded.

So as a result of previous discussion in #789 and #787, the last implementation that I had before opening this one was the one in #787 (comment).

But now that I've come across the case of defining elements after parsing (all of them at once, because doing them individually in different turns of the JS event loop will be another case), I've discovered that I need to handle this too, but because connected and disconnected callbacks are all synchronous in this case, I need unfortunately figure a way to call my childConnectedCallbacks synchronously after the connectedCallback stack of my class hierarchy so that I can have them fire in the same order as during parsing.

Here's the order I see in the console when I define elements before parsing:

CONNECTED Scene
CONNECTED AutoLayoutNode
CHILD CONNECTED parent: Scene child: AutoLayoutNode
CONNECTED Node
CHILD CONNECTED parent: AutoLayoutNode child: Node
CONNECTED Node
CHILD CONNECTED parent: AutoLayoutNode child: Node
CONNECTED Node
CHILD CONNECTED parent: AutoLayoutNode child: Node
CONNECTED Node
CHILD CONNECTED parent: AutoLayoutNode child: Node
CONNECTED Node
CHILD CONNECTED parent: AutoLayoutNode child: Node

and if I defer childConnectedCallbacks to a microtask in connectedCallback like you see in #787 (comment), then the order is

CONNECTED Scene
CONNECTED AutoLayoutNode
CONNECTED Node
CONNECTED Node
CONNECTED Node
CONNECTED Node
CONNECTED Node
CHILD CONNECTED parent: Scene child: AutoLayoutNode
CHILD CONNECTED parent: AutoLayoutNode child: Node
CHILD CONNECTED parent: AutoLayoutNode child: Node
CHILD CONNECTED parent: AutoLayoutNode child: Node
CHILD CONNECTED parent: AutoLayoutNode child: Node
CHILD CONNECTED parent: AutoLayoutNode child: Node

I may have an error in some code that consumes the WithChildren feature, due to load order differences, but that's a different topic.

At least now the callbacks are firing in all the cases in a way that is almost on par with how/when connectedCallback and disconnectedCallback fire (f.e. childConnectedCallbacks fire one-to-one with connectedCallback at the same time (save for being a microtask apart).

If the engine was had this implemented, it'd be easy: fire the connectedCallback (if any) of an element synchronously, then fire the childConnectedCallback (if any) of the element's parent synchronously. Boom, done! Super easy! Similarly, I bet it would be easy to add this feature into a Custom Element polyfill.

So, I've gotta see if I can trigger the childConnectedCallbacks in a way that they match the connectedCallback behavior as closely as possible, namely that in customElements.define the calls need to be synchronous. I'd expect to see the interwoven console.log output like the fir output example.

I bet I can achieve this with another customElement.define patch, though it is a little funky that it'll live outside of my WithChildren class. At least it'll be in the same module so the code itself will be logically co-located. I'll post back to #787 once I have that version working...

@trusktr
Copy link
Contributor Author

trusktr commented Feb 6, 2019

Oh, plus, SO is not conducive to discussion: cross-linking issues is not a prime feature, there's no markdown, pasting URL-shortened links in comments fails, etc. It's just not a good discussion medium.

I think the questions are better here because they help standards implementers have better visibility into what users of the API are doing or want to do, and how. Do you think this is the case? Or do the implementers view the questions on SO? It doesn't seem like it'd be easy to track anything on SO based on the points of the previous paragraph.

@domenic
Copy link
Collaborator

domenic commented Feb 6, 2019

These kinds of questions are not bugs with the standard and indeed belong on StackOverflow or other forums.

Additionally, please realize that for every comment and issue you make you send 295 people an email. So far you have made not one, but five, comments on this thread, some of which are rather repetitive and off-topic. Please be more conservative with how you use other peoples' time and attention, for example by editing your posts instead of re-posting them with slight modifications or follow-up points.

@domenic domenic closed this as completed Feb 6, 2019
@trusktr
Copy link
Contributor Author

trusktr commented Feb 6, 2019

I just noticed my comment was posted twice due to network disconnection. Nothing I tried to post here was repetitive from my perspective, but fault of GitHub's erroneous way of handling internet connectivity.

Sorry if you thought I posted anything twice.

My point stands: posting to SO will make any sort of discussion on the topic difficult, and will not help the standards process.

The standards process should involve user-centric analysis of what users can/want to do, and SO is obviously not the best place for that.

The things I'm trying to discuss are about wants/desires that are otherwise difficult, unintuitive, and error-prone to achieve, and telling people to go to Stack Overflow is similar to saying "shoo fly" without caring about the end user as much as they can be cared for.

This is same reason, IMO, why the private fields proposal has such an incredibly high ratio of dislike to like yet "open standards implementers" ignore it.

That's how I see it when I'm told to move along to Stack Overflow. Maybe it's something you could consider.

@rniwa
Copy link
Collaborator

rniwa commented Feb 7, 2019

It's definitely helpful to see what authors want & can do. What we can't really do is to answer any question you may have to do X, Y, or Z. Those things need to be discussed in some other forum, and only when all the options are exhausted, should you post a proposal or file an issue on this issue tracker in the form of what you need it and why you need it with detailed explanation as to why that's impossible, or really hard to do.

Just to echo what @domenic said, I can only work 10-12 hours a day to fix bugs & implement new features in WebKit. Every minute I spend reading & responding any comments on this issue tracker is the time I didn't spend fixing bugs. In that sense, any new issue filed on this issue tracker is basically resulting in multiple WebKit bugs not getting fixed or a new feature not getting implemented or implemented later over years. That's a pretty steep price we, as the humanity, are paying if you ask me.

Now, many of the issues you filed in the past and present are very insightful, and we've learned quite a few things, and it's totally worth whatever collective time we've spent discussing. However, at times, some of your posts can be repetitive & hard to follow because your understanding of the problem is rapidly evolving throughout the issue over a span of days. So as much as I really appreciate all your enthusiasm, and I really enjoy working with you, I respectfully ask you to study & research the problem space fully before filing a new issue, and try to make your description more concise in the future.

@trusktr
Copy link
Contributor Author

trusktr commented Feb 17, 2019

Thanks for explaining your perspective @rniwa, I appreciate that.

study & research the problem space fully before filing a [concise] new issue

👍 Will do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants