-
Notifications
You must be signed in to change notification settings - Fork 378
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
Finding a definition for disconnected elements #1040
Comments
The reason why the proposal doesn't have elements remember their registry is because the Chrome DOM team at the time was very reluctant to adding another reference to all elements to point to the registry due to memory concerns. That changed the design to dynamically looking up the registry by using the element's current root node if the root node is a ShadowRoot. Maybe there's a pattern where only root node elements created within a scope need to store their registry, and all other elements don't store a registry. I'm not sure if most DOM implementations put some references into a dynamic map (I think Blink calls this "rare data" maybe?), but that could reduce the memory overhead. |
100%I can't think of how this can be allowable. It's going to throw many people off. |
After discussion at the April F2F. Implementor consensus seemed to point to a concern over memory use related to the need to track an element's "active registry." An alternate proposal that seemed more feasible was to add const d = shadowRoot.createElement('div');
d.setHTML(`<x-foo></x-foo>`, {registry: shadowRoot.registry}); And effectively this would do the same thing as: setHTML = (node, html, registry) => {
const template = document.createElement('template');
template.innerHTML = html;
registry.upgrade(template.content);
node.replaceChildren(...template.content.childNodes);
} |
tpac meeting notes:
|
It's clear that we want to use the scoped custom element registry in the case of setting innerHTML to an element in an element newly created for a scoped custom element registry: function createConnectedShadowTree(registry) {
const host = document.createElement('div');
const shadowRoot = host.attachShadow({mode: 'closed', registry});
document.body.appendChild(host);
return shadowRoot;
}
const registry = new CustomElementRegistry;
class SomeElement extends HTMLElement { };
registry.define('some-element', SomeElement);
class OtherElement extends HTMLElement { };
registry.define('other-element', OtherElement);
const shadowRoot = createConnectedShadowTree(registry);
const someElement = shadowRoot.createElement('some-element');
someElement.innerHTML = '<other-element></other-element>';
someElement.querySelector('other-element') instanceof OtherElement; // This should evaluate to true. But what happens when such an element gets inserted into another shadow tree with a different scoped registry? Or what happens if the same element gets removed from the shadow tree? class OtherElement1 extends HTMLElement { };
customElements.define('other-element', OtherElement1);
const registry1 = new CustomElementRegistry;
class SomeElement extends HTMLElement { };
registry1.define('some-element', SomeElement);
class OtherElement2 extends HTMLElement { };
registry1.define('other-element', OtherElement2);
const registry2 = new CustomElementRegistry;
class OtherElement3 extends HTMLElement { };
registry2.define('other-element', OtherElement3);
const shadowRoot1 = createConnectedShadowTree(registry1);
const shadowRoot2 = createConnectedShadowTree(registry2);
const someElement = shadowRoot1.createElement('some-element');
someElement.innerHTML = '<other-element></other-element>';
someElement.querySelector('other-element') instanceof OtherElement2; // This should evaluate to true.
shadowRoot2.appendChild(someElement);
someElement.innerHTML = '<other-element></other-element>';
someElement.querySelector('other-element') instanceof OtherElement1; // This should evaluate to true.
someElement.remove();
someElement.innerHTML = '<other-element></other-element>';
someElement.querySelector('other-element') instanceof OtherElement1; // Should this evaluate to true? |
I think the ideal behavior is that the registry lookup acts similar to Therefore I'd expect the last 2 instanceof's to be |
Oh oops, I meant to say |
Also, we have to deal with cases where elements from different scoped custom element registries coexist:
|
Not the original registry, right? Just the last one it was attached to... |
Using the logic I'm proposing:
|
What happens if we removed |
Using the rule in #1040 (comment), removing should not change the "active registry" so the result is unchanged. |
Ok, other examples: class OtherElement1 extends HTMLElement { };
customElements.define('other-element', OtherElement1);
const registry1 = new CustomElementRegistry;
class SomeElement extends HTMLElement { };
registry1.define('some-element', SomeElement);
class OtherElement2 extends HTMLElement { };
registry1.define('other-element', OtherElement2);
const registry2 = new CustomElementRegistry;
class OtherElement3 extends HTMLElement { };
registry2.define('other-element', OtherElement3);
const shadowRoot1 = createConnectedShadowTree(registry1);
const shadowRoot2 = createConnectedShadowTree(registry2);
shadowRoot1.innerHTML = '<div></div>';
shadowRoot1.querySelector('div').innerHTML = '<some-element></some-element>';
const otherElement = shadowRoot2.createElement('other-element');
shadowRoot1.querySelector('some-element').appendChild(otherElement);
otherElement.innerHTML = '<other-element></other-element>';
const someElements = shadowRoot1.querySelectorAll('some-element');
someElements[0] instanceof OtherElement2; // Should this evaluate to true?
someElements[1] instanceof OtherElement2; // Should this evaluate to true? |
Okay, so I think I understand @sorvell's proposal. An element is associated with the registry of a shadow tree to which it last belonged. To put it another way, every time an element is inserted into a shadow tree, its associated registry changes to the registry of that shadow root. @annevk and I discussed an alternative approach, which is to permanently associate each element with the registry from which it was created. Previously, we thought this will incur extra pointer per element and therefore not permissible in terms of memory cost. But we can mostly avoid this cost if the common scenarios involve all nodes in a given shadow tree sharing the same registry. In that scenario, we can continue to associate the shared registry with its |
What happens in this case and would this be a breaking change? customElements.define('x-foo', class XFoo extends HTMLElement {});
customElements.define('x-bar', class XBar extends HTMLElement {});
const template = document.createElement('template');
template.innerHTML = `<x-foo></x-foo>`;
const clone = template.content.cloneNode(true);
// it has the registry from the template content's document which is empty?
const xFoo = clone.firstChild;
document.body.append(clone);
xFoo instanceof XFoo // is this true?
xFoo.innerHTML = `<x-bar></x-bar>`;
xBar = xFoo.firstChild;
xBar instanceof XBar // is this true? |
I think if the null registry case behaves the same as "I need to look at my parent for the registry" there would not be a breaking change. |
In response to #1040 (comment), following the rule in the explainer: someElements[0] instanceof OtherElement2; // Should this evaluate to true?
// false, this is otherElement, which is OtherElement3
someElements[1] instanceof OtherElement2; // Should this evaluate to true?
// true, since it was created while attached to a root with registry1 |
Noting that this is a change from the proposal in the explainer which says the registry is found by getting the context node's root (aka What happens here? const registry1 = new CustomElementRegistry();
registry1.define('x-foo', class XFooScoped extends HTMLElement {});
const shadowRoot1 = createConnectedShadowTree(registry1);
const div = document.createElement('div');
shadowRoot1.append(div);
div.innerHTML = `<x-foo></x-foo>`;
xFoo = div.firstChild;
xFoo instanceof XFooScoped // is this true?
customElements.define('x-foo', class XFooGlobal extends HTMLElement {});
xFoo instanceof XFooGlobal // is this true? |
In that case the |
I think it's likely important to be able to understand what will happen when setting shadowRoot1.append(div1);
shadowRoot1.append(div2);
div1.innerHTML = `<x-foo></x-foo>`;
div2.innerHTML = `<x-foo></x-foo>`; It seems fairly unintuitive to me that these |
I think all models have some amount of non-intuitiveness, but not changing the registry seems the most predictable. If we need an API for testing we could consider adding something to WebDriver or some such, for sure. It's usually best to wait a bit though and develop these things in baby steps. Doing it all at once usually results in oversights. |
It seems most predictable to me to always use the registry of your root node, and this was the idea in the proposal. As far as I can tell, there hasn't been an explicit argument made about why to change this other than implementation feasibility. If I understand correctly, the case that requires more information than Perhaps we can focus on that case and consider independently how it should behave such that it is both feasible to implement and the behavior is acceptable. It seems like we have these options for this case:
Which of these would have implementation feasibility concerns? Only (1)? From a behavior perspective, I think (1) makes most sense, but probably only (4) would be unacceptable. |
It wasn't really about implementation feasibility. I think it's rather weird behavior that when you create an element in registry1 and then append it to some subtree of registry2 further new child elements will now use registry2, despite the element being from registry1. |
This makes sense to me because an element's children are not part of the shadowRoot it "owns" and where it can have an explicit registry. They are part the element's public API that it shouldn't really know much about or have much control over. |
I should have taken a bit more time to fully explain where my idea came from. I was thinking that apart from making the registry association with the element immutable, another benefit this brings is that it would allow for registries to be independent of shadow roots. We often had community feedback that features are too tied to shadow roots and this feature doesn't have to be. This would make it possible to reuse elements from various libraries in your document without adopting shadow roots at all. |
Assuming the behavior described in the explainer's Finding a custom element definition section, consider the following.
x-foo
in the global and a scoped registry.el = shadowRoot.createElement('div')
.x-foo
viainnerHTML
:el.innerHTML = '<x-foo></x-foo>'
.I believe this results in an
x-foo
element upgraded via the global registry, and that behavior seems surprising and unexpected. I would expect in that case thex-foo
element would use the scoped registry definition.Perhaps this could be addressed if elements had an
ownerRegistry
setting (similar toownerDocument
) that could be used to lookup the correct definition.The text was updated successfully, but these errors were encountered: