Skip to content

Allow passing null for customElementRegistry#1386

Merged
annevk merged 5 commits intomainfrom
annevk/shadowrootinit
Jul 1, 2025
Merged

Allow passing null for customElementRegistry#1386
annevk merged 5 commits intomainfrom
annevk/shadowrootinit

Conversation

@annevk
Copy link
Copy Markdown
Member

@annevk annevk commented Jul 1, 2025

Apparently people are passing a ShadowRoot instance as the argument to attachShadow().

Context: https://bugs.webkit.org/show_bug.cgi?id=295174. We'll add tests as part of fixing this in WebKit.

  • At least two implementers are interested (and none opposed):
    • WebKit
    • Chromium
    • Gecko
  • Tests are written and can be reviewed and commented upon at:
    • See above.
  • Implementation bugs are filed:
    • Chromium: Doesn't have an implementation yet.
    • Gecko: Doesn't have an implementation yet.
    • WebKit: https://bugs.webkit.org/show_bug.cgi?id=295174
    • Deno (only for aborting and events): …
    • Node.js (only for aborting and events): …
  • MDN issue is filed: N/A
  • The top of this comment includes a clear commit message to use.

(See WHATWG Working Mode: Changes for more details.)


Preview | Diff

Apparently people are passing a ShadowRoot instance as the argument to attachShadow().

Context: https://bugs.webkit.org/show_bug.cgi?id=295174. We'll add tests as part of fixing this in WebKit.
@annevk annevk requested review from domenic, keithamus and smaug---- July 1, 2025 05:16
Comment thread dom.bs
boolean clonable = false;
boolean serializable = false;
CustomElementRegistry customElementRegistry;
CustomElementRegistry? customElementRegistry = null;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe add a comment here explaining why we're departing from our usual preference to only have one way of signaling absence.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, some comment would good, since this is a rather odd case. Wouldn't have guessed anyone using attachShadow that way, but don't see reason why not.

@annevk annevk merged commit 780b8dd into main Jul 1, 2025
2 checks passed
@annevk annevk deleted the annevk/shadowrootinit branch July 1, 2025 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants