Skip to content
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

[scoped-registries] Describe interaction with declarative shadow DOM #915

Open
wants to merge 3 commits into
base: gh-pages
Choose a base branch
from

Conversation

justinfagnani
Copy link
Contributor

@justinfagnani justinfagnani commented Mar 12, 2021

Fixes #914

This adds a shadowrootregistry attribute for <template shadowrootmode> and ShadowRoot.prototype.scopedRegistry, as discussed in the String 2023 F2F meeting.

@justinfagnani justinfagnani changed the title Describe interaction with declarative shadow DOM [scoped-registries] Describe interaction with declarative shadow DOM Mar 12, 2021
@justinfagnani
Copy link
Contributor Author

@rniwa @mfreed7 I updated this PR with what we resolved from last weeks meeting, by adding ShadowRoot.prototype.scopedRegistry.

Can one of you review? Thanks!


The shadow root created by this HTML will have a `null` registry, and be in a "awaiting scoped registry" state that allows the registry to be set once after creation.

To identify this "no registry, but awaiting one" state, ShadowRoot will have a `scopedRegistry` boolean property. `scopedRegistry` will set to `true` for all ShadowRoots with a scoped registry, or awaiting a scoped registry. Code can tell that ShadowRoot has an assignable `registry` property if `root.registry === null && root.scopedRegistry === true`.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably have "shadowRoot.customElements" instead of "shadowRoot.scopedRegistry" to be consistent with the global registry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that would be shadowRoot.customElements instead of shadwoRoot.registry?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I find shadowRoot.customElements a bit harder to understand, as it's defined. In other words, this boolean is true "when the shadowRoot has a scoped registry, or is waiting for a scoped registry". I feel like "registry" should be at least somewhere in the name of the property, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rniwa meant that registry should be named customElements to match the global window.customElements.

shadowRoot.scopedRegistry would still be the boolean, if that's agreeable. (I don't think we do is or has prefixes in the DOM, correct?)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh! Thanks, I missed that. I agree with renaming shadowRoot.registry to shadowRoot.customElements, which is exactly what you both wrote, but I failed to read correctly.

The DOM does use is sometimes, e.g. isConnected, isTrusted, isMap, isContentEditable, etc. The has prefix (which seems like what we'd want here) is also used, though less so. For example, hasBeenActive. If has is acceptable, I'd definitely lobby for renaming shadowRoot.scopedRegistry to shadowRoot.hasScopedRegistry.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll change to hasScopedRegistry in this PR and we can see what other reviewers think.

As for registry vs customElements: that effects the argument to attachShadow() too. I think I should make a separate PR for both so the change is more obvious?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[scoped-registries] Interaction with declarative shadow DOM
3 participants