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

:unresolved vs :not(:upgraded) #418

Closed
rniwa opened this issue Mar 7, 2016 · 13 comments
Closed

:unresolved vs :not(:upgraded) #418

rniwa opened this issue Mar 7, 2016 · 13 comments

Comments

@rniwa
Copy link
Collaborator

rniwa commented Mar 7, 2016

It seems odd that the custom elements API and spec refers to upgrading custom elements and yet the CSS pseudo class that corresponds to a custom element not being upgraded is called :unresolved. unresolved doesn't tell us what hasn't been resolved and inconsistent with the rest of the spec / API terminology we're using.

Perhaps we should consider negating it to :upgraded and use :not(:upgraded) for this use case for consistency. I do admit the the latter form is a bit more verbose but I think consistency in the terminology is very valuable here.

@rniwa
Copy link
Collaborator Author

rniwa commented Mar 7, 2016

@rniwa
Copy link
Collaborator Author

rniwa commented Mar 7, 2016

I'm also fine with an alternative approach in which we replace every instance of the term "upgrade" to "resolve" in the spec if we think the term "resolved" is more descriptive than the term "upgrade".

@domenic
Copy link
Collaborator

domenic commented Mar 7, 2016

Another reasonable solution seems to be just changing it to :unupgraded.

@JanMiksovsky
Copy link

Until they're upgraded, the elements are of type HTMLUnknownElement, so how about :unknown?

That describes their current state using existing terminology, avoids use of :not, and is a simple, natural word.

It also has the benefit of not presuming that the unknown element is going to be upgraded using the Custom Elements API. There are JS frameworks that make use of non-standard tags, and they could use :unknown as well.

@domenic
Copy link
Collaborator

domenic commented Mar 7, 2016

Until they're upgraded, the elements are of type HTMLUnknownElement, so how about :unknown?

This isn't true actually. Second paragraph of https://w3c.github.io/webcomponents/spec/custom/#registering-custom-elements. It's HTMLElement.

@JanMiksovsky
Copy link

Ah, my bad, I'd been thrown off my experience with the v0 implementation in Blink.

Of the choices already listed, then, I'd prefer :unresolved (with the accompanying spec changes) or :not(:upgraded). While I like the intent of :unupgraded, I fear it will be hard for people to visually parse (especially if they're not familiar with the upgrade terminology) and it feels like a mouthful.

@domenic
Copy link
Collaborator

domenic commented Mar 8, 2016

One thing I realized is that using "upgraded" might not be the best idea given that some elements (those created by the parser and by createElement) never need to be upgraded. They just start out with the right class.

So maybe "resolved" = "upgraded" or "created with the right class in the first place".

What this really means is that :unupgraded means :unresolved, but :not(:upgraded) does not mean :not(:resolved). So I stand by :unupgraded as the best name :). But I am happy to go with anything except :not(:upgraded) since that is not accurate for what we want.

I will tentatively work from the conclusion that I should spec the concept of "resolved" to mean "upgraded" or "created with the right class in the first place" and spec that tomorrow. We'd keep :unresolved. Please comment and re-add "needs consensus" if you disagree, including if you think we should have :resolved and :not(:resolved).

@rniwa
Copy link
Collaborator Author

rniwa commented Mar 8, 2016

How about :not(:defined) since unresolved elements have yet to be defined by defineElement?

@annevk
Copy link
Collaborator

annevk commented Mar 8, 2016

:defined does not seem good since something can be defined but not upgraded (there's a timing difference, right?).

@rniwa
Copy link
Collaborator Author

rniwa commented Mar 8, 2016

@annevk : in theory, between when defineElement had finished defining the element and all Construct calls are made, you could observe an element that's already defined but not yet upgraded amongst your peers but I feel like that's such an edge case...

@domenic domenic closed this as completed in 2432819 Mar 8, 2016
@domenic
Copy link
Collaborator

domenic commented Mar 8, 2016

I went with :defined for now since I like that it makes all of the user-facing APIs match. We can definitely change it if people don't like it (feel free to reopen).

Now that we have a precise defined flag, I think there is less of a timing difference than you might imagine. I don't set the flag until after the constructor is completely finished running.

@rniwa
Copy link
Collaborator Author

rniwa commented Mar 10, 2016

Great. I uploaded a WebKit patch with tests on https://bugs.webkit.org/show_bug.cgi?id=155108. Please check to see if that matches your expectation.

teoli2003 pushed a commit to mdn/data that referenced this issue Nov 7, 2017
Per WICG/webcomponents#418, the current name for this pseudo-class is `:defined`.
depeele referenced this issue in lit/lit Jul 26, 2018
The template's document doesn't have a browsing context, so
custom elements aren't upgraded. This change defers upgrading
any custom elements inside the template until after the
treewalker that links the Parts to the DOM.
This is important because custom elements can modify their DOM
at upgrade time (e.g. when using ShadyDOM).

Fixes #231
@trusktr
Copy link
Contributor

trusktr commented Sep 2, 2020

Timing issues are still a problem. This works well:

<my-el id="el"></my-el>

<script>
	const defined = el.matches(':defined')

	// This code needs to do something once the element is defined.
	if (defined) {
		console.log('el is defined')
	} else {
		console.log('el not yet defined')
		customElements.whenDefined(el.tagName.toLowerCase()).then(() => {
			console.log('el is defined? ', el.matches(':defined'))
			// ^ ------------------------------------------------------------------- true (GOOD)
		})
	}
</script>

<!-- Definition happens sometime later: -->
<script>
	customElements.define('my-el', class extends HTMLElement {
		constructor() {
			super()
			console.log('my-el constructor')
		}
		connectedCallback() {
			console.log('my-el connected')
		}
	})
</script>

codepen example

But this does not work:

<script>
	const el = document.createElement('my-el')
	const defined = el.matches(':defined')

	// This code needs to do something once the element is defined (upgraded).
	if (defined) {
		console.log('el is defined')
	} else {
		console.log('el not yet defined')
		customElements.whenDefined(el.tagName.toLowerCase()).then(() => {
			console.log('el is defined? ', el.matches(':defined'))
			// ^ ----------------------------------------------------------------- false (BAD)
		})
	}
</script>

<!-- Definition happens sometime later: -->
<script>
	customElements.define('my-el', class extends HTMLElement {
		constructor() {
			super()
			console.log('my-el constructor')
		}
		connectedCallback() {
			console.log('my-el connected')
		}
	})
</script>

<!-- The element is appended sometime later: -->
<script>
  document.body.append(el)
</script>

codepen example

I think there should be a better solution to this. Some possible solutions:

  1. Upgrade the element reference even if it is not in the DOM. In the second example, the customElements.define call would run the constructor, and :defined would be true before the whenDefined.then callback runs. This would greatly improve intuitiveness of code that wishes to rely on whenDefined
    The current output of the second example is
    el not yet defined
    el is defined?  false
    my-el constructor
    my-el connected
    
    but the new output would be
    el not yet defined
    my-el constructor
    el is defined?  true
    my-el connected
    
  2. Provide a new builtin API to allow waiting for an element to be upgraded. f.e. el.addEventListener('defined', () => {...}) . This does not improve the intuitiveness of whenDefined code, but provides a workaround in a non-breaking way as this would not change behavior of existing code in the wild. (I still prefer the first option).

(cc @justinfagnani as you've discussed these sorts of issues in other topics too)

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

No branches or pull requests

5 participants