search

mozilla / standards-positions

https://mozilla.github.io/standards-positions/
Mozilla Public License 2.0
639 stars 69 forks source link

Declarative Shadow DOM #335

Closed mfreed7 closed 1 year ago

mfreed7 commented 4 years ago

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 commented 4 years ago

cc @smaug---- @hsivonen @edgarchen

dbaron commented 4 years ago

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:

mfreed7 commented 4 years ago

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.

EdgarChen commented 4 years ago

Is https://github.com/whatwg/dom/issues/831#issuecomment-585580174 still a valid concern?

mfreed7 commented 4 years ago

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---- commented 4 years ago

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 https://github.com/whatwg/dom/issues/831#issuecomment-608131292 , 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 https://github.com/w3c/webcomponents/issues/809 needs to be solved in some way.

mfreed7 commented 4 years ago

@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 commented 4 years ago

Some thoughts in no particular order:

mfreed7 commented 4 years ago

@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 commented 4 years ago

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

mfreed7 commented 3 years ago

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 commented 3 years ago

I think as per the discussion in https://github.com/whatwg/dom/issues/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 commented 3 years ago

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!

mfreed7 commented 3 years ago

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