Skip to content

[ElementInternals] ElementInternals.shadowRoot is null but this.shadowRoot is not (its a ShadowRoot) #1365

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

Open
trusktr opened this issue Apr 6, 2025 · 8 comments

Comments

@trusktr
Copy link

trusktr commented Apr 6, 2025

What is the issue with the DOM Standard?

I've found a situation in which an element's ElementInternals.shadowRoot property is not in sync with the actual state of the DOM, i.e. when an open ShadowRoot is added to an element, element.shadowRoot !== elementInternals.shadowRoot.

This is reproducible in all browsers:

https://codepen.io/trusktr/pen/NPWmGMV

full code:
<script>
	customElements.define(
		'x-template',
		class extends HTMLElement {
			connectedCallback() {
				const parent = this.parentElement
				this.remove()
				const root = (() => {
					try {
						console.log('Attaching shadow root with mode open')
						return parent.attachShadow({mode: 'open'})
					} catch (e) {
						console.error('A second declarative shadow root cannot be created on a host.')
					}
				})()
				if (!root) return
				root.innerHTML = this.innerHTML
			}
		},
	)
</script>

<script type="module">
	customElements.define(
		'x-x',
		class extends HTMLElement {
			#internals
			constructor() {
				super()
				this.#internals = this.attachInternals()
				console.log(this.#internals.shadowRoot)
				console.log(this.shadowRoot)
				if (!this.#internals.shadowRoot)  this.attachShadow({mode: 'open'})
			}
		},
	)
</script>

<x-x>
	<x-template shadowrootmode="open">
		<button>c</button>
	</x-template>
</x-x>

This code,

console.log(internals.shadowRoot) // null
console.log(this.shadowRoot) // ShadowRoot

should either log two null values at any point in time, or two ShadowRoot values at any point in time when the root mode is open, for consistency.

When the root mode is closed (if you update the example), the output should be ShadowRoot null instead of null ShadowRoot, but the output is actually null null.

Either way, an error like the following will be show in console despite what the state says:

Uncaught NotSupportedError: Failed to execute 'attachShadow' on 'Element': Shadow root cannot be created on a host which already hosts a shadow tree.
@trusktr trusktr changed the title ElementInternals.shadowRoot is null but this.shadowRoot is not (its a ShadowRoot) [DSD] ElementInternals.shadowRoot is null but this.shadowRoot is not (its a ShadowRoot) Apr 6, 2025
@trusktr trusktr changed the title [DSD] ElementInternals.shadowRoot is null but this.shadowRoot is not (its a ShadowRoot) [ElementInternals] ElementInternals.shadowRoot is null but this.shadowRoot is not (its a ShadowRoot) Apr 6, 2025
@trusktr
Copy link
Author

trusktr commented Apr 6, 2025

The solution seems to be that the upgrade process should set the root's available to element internals to true before running a custom element constructor, as the custom element constructor is the first place for a custom elements author will typically use ElementInternals.

@trusktr
Copy link
Author

trusktr commented Apr 6, 2025

Why is a ShadowRoot available to element internals state even needed?

It is not possible to get ElementInternals before upgrade right? For example, calling attachInternals like this throws:

<script type=module>
	customElements.define( 'x-x', class extends HTMLElement { } )
	
	const t = document.createElement('template')
	t.innerHTML = `<x-x></x-x>`
	const el = t.content.children[0]
	el.attachShadow({mode: 'open'})
	const internals = el.attachInternals()
	console.log(internals.shadowRoot, el.shadowRoot)
</script>

CodePen demo

Based on the that, the only place that a custom element author can ever start to use ElementInternals is (should be) in the custom element constructor. And based on that, there seems to be no reason to have a ShadowRoot available to element internals state, and ElementInternals.shadowRoot should just show any existing root to the custom element author anyway (with or without that state).

Unless I missed something, it seems that

Are ElementInternals somehow available before an element's upgrade in some other way than in my last example? If so, it may not be worth fixing that, however fixing that would be better than the available to element internals problem we have here.

@annevk
Copy link
Member

annevk commented Apr 7, 2025

I recommend looking at blame as to why it was introduced.

@trusktr
Copy link
Author

trusktr commented Apr 8, 2025

It was introduced to prevent someone defining an non-defined custom element (element with dash in the name) later, and getting access to a closed ShadowRoot. That's not worth the problems this trick causes.

Why:

1) This issue causes another problem that is not so obvious: Custom Elements defined with synchronous script tags before parsing cannot use DSD, because this issue breaks CE code (constructors will see null instead of DSD ShadowRoot).

In order to use DSD, custom element definitions have to happen after parsing. This is a huge problem. I know people who have already sworn that defining elements up front is the best way to go for various reasons (name: avoid upgrade ordering issues, avoid flash of unstyled content).

And now, with this ElementInternals.shadowRoot problem, all of those people have to defer their script loading to use DSD, introducing upgrade order issues, and flashes of unstyled content.

2) DSD requires custom elements being able to accept declarative ShadowRoots, for server side rendering. ShadowDOM is not a security feature, right? Right? If not, then there's no strong need to have the odd behavior.


I believe ElementInternals needs to be in sync with the state of the element's ShadowRoot existence for the sake of developer sanity.

We're starting to more deeply explore the era of SSR for Custom Elements, and I don't want people who haven't tried SSR yet to hit a road block due to this. People have already had enough trauma from element upgrade ordering.


Strong vote to remove this limitation.

@trusktr
Copy link
Author

trusktr commented Apr 8, 2025

There's one more issue that needs to be addressed for DSD to be useful for everyone: both the element tag name, and the template, should be parsed together:

Example 1:

<my-el>

  <p>
    <!-- parser will now instantiate <my-el> because it knows there is no `<template shadowrootmode>` at the top --> 

Example 2:

<my-el>

  <template shadowrootmode="open">
    <!-- parser will now instantiate <my-el> because it knows it has a `<template shadowrootmode>` at the top --> 

I know that someone will say "this ship has sailed". Can we turn it around? So that we can simplify developer lives for CEs + DSD?

My hope and wish is for web specs to move forward without making things so complicated for CE users (vanilla web platform users).

@trusktr
Copy link
Author

trusktr commented Apr 8, 2025

@sorvell mentioned that to maintain BC, I think we will likely need something like:

parsingCompleteCallback() {
  if (!this.#internals.shadowRoot) {
     this.attachShadow(...);
  }
}

but this means it requires JavaScript. We are also thinking about a Declarative Custom Elements solution. We don't want those people to be required to use JavaScript for DSD.

@annevk
Copy link
Member

annevk commented Apr 9, 2025

You are combining too many things in a single issue. It's no longer clear to me what this issue is about. I'm quite certain however we don't want to revisit the design OP discusses.

@trusktr
Copy link
Author

trusktr commented Apr 18, 2025

(content deleted, need to think about this more to provide a concise explainer with solution ideas)

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

No branches or pull requests

2 participants