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

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

Open
noamr opened this issue Jul 8, 2024 · 20 comments

Comments

@noamr
Copy link
Collaborator

noamr commented Jul 8, 2024

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?

@noamr noamr added the Agenda+ label Jul 22, 2024
@noamr noamr changed the title [css-properties-values-api] "Property registration cannot be scoped" differs from implementations [css-properties-values-api] "Property registration cannot be scoped" differs from all implementations in a consistent way Jul 22, 2024
@astearns astearns moved this to TPAC/FTF agenda items in CSSWG Agenda TPAC 2024 Sep 13, 2024
@astearns astearns moved this from TPAC/FTF agenda items to Regular agenda items in CSSWG Agenda TPAC 2024 Sep 13, 2024
@css-meeting-bot
Copy link
Member

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
Copy link
Contributor

flackr commented Sep 27, 2024

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
Copy link
Collaborator Author

noamr commented Sep 27, 2024

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
Copy link
Contributor

flackr commented Sep 27, 2024

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
Copy link
Collaborator Author

noamr commented Sep 27, 2024

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
Copy link
Member

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
Copy link
Collaborator Author

noamr commented Sep 27, 2024

[tab's comment]

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

@tabatkins
Copy link
Member

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

  • host element can be styled with both versions of the property
  • so can ::part()

It has some issues too:

  • 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.
  • no idea how we'd expose these distinctions via the OM. The methods only take a property name.

@noamr
Copy link
Collaborator Author

noamr commented Sep 27, 2024

  • 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:

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

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

@noamr
Copy link
Collaborator Author

noamr commented Sep 27, 2024

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
Copy link
Contributor

flackr commented Sep 27, 2024

  • 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
Copy link
Collaborator Author

noamr commented Sep 27, 2024

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
Copy link
Collaborator Author

noamr commented Oct 1, 2024

So to summarize the direction:

  • A @property inside a shadow root makes the property private to that scope and not inherited from the ancestor scope
  • There are two options about how to deal with ::host and ::part
  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
Copy link
Collaborator

emilio commented Oct 21, 2024

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.

@noamr
Copy link
Collaborator Author

noamr commented Oct 30, 2024

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.

I think this is less about cascade order, and more about the notion of making shadow-defined property declarations not inherit from the parent tree.

If we allow that, it makes sense that the definition of "private" is for where the stylesheet was defined and not to where the element resides, because the @property declarations are part of a stylesheet and not part of an "element".

Since :host is defined in the inner scope, it should share the inner scope definitions.
Since ::part is defined in the outer scope, it should share the outer scope definitions.

I'm not sure if this is entirely in line with #6466, or if it should/can be.

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.

Yea, I am hesitant to go that far. A property should have only a string as a key, we've complicated the world enough (despite the benefits of that solutions on its own accord)

@emilio
Copy link
Collaborator

emilio commented Oct 30, 2024

Since :host is defined in the inner scope, it should share the inner scope definitions.

But rules from :host inherit through the inner and outer scopes, right?

And what if the same property is specified on a shadow host via regular selectors from the outer tree, and :host from the inner tree?

@noamr
Copy link
Collaborator Author

noamr commented Oct 31, 2024

Since :host is defined in the inner scope, it should share the inner scope definitions.

But rules from :host inherit through the inner and outer scopes, right?

Let's look at an example:

/* in outer scope */
@property --some-color {
  syntax: "<color>";
  inherits: false;
  initial-value: red;
}

body {
  outline-color: var(--some-color);
}

my-calender { 
  background-color: var(--some-color); 
}

/* in inner-shadow style of my-calendar */
@property --some-color {
  syntax: "<color>";
  inherits: false;
  initial-value: black;
}

:host { 
  color: var(--some-color);
}

/* a table inside the shadow tree */
table {
  border-color: var(--some-color);
}

I'd probably expect the background to be red and the color + the internal table's border color to be black.
What @flackr suggested in #10541 (comment) is what would achieve this, but it complicates the model in terms of what a "custom property" is.

It's clear that the table's border-color should be black and the body outline color should be red.
However, it's unclear what the my-calender color should be. If we say properties are private, then the host should also not inherit them. That's the "special" thing about :host - it's indeed in the light DOM, but it's part of the custom element.
However, this expectation is not obvious and in some cases you might want something different...

Perhaps this can be another @property descriptor that defines whether this property can pierce shadow roots, as long as the is the same, and if so it would just use the initial-value as a new default in case it's not delegated?

something like scope: private | inherit that makes this explicit, with a private default and fall back to private when there is a syntax mismatch.

@emilio
Copy link
Collaborator

emilio commented Nov 1, 2024

I think I agree with you in your test-case, but also that test case is not the most useful thing to look at, because the fallback to the initial value on a non-inherited property is probably not the most common thing.

Things like having an inherited property set outside, but with a conflicting definition inside the shadow tree seems more concerning, and I'm not sure I have a right answer there...

@emilio
Copy link
Collaborator

emilio commented Nov 1, 2024

You need to inherit the properties from outside, otherwise you get really unintuitive behavior with slotted contents even when there's no conflicts.

@noamr
Copy link
Collaborator Author

noamr commented Nov 1, 2024

You need to inherit the properties from outside, otherwise you get really unintuitive behavior with slotted contents even when there's no conflicts.

OK I see what you're saying.
I think the alternatives are either to make shadow property definitions apply to :part and not apply to :host, or do something more elaborate only when it comes to conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Thurs afternoon
Development

No branches or pull requests

6 participants