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

Measuring performance of shadow DOM #848

Open
npm1 opened this issue Oct 16, 2019 · 9 comments
Open

Measuring performance of shadow DOM #848

npm1 opened this issue Oct 16, 2019 · 9 comments

Comments

@npm1
Copy link

npm1 commented Oct 16, 2019

Following up on #816, there is a design problem with shadow DOM that needs to be resolved. There seems to be no way to measure performance within shadow DOM, even if the author of the shadow tree wants to opt in. There are many instances where this seems desirable, for instance when web components encapsulate a web app. Since encapsulating the whole web app in a component seems to be fairly common practice, it would be great to arrive to some consensus on how to allow exposing performance of shadow DOM.

@annevk
Copy link
Collaborator

annevk commented Oct 16, 2019

I think it would help if you made the problem statement a bit more precise. There were also various suggestions in the mentioned thread that you are not addressing here and it's unclear why they are not sufficient (in particular using ElementInternals or some kind of explicit piercing). I also don't think calling it a design problem with shadow trees is particularly productive or helpful to what you are trying to achieve.

@npm1
Copy link
Author

npm1 commented Oct 16, 2019

I think it would help if you made the problem statement a bit more precise.

The problem is that the encapsulation principles prevent leaking information about nodes inside a shadow tree, but there are many cases where web applications encapsulate their whole webpage. Therefore, it's not possible to measure element-specific performance of those websites. In particular I'm thinking about ElementTiming or LargestContentfulPaint, but as the issue was closed I guess this issue is supposed to cover potentially other affected APIs.

There were also various suggestions in the mentioned thread that you are not addressing here and it's unclear why they are not sufficient (in particular using ElementInternals or some kind of explicit piercing).

I'm not looking to address ideas as sufficient/insufficient because ultimately what I want is consensus on a solution. I'm happy to send a PR once people on the bug agree on the solution, but such agreement does not seem to have been reached. And sure, I can recap, here are the main ideas:

  • @domenic suggested removing strong encapsulation from open shadow DOM along with opt in from the page author (so it does not accidentally traverse the shadow tree). @rniwa said MutationObserver does not traverse the shadow trees.
  • @annevk suggested using ElementInternals but Domenic pointed out that that is specific to custom elements, and thus would not cover all shadow trees.
  • In general there are two perspectives: exposing performance to the authors of the shadow tree or to the users. I don't have a strong opinion on which is preferable because they coincide in the

I also don't think calling it a design problem with shadow trees is particularly productive or helpful to what you are trying to achieve.

Apologies if this offended you, I didn't mean to say 'shadow DOM design is flawed because you cannot measure performance inside it', what I mean is 'I want to enable measuring performance inside shadow DOM and this requires some design work'. Help is appreciated in reaching a solution that people are happy with.

@annevk
Copy link
Collaborator

annevk commented Oct 17, 2019

I still don't think it's particularly precise. Presumably those with access to the shadow tree have access to the information, if it's exposed on nodes. So they could relay it to others if they so desire.

As for ElementInternals and the slightly wider applicability of shadow trees, I think folks are largely coming around to the idea that if we had ElementInternals first, shadow trees would only be exposed through that.

Explicit piercing for open shadow trees could work (and could be applied to MutationObserver in theory, but I don't think we want to do that), but in general it's better if we design things that work for open and closed trees.

@domenic
Copy link
Collaborator

domenic commented Oct 17, 2019

As for ElementInternals and the slightly wider applicability of shadow trees, I think folks are largely coming around to the idea that if we had ElementInternals first, shadow trees would only be exposed through that.

You make this claim a lot, but I have never seen others agree with you. In particular I strongly disagree. So let's not speak on behalf of "folks", or start coupling shadow DOM and custom elements based on this counterfactual.

@annevk
Copy link
Collaborator

annevk commented Oct 17, 2019

Apologies, I vaguely recalled it coming up in the last meeting for some APIs, but I don't recall specifics unfortunately.

@bathos
Copy link

bathos commented Oct 17, 2019

not really on the OP topic, hence collapsing, but I found the previous two comments interesting and thought it could be helpful to share a ‘developer usage in the wild’ story:

I recently found it useful that I could attach shadow roots to non-custom elements. I had an element which would create transient containers in an external modal layer to achieve a ‘remote descendants’-like API (think react portals, or the weirder forms of angular transclusion that slots don’t cover). Stylesheets from the shadow root which the nodes would have lived in if they were really descendants of the element which was controlling them were brought along for the ride; this was the key to achieving the intended result.

It’s true that I could have created a ‘does nothing but append a shadow’ custom element for this purpose, but it was convenient (and more expressive) that I didn’t have to jump through that hoop and could just mint ‘anonymous’ divs to act as shadow hosts.

@caridy
Copy link

caridy commented Oct 17, 2019

The shadowDOM encapsulation is just that, encapsulation. And if you have arbitrary code traversing into the internals of the shadow, then why bother to have encapsulation in the first place? So, no, I don't think asking for a way to introspect into a shadow that you don't own just to measure the performance of elements is something that should be solved by breaking the encapsulation. This remind me of the webdriver shadow dom traversals conversations.

Now, if the code that is doing the measurements is "high-privilege" code (runs before everything else), then you might have other ways to achieve the same, e.g.: patching Element.prototype.attachShadow to get access to any constructed shadow, not pretty, but has worked for us in the past for performance and click tracking.

@npm1
Copy link
Author

npm1 commented Oct 17, 2019

I still don't think it's particularly precise. Presumably those with access to the shadow tree have access to the information, if it's exposed on nodes. So they could relay it to others if they so desire.

Those with access to the shadow tree would not have access to these APIs, as currently the scope of PerformanceEntry/PerformanceObserver is the window or worker. Perhaps this could be solved with observers scoped to shadow roots.

The shadowDOM encapsulation is just that, encapsulation. And if you have arbitrary code traversing into the internals of the shadow, then why bother to have encapsulation in the first place?

"Encapsulation" here can be different kinds of encapsulation. I don't see why wanting style encapsulation for your component implies that you should be unable to measure its performance. And to be clear, I understand that people would want a mechanism which requires opt in from the author of the shadow tree.

@npm1 npm1 closed this as completed Oct 17, 2019
@npm1 npm1 reopened this Oct 17, 2019
@isabel-nielson
Copy link

is there still no way to measure performance within the shadow DOM?

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

6 participants