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

Declarative Shadow DOM #335

Closed
mfreed7 opened this issue May 6, 2020 · 22 comments · Fixed by #461 or #740
Closed

Declarative Shadow DOM #335

mfreed7 opened this issue May 6, 2020 · 22 comments · Fixed by #461 or #740
Labels
awaiting feedback Awaiting feedback from other Mozillians (request must have either been emailed or acknowledged) position: positive venue: WHATWG Specifications in a WHATWG Workstream

Comments

@mfreed7
Copy link

mfreed7 commented May 6, 2020

Request for Mozilla Position on an Emerging Web Specification

Other information

A declarative API to allow the creation of #shadowroots using only HTML and no Javascript. This API allows Web Components that use Shadow DOM to also make use of Server-Side Rendering (SSR), to get rendered content onscreen quickly without requiring Javascript for shadow root attachment and population.

Much more detail, including more motivating use cases, can be found in the explainer.

Chrome is particularly interested in shipping this feature. A majority of the proposed spec has been implemented, with WPT tests, behind the DeclarativeShadowDOM feature flag.

Example syntax:

<host-element>
    <template shadowroot="open">
        <style>shadow styles</style>
        <h2>Shadow Content</h2>
        <slot></slot>
    </template>
    <h2>Light content</h2>
</host-element>
@annevk
Copy link
Contributor

annevk commented May 6, 2020

cc @smaug---- @hsivonen @EdgarChen

@dbaron
Copy link
Contributor

dbaron commented May 6, 2020

My initial reaction to this (although I wasn't involved in the discussions that led to it) is that it makes me pretty nervous for a few reasons, although I haven't thought any of them through deeply:

  • it seems like declarative binding of shadow dom likely has substantially broader use cases than just Server Side Rendering of things that are going to later be generated by JS libraries, and I worry that this proposal might make those things harder
  • it seems like something that's pretty far on the side of "features that require tools to generate" rather than features that can be written by hand (whereas I think other reasons to want declarative shadow DOM probably don't)
  • I'm somewhat concerned about the effect on the relationship between source text and DOM
  • I'm somewhat concerned about the effect on the ideas of encapsulation
  • It's not clear to me that we should be promoting web components as the solution for style encapsulation or whether we should have something actually designed for that use case.

@dbaron dbaron added the venue: WHATWG Specifications in a WHATWG Workstream label May 6, 2020
@mfreed7
Copy link
Author

mfreed7 commented May 8, 2020

My initial reaction to this (although I wasn't involved in the discussions that led to it) is that it makes me pretty nervous for a few reasons, although I haven't thought any of them through deeply:

Thanks for the comments, @dbaron. Let me see if I can address at least some of them:

  • it seems like declarative binding of shadow dom likely has substantially broader use cases than just Server Side Rendering of things that are going to later be generated by JS libraries, and I worry that this proposal might make those things harder

I hope you are correct that this has more-broad applicability than just SSR. Though that clearly (to me at least) is the most-commonly-cited reason for wanting a declarative API for Shadow DOM.

  • it seems like something that's pretty far on the side of "features that require tools to generate" rather than features that can be written by hand (whereas I think other reasons to want declarative shadow DOM probably don't)

I assume here you are referring to the fact that the template content needs to be duplicated within each shadow root? If so, I do understand this concern, and I alluded to this in the explainer. The biggest problem that I see is that you need some sort of idref-based solution to enable re-using a single template for multiple shadow roots, and that is currently not quite a solved problem. I'm open to suggestions here. It does seem possible to (later) expand this first iteration (where each shadow root needs local template content) to allow a reference to another template. Something like <template shadowroot=open shadowrootcontent=my-template-id>. But I'm hoping we can do that as a phase-2?

  • I'm somewhat concerned about the effect on the relationship between source text and DOM

Hmm, I'm not sure what you mean here. Can you elaborate a bit?

  • I'm somewhat concerned about the effect on the ideas of encapsulation

Hopefully encapsulation should be ok. There were several comments on the issue thread about ensuring proper encapsulation, e.g. here, and I think the proposed API captures those. Is there something particular you are concerned about, so we can make sure it gets addressed?

  • It's not clear to me that we should be promoting web components as the solution for style encapsulation or whether we should have something actually designed for that use case.

I do agree with you here. We are actually working on a separate proposal for style encapsulation independent of shadow DOM, but that is moving slowly and not ready for prime time. In the meantime, while this proposal does list style encapsulation as something that gets easier to use with declarative Shadow DOM, I would agree that we shouldn't "push" this use case. On the other hand, I wouldn't argue with a developer who "likes" the style encapsulation features of Shadow DOM, and wants to use them, but is precluded for one reason or the other from using Javascript.

@dbaron dbaron added the awaiting feedback Awaiting feedback from other Mozillians (request must have either been emailed or acknowledged) label May 14, 2020
@EdgarChen
Copy link

Is whatwg/dom#831 (comment) still a valid concern?

@mfreed7
Copy link
Author

mfreed7 commented May 26, 2020

Is whatwg/dom#831 (comment) still a valid concern?

The question of attribute injection is not fully resolved, I don't think. However, I'm not sure what changes could be made to improve the situation. Note that the comment you reference had an answer from me, here, which was never discussed again on that thread. For completeness, here is the original comment and my response:

Attribute injection, which is what this is responding to, means injecting them on the parser level. So the fact that this is a parse-only feature is not a protection against attribute injection. Specifically, if a page has a <template> and you manage to get <script> inside it, right now that is safe. If you can also get this new attribute on the <template>, that is no longer safe...

I see - you're right. Attribute injection would allow the shadowroot attribute to get added to a <template> which would activate any child <script> elements. I'm not as familiar as I should be with the security aspects of this proposal. Can you elaborate on how one would also inject a <script> node inside the <template>? I.e. why is it more likely that this could happen inside <template> than outside? Otherwise, it seems easier to just skip the attribute injection and inject your <script> directly on the page somewhere, where it would run. Is it that existing sanitizers assume the insides of a <template> are safe?

I'd be really curious to hear your thoughts on this issue, and ideas for potential mitigations.

@smaug----
Copy link
Collaborator

The biggest worry I have here is that since the proposal deals with a small subset of the overall problem space around declarative Shadow DOM, is it possible that we end up having this one syntax where template is used within some element to define Shadow DOM, and then possibly some very different syntax for the case were idrefs are used etc. And if that happens, there is a chance that everyone will use that new syntax (since it it would avoid shortcomings like the repetition), but browser are forced to support both. But this is not very strong concern.

And rniwa's whatwg/dom#831 (comment) , especially the part around not knowing when parsing has finished makes the setup rather fragile.
It happens quite often in web pages that they have been designed to work well when network connection is fast and parsing happens pretty much as a single step, but once you start cutting that to smaller chunks, things start to break. I guess WICG/webcomponents#809 needs to be solved in some way.

@mfreed7
Copy link
Author

mfreed7 commented May 29, 2020

@smaug----, thanks for the comments!

The biggest worry I have here is that since the proposal deals with a small subset of the overall problem space around declarative Shadow DOM, is it possible that we end up having this one syntax where template is used within some element to define Shadow DOM, and then possibly some very different syntax for the case were idrefs are used etc. And if that happens, there is a chance that everyone will use that new syntax (since it it would avoid shortcomings like the repetition), but browser are forced to support both. But this is not very strong concern.

Thanks for bringing this up. I do see your point. On the flipside, however, I think there is a natural path to supporting both features that can share much of the code. For example, once idrefs are available, one could use something like this:

<template shadowroot=open shadowtemplate=#magic-global-id></template>

Several things need to be worked out, such as how the magic-global-id works, and what happens if that template is located "after" the reference. But assuming those are figured out, this would just look for the given <template id=#magic-global-id> in the DOM and clone its content, rather than moving the this <template>s content document into the shadow root. The rest of the code is shared and identical, I think.

And rniwa's whatwg/dom#831 (comment) , especially the part around not knowing when parsing has finished makes the setup rather fragile.
It happens quite often in web pages that they have been designed to work well when network connection is fast and parsing happens pretty much as a single step, but once you start cutting that to smaller chunks, things start to break. I guess w3c/webcomponents#809 needs to be solved in some way.

Here again, I agree with you. I've also said so on the issue thread. I do think #809 needs to be solved, but I really don't think it needs to be "tied" to this implementation. As mentioned on the 809 thread, there are already numerous reasons to want such a feature, and numerous potential footguns present because 809 isn't solved. I would argue that while this is one more potential footgun, it is fairly easy to avoid by simply placing declarative content before asynchronous <script> nodes in the document. Happily, this also seems like the most-likely default way to use declarative Shadow DOM. It seems like there is at least some agreement on this point from developers.

@hsivonen
Copy link
Member

Some thoughts in no particular order:

  • I worried of the interaction of declarative things and scripted things. If a script can get involved, the declarative mechanism needs to act in a very specific way so that the script interaction is interoperable.
  • The description of the attachment process doesn't talk about the event loop. Does it mean that process happens as synchronously as normal parser-performed node insertion? Has the full range of things that might be able to observe this synchronously from script (given existing definitions) been considered appropriately?
  • Has the synchronous observability (if any) been considered for the case where a declarative shadow DOM template is inside a normal template? (I'm not really a fan of there being two paths: The parser-inserted one and the one triggering upon instatiating a normal template. I don't have better suggestion, though.)
  • When a script doesn't get involved, the only benefit from having a shadow tree instead of plain descendants seems to be having the boundary for CSS purposes. Is this really important enough to justify new complexity?
  • The concern about the Shadow DOM not having a serialization and this feature adding it seems like the earlier design decision was wrong and this tries to patch it up. Mistakes happen and need to be fixed, but I'm not particularly happy about this kind of piecewise design, where one of the problems being addressed now is the result of an intentional earlier design decision.
  • I'm a little worried about adding more innerHTML-ish features due to the XSS risks associated with manipulation of the serialized form. But it's hard to go against what Web developers want to do. (See XML.)
  • Was the performance of a conditional polyfill being present but bailing out due to native support existing benchmarked? (This seems relevant to the adoption path given the motivation of the feature.)

@mfreed7
Copy link
Author

mfreed7 commented Jun 24, 2020

@hsivonen Thanks for the thoughtful comments! Please see my replies inline...

  • I worried of the interaction of declarative things and scripted things. If a script can get involved, the declarative mechanism needs to act in a very specific way so that the script interaction is interoperable.

I agree that this is a concern, and it has been discussed on the issue thread, starting roughly here. I believe (though please point out places where I've missed things!) that I've specified the process in a concrete way so that this can be made interoperable. In particular, I have DOM and HTML PRs that could use a review.

  • The description of the attachment process doesn't talk about the event loop. Does it mean that process happens as synchronously as normal parser-performed node insertion? Has the full range of things that might be able to observe this synchronously from script (given existing definitions) been considered appropriately?

The idea is that yes - the process (shadow root attachment, template contents move, template node delete) happens synchronously just as normal parser-performed node insertion. I believe I've thought of the consequences of that, but as with all such situations, more consideration would be helpful. But I did write a few WPT tests that look at observability of DOM modifications, for example.

  • Has the synchronous observability (if any) been considered for the case where a declarative shadow DOM template is inside a normal template? (I'm not really a fan of there being two paths: The parser-inserted one and the one triggering upon instatiating a normal template. I don't have better suggestion, though.)

Yes, again here I believe I've thought of that - see this WPT test. The TL/DR is that a normal template containing a declarative template will now contain a shadow root inside the template.content. I had to modify the spec for cloning nodes to deal with this, so that the cloned nodes get fresh shadowroots. But again, let me know if you see a corner case I missed or didn't think about.

Related to the above, we also modified the spec so that the template.content accessor return null for the <template shadowroot> template node while the contents are being parsed. This is to avoid scripts getting access to what might eventually become the closed shadow root contents.

  • When a script doesn't get involved, the only benefit from having a shadow tree instead of plain descendants seems to be having the boundary for CSS purposes. Is this really important enough to justify new complexity?

Yes, Shadow DOM provides style and JS encapsulation. In the no-JS case, all that is left is style encapsulation. One of the motivations, though admittedly a small one, is to allow CSS developers to use declarative Shadow DOM for "pure" style encapsulation. However, as stated in the explainer, the primary motivation for this feature is to allow full Web Components users (who also expect the JS encapsulation) to use Server Side Rendering, which produces an isomorphic representation on the client and server.

  • The concern about the Shadow DOM not having a serialization and this feature adding it seems like the earlier design decision was wrong and this tries to patch it up. Mistakes happen and need to be fixed, but I'm not particularly happy about this kind of piecewise design, where one of the problems being addressed now is the result of an intentional earlier design decision.

I agree with you, generally, but as you know, sometimes you only find out about problems "later". In this case, I believe the work to get to Shadow DOM v1 was long and hard, and I'm guessing it was easier to make it an imperative-only API to start. Now that this is an established standard, implemented in all browsers, we can start to think about the next thing - a declarative version.

  • I'm a little worried about adding more innerHTML-ish features due to the XSS risks associated with manipulation of the serialized form. But it's hard to go against what Web developers want to do. (See XML.)

I agree that this expansion of the innerHTML "power" might come with additional risks. I'm open to suggestions for how to minimize that while retaining the usefulness of the feature!

  • Was the performance of a conditional polyfill being present but bailing out due to native support existing benchmarked? (This seems relevant to the adoption path given the motivation of the feature.)

So the polyfill here is relatively simple I think. First, feature detection is possible. And if not natively supported, it is relatively straightforward to do a querySelectorAll('template[shadowroot]') after the declarative content has loaded, and attach shadow roots to parentElement for each <template shadowroot> found. There should be no performance penalty (other than the feature detection) when the feature is natively supported. I'm not sure I answered this question - lmk if not.

@mfreed7
Copy link
Author

mfreed7 commented Aug 4, 2020

Just a quick friendly ping to see if there was any additional input on declarative Shadow DOM?

@mfreed7
Copy link
Author

mfreed7 commented Oct 21, 2020

Friendly ping on this thread. Declarative Shadow DOM will be discussed next week at TPAC, and Chromium is hoping to ship this feature soon, so we'd love your feedback.

@annevk
Copy link
Contributor

annevk commented Dec 3, 2020

I think as per the discussion in whatwg/dom#831 and above our position is non-harmful. We are not convinced the feature carries its weight, but the proposal is a reasonable approach given the various constraints. There's also a risk that it might not meet the needs for declarative custom elements as this was largely developed in isolation.

Happy to take further feedback for a bit before closing this with a PR.

@mfreed7
Copy link
Author

mfreed7 commented Dec 4, 2020

I think as per the discussion in whatwg/dom#831 and above our position is non-harmful. We are not convinced the feature carries its weight, but the proposal is a reasonable approach given the various constraints. There's also a risk that it might not meet the needs for declarative custom elements as this was largely developed in isolation.

Happy to take further feedback for a bit before closing this with a PR.

Thanks @annevk for this feedback, and for considering this feature non-harmful. I do understand your concern about the feature carrying its weight - perhaps we'll learn more on this from the use counters after it launches. I also agree that this feature has been developed separately from declarative custom elements, and that does pose a risk. Though as I've pointed out previously, the current strawman proposal uses a syntax very similar to this one for the shadow content, so hopefully they will eventually play nicely together.

Again, thanks!

annevk added a commit that referenced this issue Dec 7, 2020
annevk added a commit that referenced this issue Dec 7, 2020
@mfreed7
Copy link
Author

mfreed7 commented Feb 18, 2021

I just wanted to follow up on this request for position, as there has been significant offline discussion about this feature.
This post attempts to incorporate the key points from that discussion.

It is Chromium's position that this feature is ready to go, and all known/discussed issues have been resolved. To quickly summarize the changes and discussions that have happened since I proposed this feature over a year ago:

  1. Issue 871 was resolved. As a result:
    a. The ElementInternals.shadowRoot attribute now gives access to declarative shadow root content, including for closed shadow roots.
    b. The attachInternals() API is now inaccessible prior to the custom element constructor being run.

  2. The behavior of attachShadow() was improved to ensure interop with existing components - existing declarative shadow roots are emptied and returned.

  3. The .content attribute on now returns null during parsing, to avoid leaking closed shadow root contents.

  4. The getInnerHTML() method was defined for serialization:
    a. The prior proposal modified innerHTML, and required each shadow root to opt-in.
    b. There is now a parameter that allows serialization of closed shadow roots.

  5. The potential for client-side XSS attacks was identified and mitigated:
    a. the original setInnerHTML() API was removed from the feature
    b. an "opt-in" was added to DOMParser so that existing sanitizers cannot "accidentally" allow DSD content to bypass the filter.

  6. The spec PRs (HTML, DOM, and XHR) have had a number of helpful comments, all of which I believe have been addressed.

  7. An additional exploration of speed was undertaken, including an investigation of the general overhead of Shadow DOM, and another investigation into potential MutationObserver-based polyfills. Our conclusions are generally and specifically that DSD should improve the speed of the platform.

  8. Additional thought (see #3 here) was given to the potential to use DSD as the serialization format for the selection APIs and Shadow DOM.

  9. Chromium has completed Origin Trials with successful results. In particular, one trial found (preliminarily!) a 15% improvement in FCP when DSD was used in conjunction with Server Side Rendering.

  10. The TAG Review was completed successfully.

  11. Based on our perception that this feature is in a good state, and would benefit the platform, Chromium intends to try to get this feature shipped soon. We would love to have the support of Mozilla.

@smaug----
Copy link
Collaborator

FWIW, the discussion here refers to the old declarative shadow DOM proposal, which was quite different to the new one.
At least personally I consider the new one, which isn't quite ready yet, be more reasonable API and it should hopefully work better with certain use cases and be also relatively fast.

@keithamus
Copy link

Worth pointing out that WebKit has a patch to enable declarative shadow dom by default on all platforms: WebKit/WebKit#7798

Their standards position on this is “support”.

@smaug----
Copy link
Collaborator

And worth to note that there isn't even spec yet for declarative shadow DOM.

@mfreed7
Copy link
Author

mfreed7 commented Jan 24, 2023

I had been assuming, based on this comment (whatwg/dom#831 (comment)) that Mozilla was at least approaching supportive. Could we please re-open this issue and perhaps update the position?

And worth to note that there isn't even spec yet for declarative shadow DOM.

There is certainly an in progress spec PR for DSD: whatwg/html#5465 and whatwg/dom#892

@zcorpan
Copy link
Member

zcorpan commented Jan 26, 2023

We're positive for the new approach. Thanks for raising. I'll submit a PR to update the dashboard entry.

@mfreed7
Copy link
Author

mfreed7 commented Jan 27, 2023

We're positive for the new approach. Thanks for raising. I'll submit a PR to update the dashboard entry.

🎉 Thank you!

@michaelwarren1106
Copy link

hey all-

I'm working on preparing a TPAC report for Declarative Shadow DOM, and I'm trying to get a sense of the current state of DSD.

Is there an Intent to Prototype/Ship declared somewhere for firefox that someone could send a link to?

I have the bugzilla as far as I know. Is that one the best one?

If there are no Intent to prototype/ship declarations, another question would be whether there are any blockers from the spec/proposal perspective that we can call out in TPAC to get resolutions for.

Thanks for the help!

@smaug----
Copy link
Collaborator

The implementation work should start soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting feedback Awaiting feedback from other Mozillians (request must have either been emailed or acknowledged) position: positive venue: WHATWG Specifications in a WHATWG Workstream
Projects
None yet
9 participants