w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.48k stars 660 forks source link

[css-properties-values-api] "Property registration cannot be scoped" differs from all implementations in a consistent way #10541

Open noamr opened 3 months ago

noamr commented 3 months ago

See https://drafts.css-houdini.org/css-properties-values-api/#shadow-dom According to this, @property descriptors work on the flattened tree and don't consider shadow boundaries. However, no implementation respects this, at least with regards to initial values. See https://css-names-in-the-shadow.glitch.me/property.html: Basically all implementations currently only respect @property declarations that come from the document scope. Perhaps the spec should be amended to reflect that?

css-meeting-bot commented 4 weeks ago

The CSS Working Group just discussed [css-properties-values-api] "Property registration cannot be scoped" differs from all implementations in a consistent way.

The full IRC log of that discussion <bkardell_> just will note for the record that I'm not sure people really like parts
<khush> astearns nobody matches the spec issue. yay
<khush> noamr repeat of the prev one with @property
<khush> i opened bugs on all browsers. browsers match but don't match the spec
<khush> prop descriptor has spec text that says you can define it in any scope and they are global
<khush> but they actually work in the global document only. prop descriptors from shadow dom are ignored
<khush> they are diff from everything else, global and inherited by the shadow tree
<khush> current way is not bad, it's actually like an api contract
<khush> if you have a custom element which uses custom props, you have to import the CSS which will have prop declarations. rather than relying on shadow root to declare those props. it's like a header file
<flackr> q+
<khush> ideal resolution would be prop descriptor have to be in the Document
<astearns> ack flackr
<khush> flackr i agree that it's a better model for props which are meant to be used outside. but for the ones meant to be encapsulated, would be nice if they were defined internally only
<astearns> +1 to flackr
<noamr> q+
<astearns> ack TabAtkins
<khush> TabAtkins: any shadow internal custom props being in capable of being defined, unless you also include a script or stylesheet globally is bad
<emilio> q+
<khush> the fact that they can't be scoped at all is a consequence of inheritance. being able to define them internally with a well named internal semantic is worth it
<khush> i can accept it being global only.
<astearns> ack noamr
<khush> noamr one way to resolve this. problem with internal props is that it could be a mismatch. so if it's defined internally, let's make it not inherited
<khush> then this is your own property, deal with it
<khush> your prop would fallback to what you defined in the descriptor as initial value and not inherit from global scope
<khush> TabAtkins what happens on the host element itself. can define something. otherwise the idea sounds reasonable
<astearns> ack emilio
<khush> emilio was gonna argue for currently implemented behaviour
<kizu> q+
<khush> as soon as there are name conflicts. you need to remove all the niceties of the shadow dom data encapsulated. have preference for the current behaviour. i need to think through, you have properties with same name. one is defined outside via part one is inside via regular stylesheet. that also seems fairly annoying. preference to just spec what is
<khush> currently there
<khush> TabAtkins i want to explore the behaviour noam was talking about. if we can find a less annoying version. but ok with specing current behaviour
<astearns> ack kizu
<khush> kizu: this is something i encounter without shadow dom. multiple sources define the same prop name with different values. if some element registers it with a different value, custom element on the host or regular element on itself. as soon as it reaches this by inheritance it will use initial value if there is a type mismatch
<khush> you define for your scope. if an internal one defines the same type then inheritance works otherwise it's overridden
<astearns> q+
<khush> TabAtkins if noam's idea is limited to defining on tree roots it matches your idea
<khush> and sounds reasonable behaviour to me
<westbrook> q+
<astearns> ack westbrook
<khush> astearns my chair hat off, i would accept what noam is proposing. prop def scoped to the shadow root. accept what's currently there, everything is global. don't accept what is currently implemented that shadow dom can't define props.
<iank_> q?
<khush> westbrook cross shadow boundary and change if there is something defined inside the shadow dom.
<emilio> q+
<khush> TabAtkins if inheritance carries a font face to the shadow, the font face being defined won't changge
<khush> if the outer face defines font face foo, and shadow defines the same. outer sets font-family: foo, it is referring to the outer one. when inherited it refers to the outer one
<khush> it won't use the internal one, name collsiion was accident
<khush> if the shadow says font-family foo, now it's the internal one
<astearns> ack emilio
<khush> emilio what is the implemention for font-face. gecko ignores them
<fantasai> strong +1 to Tab's point about binding font-family to @font-face before inheritance
<khush> TabAtkins it's consisntent. some ignore them internally, some make it global
<TabAtkins> s/consisntent/inconsistent/
<khush> emilio wouldn't want to expose font faces to document.fonts. haven't discussed this
<khush> TabAtkins that is undefined. happy to do reasonable stuff there
<khush> TabAtkins we don't have a version of .fonts for shadow trees
<noamr> q+
<khush> emilio all shadow roots have their fonts in document.fonts is not good behaviour
<astearns> ack noamr
<khush> TabAtkins figuring out reasonable behaviour there is more complicated
<chrishtr> q+
<khush> noamr prop is an API surface between shadow roots by definition. and the desc defines an interface point, fonts don't have any of this. they define something else. so the parallel is not right
<astearns> ack chrishtr
<khush> there's reason default scopung is not for prop
<khush> chrishtr let's take it back to the issue
<khush> astearns take it back to the issue. see if we can scope the prop definition.
flackr commented 4 weeks ago

Would it be possible for registered properties to be tracked as a tuple of (name, scope)? Each reference to a property could be based on the scope from which it is used making a name clash result in a second property value - which one the developer sees depends on the scope from which it is read.

noamr commented 4 weeks ago

What made sense to me (as mentioned in the minutes), is that @property in shadow-scoped styles makes the property "private" (not inherited from the outer scope), which fixes the type mismatch issue. So internal uses of the property would fall back to the @property's initial value rather than to what's inherited from the outer scope.

This can also be useful as a way to "undo" the @property definitions inherited from the outer scope.

flackr commented 4 weeks ago

What made sense to me (as mentioned in the minutes), is that @property in shadow-scoped styles makes the property "private" (not inherited from the outer scope), which fixes the type mismatch issue. So internal uses of the property would fall back to the @property's initial value rather than to what's inherited from the outer scope.

This can also be useful as a way to "undo" the @property definitions inherited from the outer scope.

I think this doesn't provide a clear answer for what to do about elements that can be styled from both inside and outside - e.g. :host or ::part() or also the property value observed inside slots.

My thinking was that with the tuple we could track both property values on these elements and inherit the outer value of the property inside of slotted elements.

noamr commented 4 weeks ago

What made sense to me (as mentioned in the minutes), is that @property in shadow-scoped styles makes the property "private" (not inherited from the outer scope), which fixes the type mismatch issue. So internal uses of the property would fall back to the @property's initial value rather than to what's inherited from the outer scope. This can also be useful as a way to "undo" the @property definitions inherited from the outer scope.

I think this doesn't provide a clear answer for what to do about elements that can be styled from both inside and outside - e.g. :host or ::part() or also the property value observed inside slots.

Those have to be defined in the outer scope. ::part() (and ::slotted) are an API surface, so the outer scope needs to know about it. They'll be naturally inherited like today.

My thinking was that with the tuple we could track both property values on these elements and inherit the outer value of the property inside of slotted elements.

Can you give an example of what this would look like?

tabatkins commented 4 weeks ago

Okay, thinking thru scoping @property. The big problem, which I cite in the spec, is the idea that an inheriting value can cross a boundary and suddenly be subject to a different registration. If the shadow didn't realize that it was going to be receiving this value (it intended the property only for internal use), this could be really weird.

Noam suggested having a scoped registration cause the property's inheritance to be blocked, so it can't accidentally receive a confusing value from the outside when it didn't intend it. I think this sounds pretty good; the big question is what to do on the host element - it exists in both trees and could be reasonably styled using either registration.

I suspect we should apply the scoped registration to the host element, and have it block inheritance from its parent. This allows (a) the shadow to set a value on the host element and have it inherit to everything (otherwise it has to set the value on all the top-level elements of the shadow, which is harder and odder), and (b) still allows the outer page to set the property on the host element to send a value into the shadow. It prevents the page using its own registration anywhere else from accidentally sending a weird value into the shadow.

It does mean that the outer page can't use its own property registration on the host element, but someone's gonna lose that tug-of-war; I think letting the shadow tree take it is better.


(This all assumes we're going with the simple mechanism of "a property only has one registration at a time on a given element", not something more complex like implicitly making custom properties a (tree-scope, property name) tuple.)

noamr commented 4 weeks ago

[tab's comment]

This makes sense to me, the descriptor applies to the shadow-DOM and to the :host shell

tabatkins commented 4 weeks ago

Enhancing custom property names to a (tree scope, property name) tuple does solve some of the issues:

It has some issues too:

noamr commented 4 weeks ago
  • a shadow tree that wants to register a custom property for the outer page to use on it is required to do so via a global stylesheet, communicated in a side channel. (or leave the property unregistered) If they register it in the shadow, the outer page won't be allowed to set it.

Isn't this true for both variants of this proposal? I wonder if we can solve it using the import/export mechanism we discussed on #10808 where we have an import/export semantic for those names to avoid the side-channel.

no idea how we'd expose these distinctions via the OM. The methods only take a property name. OK I understand what proposal means now. It's flexible which is good, but I think this will be extremely confusing. You can have the following be true:


/* in outer scope */
my-element { background: var(--accent )}

/ in inner scope / :host { color: var(--accent) }


In dev-tools or other reflection , it would look like:
```css
background: var(--accent);
color: var(--accent);

But the colors could be different based on where they're set.

noamr commented 4 weeks ago

Note that when you have a :host or ::part (which is where this matters), collisions are anyway an issue in the CSS properties themselves, so keeping that behavior for custom properties is consistent (though less flexible). Its somewhat easier to grasp a model where custom properties are like ordinary properties - an element can only have one --background-color in the same way that it has only one background-color, and authors should be careful when they use custom properties in :host and ::part just like they need to be careful with ordinary properties in these interface points.

flackr commented 4 weeks ago
  • a shadow tree that wants to register a custom property for the outer page to use on it is required to do so via a global stylesheet, communicated in a side channel. (or leave the property unregistered) If they register it in the shadow, the outer page won't be allowed to set it.

Isn't this true for both variants of this proposal? I wonder if we can solve it using the import/export mechanism we discussed on #10808 where we have an import/export semantic for those names to avoid the side-channel.

Yes, I think having some sort of export mechanism or maybe an attribute when you register the property would be ideal here. The nice thing about tree scoping the names in some way is that it would support simple aliasing to a different name outside.

no idea how we'd expose these distinctions via the OM. The methods only take a property name.

Yeah I think by default these methods would have to assume main frame (since the majority of legacy content would expect this), but maybe having an option to specify the scope so that you could get the expected property types and values for your component.

OK I understand what proposal means now. It's flexible which is good, but I think this will be extremely confusing. You can have the following be true:

/* in outer scope */
my-element { background: var(--accent )}

/* in inner scope */
:host { color: var(--accent) }

In dev-tools or other reflection , it would look like:

background: var(--accent);
color: var(--accent);

But the colors could be different based on where they're set.

Yeah, this is a feature which I think wouldn't be too bad in dev tools since these rules would come from different style blocks in which scope the variable would have different values due to the different scope.

Note that when you have a :host or ::part (which is where this matters), collisions are anyway an issue in the CSS properties themselves, so keeping that behavior for custom properties is consistent (though less flexible).

If the property was not exported to the top frame though, it is clearly unintended that they would conflict, so it is nice if they can be effectively isolated.

Its somewhat easier to grasp a model where custom properties are like ordinary properties - an element can only have one --background-color in the same way that it has only one background-color, and authors should be careful when they use custom properties in :host and ::part just like they need to be careful with ordinary properties in these interface points.

Even if a developer controls both scopes and is really careful, which scope's type / default value would you use on these elements?

The other question which is still a bit unclear to me, is would we be able to inherit a value on the outer scope set on the host in the parts if these conflict?

TLDR, not saying that we have to go with this more complex option, but I think it could have some nice flexibility to manage the conflict. I think we should consider some real use cases or find some means to work out whether it's worth the complexity of custom properties being tree scoped.

Note that making it a tuple on registered scope could be internally handled by some hash of the scope on the name which is internally removed before exposing the property name to the developer which could reduce the implementation complexity.

noamr commented 4 weeks ago

no idea how we'd expose these distinctions via the OM.

I think the clean way to expose this would be shadowRoot.getComputedStyle(element)

If the property was not exported to the top frame though, it is clearly unintended that they would conflict, so it is nice if they can be effectively isolated.

I agree that it's a nicer model, I'm not convinced yet that it's worth the subtle complexity it introduces.

Its somewhat easier to grasp a model where custom properties are like ordinary properties - an element can only have one --background-color in the same way that it has only one background-color, and authors should be careful when they use custom properties in :host and ::part just like they need to be careful with ordinary properties in these interface points.

Even if a developer controls both scopes and is really careful, which scope's type / default value would you use on these elements?

Like Tab suggested, the inner wins for :host. I think the outer needs to win for :part.

The other question which is still a bit unclear to me, is would we be able to inherit a value on the outer scope set on the host in the parts if these conflict?

No. You'd have to do this manually (use different names).

TLDR, not saying that we have to go with this more complex option, but I think it could have some nice flexibility to manage the conflict. I think we should consider some real use cases or find some means to work out whether it's worth the complexity of custom properties being tree scoped.

SGTM!

Note that making it a tuple on registered scope could be internally handled by some hash of the scope on the name which is internally removed before exposing the property name to the developer which could reduce the implementation complexity.

Not worried about internals at all, more about introducing subtle edge cases to the mental model where custom properties are scoped differently from regular properties. At some point saying "the flexibility/complexity ends here" is not a bad thing.

noamr commented 3 weeks ago

So to summarize the direction:

  1. The platform decides. Probably, internal declarations take precedence for::host, and external one for ::part, with perhaps an option to override with !important.
  2. Make CSS properties tree-scoped (as in, an element can have parallel values for the same name based on the style's tree scope). This has nice ergonomics but adds complexity, because now a CSS property key is not just a string (but rather a string/scope tuple), and elements can have two values for the same (string) property key.

Hoping to get @emilio's take on this :)

emilio commented 5 days ago

Probably, internal declarations take precedence for::host, and external one for ::part, with perhaps an option to override with !important.

That is the opposite to how usual cascade order behaves for :host tho, right? See also #6466.

That said I dislike the other option even more... having multiple registrations for the same property name seems rather unfortunate and confusing, and raises tons of questions about how would things like computed style enumeration work and such.