Re: [whatwg/dom] Declarative Shadow DOM (#831)

Thanks to everyone for the great comments here. There seem to be a few themes - I'll try to summarize and respond:

1. What should the custom element definition look like?

   The explainer does have a [section](https://github.com/mfreed7/declarative-shadow-dom/blob/master/README.md#feature-detection-and-polyfilling) for this, but I really like the example provided by @Rich-Harris in [this comment](https://github.com/whatwg/dom/issues/831#issuecomment-585372554). That is exactly what I was envisioning - a small `if (this.shadowRoot)` block that just hooks things up if the `shadowRoot` already exists, and the actual construction code if not. That code works on both client and server (it is isomorphic), and the added code is minimal. @Rich-Harris asked what happens if the declarative content is malformed - if that is a possibility (due to versioning, etc.), then no matter what the declarative solution, you'll need to do extra work. And in the case where you *can* assume that an existing `#shadowroot` means your content is "good to go", you'll get a performance win from not having to blow away the existing content and re-create it.

2. Wouldn't it be better to re-use a single `<template>` rather than duplicating it for each shadow root?

   Several people already responded to this, but I wanted to point out [this section of the explainer](https://github.com/mfreed7/declarative-shadow-dom/blob/master/README.md#instead-of-inline-contents-use-an-idref-to-an-existing-template) that discusses this point at length. The important three points in my mind are:
      a. As @Rich-Harris and @justinfagnani point out, it is important to remember that we're serializing *instances* of elements, which likely differ from one another slightly in terms of their DOM content.
      b. In terms of data/overhead, gzip nicely fixes most of the ills of almost-perfectly-repeated content. Aside from the potentially-shared styles (see the point below), you'll get another copy of the DOM no matter what you do here. So the "overhead" benefits of sharing a single `<template>` for this seem rather limited.
      c. Re-using a `<template>` like this requires a solution to the previously-unsolved "idref" issue. See [here](https://github.com/whatwg/html/issues/4925) for the discussion around ARIA labelled-by. The problem is: how do you deal with nested shadow roots? The idref would then need to cross shadow bounds, potentially in both directions. We don't have a way to allow that, yet.

3. How to handle styles?

   This is definitely an open question. I'm hoping we can come up with a declarative Shadow DOM solution that isn't **tied** to a particular solution to the styling problem. To do that, I have proposed just using inline `<style>`s within each shadow root. As [mentioned](https://github.com/whatwg/dom/issues/831#issuecomment-585372554), this would result in a) more bytes on the wire, and b) more DOM memory used. Of those, I'm least concerned with a). For the example HTML provided in [this comment](https://github.com/whatwg/dom/issues/831#issuecomment-585372554), when gzipped, the inline `<style>` example takes 290 bytes, while the "shared stylesheet" example takes 223 bytes. Yes, that's 25% more, but not the factor of two that it would appear from the raw HTML. I agree that problem b) is a problem that needs a solution. Perhaps the parser could detect exactly-duplicate `<style>` elements and condense them into a single `CSSStyleSheet` that gets added to `adoptedStylesheets`? That might be crazy. I **do** agree that styles need a solution. I **don't** think it needs to be solved for this declarative Shadow DOM solution to be useful as-is.

4. Serialization and the "serializable:true" option.

   I love the @Rich-Harris [suggestion](https://github.com/whatwg/dom/issues/831#issuecomment-585372554) to add another API (`element.innerHTMLWithShadowDOM`) that serializes all shadow roots by default. That avoids the need to retrofit existing components with `serializable:true`, and as you pointed out, this isn't the component's decision to make anyway. I'd be in favor of changing the explainer to match this suggestion.

5. What about existing html parser behaviors, e.g. table fixup: `<template shadowroot=open><td>Foo</td></template>`

   The advantage of this proposal is that nearly all of the existing "standard" `<template>` behavior still applies. For example, for this specific example, the ["in template" insertion mode rules](https://html.spec.whatwg.org/multipage/parsing.html#parsing-main-intemplate) apply. I don't think there is any additional ambiguity created by this proposal.

6. Is script injection a problem? Can't a previously-harmless `<template>` be made active by attribute injection?

   No, at least according to the existing [proposal](https://github.com/mfreed7/declarative-shadow-dom/blob/master/README.md#other-details--questions). This would be a "parser-only" feature, and adding the `shadowroot` attribute to an existing `<template>` would have no effect. As pointed out by @hayatoito, you could still imperatively build a `<template>`, add a `shadowroot` attribute, and then do element.innerHTML = element.innerHTML. The innerHTML assignment *would* see the full `<template shadowroot>` and would attach a shadow as it is parsed. But that doesn't seem like a security risk, since you're blowing away the entire innerHTML in that case anyway. Please correct me if I'm wrong.

7. It is weird to have a `<template shadowroot>` turn into a `#shadowroot` and then have previously-sibling elements get slotted into the `#shadowroot`.

   Yes, this is definitely *different* and will take getting used to, no question. But this statement seems like it would apply to *any* declarative Shadow DOM solution. No matter the semantics, *some* element will become, or create, a `#shadowroot` which will then start "pulling in" sibling content into `<slot>`s.

8. What happens if there is already a shadow root?

   For compatibility and alignment, this needs to be an error. I mentioned several such scenarios in the explainer, [here](https://github.com/mfreed7/declarative-shadow-dom/blob/master/README.md#other-details--questions). Basically, you can (still) only attach a shadow root once, and any subsequent attempts (either declarative or imperative) will result in an error.

9. Adding `shadowroot` to `<template>` changes the semantics of the element, which is weird.

   Yes, it is, I agree. This is discussed [here](https://github.com/mfreed7/declarative-shadow-dom/blob/master/README.md#syntax-template-shadowrootopen-vs-shadowroot) in the explainer. The one point that seems to kill the idea of creating a new element (e.g. `<shadowroot>`) is the backwards-compat problem. Until all browsers understand the new element, enclosed `<style>` and `<script>` elements will be exposed to the parent page, with potentially bad consequences.

Thanks again for the great points raised here!

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/issues/831#issuecomment-585451735

Received on Wednesday, 12 February 2020 22:32:45 UTC