[scoped-registries] Moving elements with shadow roots between documents

[caridy]

follow up from #895

This issue has been discussed briefly multiple times (#716, #865 (comment)), without a clear consensus. We could not get to it during the recent F2F meeting, and instead we decided to try to resolved via an issue.

After reviewing the current state of the spec, and current implementations, I now believe that what @rniwa was saying was correct (not a surprise). Scoped Custom Element Registries are fundamentally incompatible with moving custom element instances between documents. Here is my take on this:

Current state of affairs

1. Moving custom element instances between documents is not very common, but still possible. After creating a custom element somehow, in another document, the element can be moved into a new document without too much hazard (note: it is not hazard free, but the risk is pretty small).

2. Moving custom element declarations between documents is not disallowed, but the creation of instances via document.createElement will throw, and creation of those via new is not different from point 1.

3. Instances moved are hazardous because they could incur on invalid operations, e.g.: an instance that when connected, attempt to populate its shadow root (which is a common operation) where elements used by the shadow are not registered in the destination document (new owner document).

4. Invariant: Any element created from markup or document.createElement must be registered in the document itself, and must produce an instance of the corresponding HTMLElement.

The problem with Scoped Registry

When an instance is moved (see 3), and any element created from markup or shadowRoot.createElement, from that point on, must produce an instance of the ownerDocument's corresponding HTMLElement. This is the fundamental problem, because that registry was most likely created and populated by the declaration, or instantiation of the custom element. E.g. of such broken code:

const registry = new CustomElementRegistry();
registry.define('x-child', SomeConstructor);
export class extends HTMLElement {
    constructor() {
        super();
        this.attachShadow({ mode: 'open', registry });
    }
    connectedCallback() {
        this.shadowRoot.innerHTML = '<x-child></x-child>'
    }
}

Proposed solution

This proposed solution assumes that moving custom elements with scoped registries is going to be a very very rare situation:

A. Use the same protection mechanism used today by the global registry. Fail with A newly constructed custom element belongs to a wrong document (Safari) or a similar error when needed.

B. Allow a new registry to be set into an existing shadow root (a setter on ShadowRoot.prototype.registry), that way the authors expecting elements to be moved between documents can do the extra work of redefining the registry for their necessary elements when an instances are moved around.

cc @justinfagnani @rniwa @leobalter @annevk

[annevk]

Where you mention "moving", that's actually three operations: remove and insert, whereby insert adopts and invokes the adoptedCallback for custom elements so they can adjust to the new document.

I don't really understand where Safari would throw today (and if it's only Safari, is that a bug in other browsers or a bug in Safari not following the standard?).

And yeah, changing the registry upon adoption is an option, though I'm not sure it should be a setter as that would give much more opportunities for changing it than are warranted by the use case.

[caridy]

@annevk thanks for the quick feedback, few notes:

Where you mention "moving", that's actually three operations: remove and insert, whereby insert adopts and invokes the adoptedCallback for custom elements so they can adjust to the new document.

Yes, moving connected elements between documents or inserting an element created from another document, which, as a result, kick off the adopting routine.

I don't really understand where Safari would throw today (and if it's only Safari, is that a bug in other browsers or a bug in Safari not following the standard?).

I was using safari as a reference, but all browsers are throwing a similar error when doing number 2. Here is a simple example:

    class Foo extends iframe.contentWindow.HTMLElement {}
    iframe.contentWindow.customElements.define('x-foo', Foo);
    // moving declaration to another global registry (which doesn't fail but cause other errors later on)
    customElements.define('x-foo', Foo);
    const elm = document.createElement('x-foo'); // throws because of number 4 invariant violation 

There are the errors that you can see today:

• Safari: NotSupportedError: A newly constructed custom element belongs to a wrong document
• Chrome: DOMException: Failed to construct 'CustomElement': The result must be in the same document
• FF: DOMException: Operation is not supported

And yeah, changing the registry upon adoption is an option, though I'm not sure it should be a setter as that would give much more opportunities for changing it than are warranted by the use case.

I have raised other use-cases in the past, e.g.: switching the template used to populate the shadow root could also require different entries in the registry, which is only possible if the registry is interchangeable. I believe the original proposal from @justinfagnani allows that (it only access the registry when needed). And this new use-case is just another reason for having the setter in place.

Another use-case is the integration with Declarative Shadow DOM. If you want to reuse the markup (no attaching a new shadow), you must have a way to link it to a registry to get those elements upgraded (cc. @mfreed7)

[leobalter]

@rniwa @annevk ping. I'd like your feedback here, please! This is very important for me to know how to move this forward on the proposal/draft specs. TIA!

And yeah, changing the registry upon adoption is an option, though I'm not sure it should be a setter as that would give much more opportunities for changing it than are warranted by the use case.

@annevk, considering the other use cases pointed out by @caridy, maybe the setter seems ok? (I might be just repeating Caridy's comment) I believe the key point is actually to have the registry interchangeable and upgrading elements.

[annevk]

I don't understand how throwing for inconsistent documents during setup relates to adoption, since we don't throw for adoption and you can move custom elements around between documents.

It would help a lot to have a more detailed proposal as that would be easier to evaluate. And writing that out might also help discover some of the issues. @rniwa might be able to reason about the cost of changing registries off the cuff though.

[Jamesernator]

An alternative to throwing could be to have a single element name reserved for webcomponents that are not in the global registry e.g. something like <custom> or something. When an element is removed from a registry scope it's name would just become custom, but otherwise it would behave the same.

e.g.:

const registry = new CustomElementRegistry();
registry.define("foo-bar", FooBarElement);
const shadowRoot = someElement.attachShadow({ registry, mode });

shadowRoot.innerHTML = `<foo-bar baz="qux"></foo-bar>`;

// Inside the shadow root it has it's expected name
const el = shadowRoot.children[0]
el.localName === "foo-bar"; // true

el.remove();
document.body.append(el);

// Inside other roots where FooBarElement is not registered it becomes <custom>
el.localName === "custom"; // true

// <custom> elements created normally would not do anything special and could not be upgraded
const custom = document.createElement("custom");
custom.constructor === HTMLElement; // true

[trusktr]

#754 was closed (I think it should be left open).

Would that solve the issue?

• What if when a root ('s host) is removed from a document and then added to a new one, perhaps the elements are downgraded, then upgraded based on the new document's registry.
• What if when a root ('s host) is moved, and the root has a scoped registry, everything simply transfers over with everything still upgraded (nothing to do)?
  • If that's not viable, then it seems downgrade and re-upgrade could work?

[caridy]

@rniwa you were absolutely right, and I was very wrong on previous comments and discussions, and most of the confusion from my side was caused by a bug in chrome (https://bugs.chromium.org/p/chromium/issues/detail?id=1199886) related to the instances created by the parser after element with its shadow were moved around between documents. I hope that they fix this bug soon.

Anyhow, the situation that you explained in previous F2F meetings is indeed a problem with this proposal, because all modern browsers are readjusting accordingly when elements are moved around to use the global registry of the new owner document, and that behavior poses an interesting question with it comes to scoped registries: What will the UA do when an element with a shadow root with a scoped registry associated is moved into another document? From our previous discussions, there are a couple of alternatives:

1. "null" out the registry. technically, it is not really set to null, but to an empty registry of some sort so it doesn't fallback to the global one. basically, it swallow any potential issue (probably ok and aligned with the current behavior for the global registry).
2. expand the adoptedCallback's behavior so when the element is moved around, if it has an associated registry, it must fulfill certain invariants to reset the registry in user-land, otherwise it throws. basically, it is a responsibility of the developer to prepare their elements for the move (probably too aggressive)

I think we should go with 1) since it is similar to what will happen today with the global registry.

[justinfagnani]

Note that we already want registries to be settable so that script can add a registry to a declaratively created shadow root. With that the behavior of moving a shadow root to a new document can work similarly to declarative created shadow roots - the registry is set to an empty registry and can be replaced later by script, in this case probably in the adoptedCallback.

[rniwa]

Note that we already want registries to be settable so that script can add a registry to a declaratively created shadow root. With that the behavior of moving a shadow root to a new document can work similarly to declarative created shadow roots - the registry is set to an empty registry and can be replaced later by script, in this case probably in the adoptedCallback.

We definitely don't want a scoped registry to be settable everywhere. That will lead to all sorts of havocs like changing the registry of a custom element in the middle of constructing or upgrading an element.

[caridy]

Note that we already want registries to be settable so that script can add a registry to a declaratively created shadow root. With that the behavior of moving a shadow root to a new document can work similarly to declarative created shadow roots - the registry is set to an empty registry and can be replaced later by script, in this case probably in the adoptedCallback.

We definitely don't want a scoped registry to be settable everywhere. That will lead to all sorts of havocs like changing the registry of a custom element in the middle of constructing or upgrading an element.

Oh! that's a very interesting observation @rniwa, I wasn't aware that the registry was also sensible to this kind of issue. To try to understand better this issue, this means that the element has a reference back to the registry (directly or indirectly) in order to carry on the node reactions (connect/disconnect/adopt/etc.). Is this statement correct? For some reason, I was assuming that the node was cache these during creation via internal slots. As for the construction/upgrade, I'm not sure I understand what the issue is, is it because the result of the construction process must return an instance that is valid for the associated registry, and this might not be true if the registry changes during the construction process?

What alternatives do you see here?

[rniwa]

We definitely don't want a scoped registry to be settable everywhere. That will lead to all sorts of havocs like changing the registry of a custom element in the middle of constructing or upgrading an element.

Oh! that's a very interesting observation @rniwa, I wasn't aware that the registry was also sensible to this kind of issue. To try to understand better this issue, this means that the element has a reference back to the registry (directly or indirectly) in order to carry on the node reactions (connect/disconnect/adopt/etc.). Is this statement correct? For some reason, I was assuming that the node was cache these during creation via internal slots.

That's not what we do in WebKit. But I'm not certain what problem(s) that kind of back reference will solve even if we did?

[I]s it because the result of the construction process must return an instance that is valid for the associated registry, and this might not be true if the registry changes during the construction process?

Yes. An upgrade of a custom element only makes sense in the context of a single custom element registry. Relatedly, if we're trying to upgrade a list of newly defined elements in define call, changing the registry of the shadow root mid way through it would result in non-sensical results.

[caridy]

@rniwa we were able to debated this topic extensibly during the virtual F2F meeting today (you were missed there for sure). Please, read the notes. Here are my takeaways from the meeting:

1. General consensus around a non-settable registry as you suggested. definitely seems problematic for other implementers. the caveat here is the intersection semantics with declarative shadow root, in which case it seems we can take a similar approach to the attach shadow mechanism, a bit in the shadowRoot instance that determines whether or not the registry can be set once (probably via elementInternals). @justinfagnani will work on defining that.
2. After talking about this topic for about 1hr, it seems that we started leaning toward preserving the registry from the original document. The general issue around releasing memory didn't have legs, and we discussed many potential shortcoming of trying to release the memory without exposing the GC mechanism (it seems that any work there could potentially expose the GC process to user-land).
3. It seems that keeping the original registry associated to the original shadow, even though the internal owner document is changing to the new document can still be achieved in a relative easy manner (according to the other implementers in the room) by lifting some of the restrictions/validations during the construction phase of the elements.
4. Mason from google will be looking into the issue reported for Chrome, in principle, he thinks that what other browsers are doing with respect to resetting the parser is the way to go for this edge case, and hence all built-in elements created by the parsing process should be bound to the new owner-document. It seems that the issue was mostly around customization of built-ins, which is only supported by them.

Do you have any comments around these 4 bullet points? Maybe we can get @leobalter to organize something for a smaller group if you think we need more discussions around this topic.

[rniwa]

@rniwa we were able to debated this topic extensibly during the virtual F2F meeting today (you were missed there for sure). Please, read the notes. Here are my takeaways from the meeting:

1. General consensus around a non-settable registry as you suggested. definitely seems problematic for other implementers. the caveat here is the intersection semantics with declarative shadow root, in which case it seems we can take a similar approach to the attach shadow mechanism, a bit in the shadowRoot instance that determines whether or not the registry can be set once (probably via elementInternals). @justinfagnani will work on defining that.

Ok.

2. After talking about this topic for about 1hr, it seems that we started leaning toward preserving the registry from the original document. The general issue around releasing memory didn't have legs, and we discussed many potential shortcoming of trying to release the memory without exposing the GC mechanism (it seems that any work there could potentially expose the GC process to user-land).

The issue isn't about memory usage. It's an issue of a single registry being associated with multiple documents. The way WebKit's GC will work, it's highly problematic for custom elements in multiple documents to share a single custom element registry and the registry to have back reference to those custom elements for one reason or another (e.g. for parsing, upgrading, etc...).

The same issue came up during element reflection discussion but element reflection didn't pose as much of an issue as a scoped custom element registry because scripts can't observe that the weak reference from one element to another unless they're in the same tree such that they're already accessible from scripts in some other way. With a scoped custom element registry, being able to define a new custom element and upgrading existing custom elements in shadow trees associated with the registry will require the registry to have back reference to those shadow trees as they're observable.

This is highly problematic because:

1. It would mean that none of the shadow trees associated with a scoped custom element registry can ever be GC'ed until all shadow trees are no longer accessible from scripts, resulting in a very high memory usage.
2. The semantics of (1) isn't implementable in WebKit as things stand today unless we re-architect how GC works with DOM objects in WebKit.

[caridy]

@rniwa I'm still not clear about what the problem is. Specifically, I'm having a hard time understanding how is this any different with today's implementation with the global registry, I feel that I'm missing something obvious here. Let me try to articulate an examples:

1. A page with 3 same domain iframes, A, B and C
2. Iframe A gets definitions for elements x-foo and x-bar
3. Two instances of x-foo are created and each adopted by iframe B and C
4. Iframe A is detached, but both definitions (x-foo and x-bar) are still accessible from scripts

The example above seems like a good description of a similar problem exhibited today, which will prevent the GC to collect iframe A until after B and C are collected. How is this different from what we are discussion here in the context of a scoped registry?

[rniwa]

@rniwa I'm still not clear about what the problem is. Specifically, I'm having a hard time understanding how is this any different with today's implementation with the global registry, I feel that I'm missing something obvious here. Let me try to articulate an examples:

1. A page with 3 same domain iframes, A, B and C
2. Iframe A gets definitions for elements x-foo and x-bar
3. Two instances of x-foo are created and each adopted by iframe B and C
4. Iframe A is detached, but both definitions (x-foo and x-bar) are still accessible from scripts

The example above seems like a good description of a similar problem exhibited today, which will prevent the GC to collect iframe A until after B and C are collected. How is this different from what we are discussion here in the context of a scoped registry?

They're different in that in this example, only the element is shared between frames A, B, and C, not its registry. The problem with sharing a registry is that in order for upgrading via define to work, we'd need to keep track of all shadow roots with which the registry is associated. To put it another way, there is a semantic requirement that the registry needs to keep track of all shadow roots with which it is associated. There is no such a requirement in the above scenario because there is no custom elements feature or semantics which requires such a back reference.

[annevk]

That would also be required for #923, no? At least if you want the efficient-but-inconsistent-with-global-registry approach.

[rniwa]

That would also be required for #923, no? At least if you want the efficient-but-inconsistent-with-global-registry approach.

That's precisely why this is a problem. If it weren't for #923, we probably don't need such a back reference.