Should there be an opt-out for declarative shadow roots having clonable=true?

[mfreed7]

What is the issue with the HTML Standard?

The spec for declarative shadow DOM says:

• Attach a shadow root with declarative shadow host element, declarative shadow mode, true, declarative shadow delegates focus, and "named".

The third true argument there corresponds to the clonable flag, meaning all declarative shadow roots are clonable. It seems like there should be a way to opt out of that behavior somehow, e.g.

<template shadowrootmode=open notclonable>

or something like that. Otherwise, every declarative shadow root becomes clonable. There is some concern from Chromium that shipping the general clonable behavior might be a web-incompatible change, for sites that use SSR/DSD and assume those shadow roots aren't cloned. @justinfagnani raised this concern for Lit users, for example.

@annevk @emilio

[justinfagnani]

Sorry, I somehow missed the clonable aspect of DSD entirely.

I think it's very much not true that just because a shadow root was created via DSD that it would be valid to clone it later.

For Lit, there are at least two major problems that make cloning problematic:

1. For client side rendering we don't emit the necessary extra DOM structure in order to rehydrate. We don't emit template IDs to be able to re-associate DOM to the template that rendered it, and we don't emit marker comments for attribute bindings. The code to do this isn't even part of the client-side packages.
2. A shadow root could easily go through state-driven changes before being cloned where that state is not available to the clone. Without something like cloneCallback() we don't even have a spot to clone that state. This will make the cloned shadow root useless, as we'd have to clear it out and re-render to be in sync with the clone's actual state.

So I'm pretty confident that right now many, if not most, cloned SSR'ed Lit components would fail, and our only fix would be to detect this case (we would need to differentiate from an actual DSD somehow) and clear the shadow root, completely defeating the purpose of cloning in the first place.

I would rather see cloneable be an opt-in.

[emilio]

cloneable being opt in makes more sense for whatwg/dom#1246 and co, actually. Otherwise any new non-declarative shadow DOM that you attach is likely to throw...

[mfreed7]

cloneable being opt in makes more sense for whatwg/dom#1246 and co, actually. Otherwise any new non-declarative shadow DOM that you attach is likely to throw...

Gah, that's a very good point. That would completely break the use case for attachShadow() emptying and returning the declarative shadow root - to avoid breaking old components that don't know about SSR.

I'm starting to wonder about the rest of the behavior in whatwg/dom#1246 actually. I wonder if the behavior should be that it throws if the call to attachShadow() explicitly passes an argument that disagrees with the existing shadow root, but doesn't throw if that parameter is not passed. E.g.

<div><template shadowrootmode=open></template></div>
<script>
  div.attachShadow({mode: open, clonable: false}); // Throws - clonable mismatch
  div.attachShadow({mode: open}); // Don't throw because `clonable` not passed explicitly
</script>

[annevk]

Why did we make declarative shadow roots clonable by default? whatwg/dom#831 and whatwg/dom#1137 are not very clear on this.

Also, hasn't this already shipped as part of declarative shadow trees?

[mfreed7]

Why did we make declarative shadow roots clonable by default? whatwg/dom#831 and whatwg/dom#1137 are not very clear on this.

Chrome's originally shipped behavior (and initial spec PR) only cloned declarative shadow roots inside template elements. It modified only the template cloning steps. As part of the review, that was changed to the currently spec'd behavior with the clonable flag. At the time that change was made, I didn't realize the implications of that for DSD roots outside templates. (I should have.)

Having said all of that, the use case was, and still is, to allow declarative shadow DOM to be used in a <template> that gets stamped out and still contains the shadow roots. Cloning general shadow roots is perhaps not a real world use case, I'm not sure. I'd be happy if we went back to the original proposal of just cloning shadow roots within templates, but I'm open to suggestions. Perhaps clonable can be made to be true by default only for DSD within a <template>? That would at least be web compatible for sure.

Also, hasn't this already shipped as part of declarative shadow trees?

The behavior that already shipped (in 2020 originally, and no change was made to this part in the re-shipment with shadowrootmode in 2023) is to clone shadow roots found in <template>s.

[annevk]

I think it would be better if:

1. By default they do not clone, if that is indeed the desired behavior.
2. Adding a shadowrootclonable attribute makes them clonable.

So if you use them inside template you'd have to add that attribute.

[mfreed7]

I think it would be better if:

1. By default they do not clone, if that is indeed the desired behavior.
2. Adding a shadowrootclonable attribute makes them clonable.

So if you use them inside template you'd have to add that attribute.

I think I like this proposal. It's a bit of a change from the existing shipped behavior, so there might be compat issues. But it feels like there's a very straightforward migration path for that breakage, to simply add shadowrootclonable. And it feels a lot less risky than the currently-spec'd behavior.

I'm willing to give this a try to see what the compat implications are, if there's consensus enough to get that change landed in the spec first?

[emilio]

That sounds reasonable to me, and makes clonable consistent with all other attributes, and with itself in the scripting and non-scripting case.

[mfreed7]

Alright, I wrote that up in #10117. It'll have a merge conflict with #10069 once that lands, but I can fix it up after that. The DOM PR (whatwg/dom#1246) doesn't need changes to accommodate this, since it already checks clonable and that now seems ok since clonable is opt-in everywhere.

If we're good with this, let's land the spec changes, and I'll try shipping this new behavior in Chrome to make sure it's web compatible.

[justinfagnani]

We chatted about this on the Lit team yesterday, and I think we'd like a bit more discussion on the intention and use cases for this feature if possible.

I'm somewhat sympathetic to the idea that trees with declarative shadow roots should be clonable with the idea that one would expect a deep clone to produce the same tree and that it functions the same. Was that the intention behind this feature?

The problems arise after the yet-to-be-cloned shadow root is modified. One way around those problems would be making the shadow root non-clonable during hydration, so it could still be cloned when we knew it had all the correct structure to be hydrated but not clone when it may be out-of-sync.

So, could clonable be mutable?

[mfreed7]

So, could clonable be mutable?

I was previously reluctant to do this, but now, having implemented the rest of this feature, I think that'd be trivial. The clonable flag is just checked at clone time, so there's no harm (and nothing to be done) when flipping it between false/true.

@annevk @emilio any objections to that small change? If not I'll incorporate it.

[emilio]

Any objections to that small change? If not I'll incorporate it.

I think that same simplicity argument could apply as well to other members (delegatesFocus at least).

I'm not sure I object but then throwing for those in attachShadow seems silly (could be better to "override only if present" or so). So maybe worth doing / discussing in a separate issue.