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-cascade] [css-nesting] Figure out whether we're fine with "shifting up" bare declarations after rules #8738

Closed
emilio opened this issue Apr 19, 2023 · 99 comments

Comments

@emilio
Copy link
Collaborator

emilio commented Apr 19, 2023

If you do:

div {
  color: green;
  @media (width > 0) {
    color: red;
    background: red;
  }
  background: green;
}

My understanding is that per spec the div color and background would be red.

That seems rather confusing. There are various alternatives here:

  • We're ok with this.
  • We forbid declarations after nested rules.
  • We deal with bare declarations by somehow sorting them together, so that would be effectively something like:
div {
  color: green;
  @media (width > 0) {
    color: red;
    background: red;
  }
  & {
    background: green
  }
}

Maybe something else?

cc @fantasai

@Loirooriol
Copy link
Contributor

We forbid declarations after nested rules

Seems potentially problematic that some people may have some garbage followed by lots of declarations, then at some point we add some feature that parses the garbage as a nested rule, and then all the declarations stop applying. Spooky action at distance.

One of the main characteristics of option 3 (later modified with lookahead) is that it allows freely mixing declarations and nested rules. So at this point I don't think it makes sense to restrict that. The proper way to have such restriction would have been choosing option 4 or similar (which BTW seemed better to me).

@tabatkins
Copy link
Member

Right, I argued in previous discussions that forbidding decls after rules is fundamentally problematic. The basic question is "when has a rule occurred?", and this needs to not be dependent on whether a rule is valid or not, as that would mean different browser levels would interpret following properties differently.

So you need to define some notion of when we've found a rule, that allows for invalid rules, and do so in a way that doesn't unduly restrict our future syntax options.

For example, if we ever allow {} in a property, and you write it invalidly so it triggers rule parsing, would that kick the "now there's a rule" switch? Does that mean we actually have to forbid ever using {} in properties?


On the other hand, sticking with the current design that just allows them, and relying on people to not write unreadable stylesheets, suffers from none of these issues.

@tabatkins tabatkins added the css-nesting-1 Current Work label Apr 19, 2023
@emilio
Copy link
Collaborator Author

emilio commented Apr 21, 2023

Not saying it's necessarily a good idea, but playing devil's advocate a bit, we already have that kind of concept for e.g. @import / @namespace / etc, don't we?

Imagine someone wrote this ten years ago:

:has(body) {}

@import "something.css";

(Or something along those lines)

Their @import would stop working on browsers that support :has(), yet that hasn't been ever a concern (and I don't think it should be).

The switch for that is "A valid rule has been parsed", and I think we could do the same, so that random garbage doesn't cause that action at a distance.

@tabatkins
Copy link
Member

I think these are pretty different situations in practice, tho. We add to the set of "things that can go before @import" super rarely -- in fact, we've only done it once, with @layer.

On the other hand, we add new things that would qualify for nesting fairly regularly - we recently added @scope, and just resolved to add @initial-name-tbd. So the set of things that may or may not trigger "no more properties from here on" is regularly changed, presenting new hazards.

Ultimately, the question is still - what are we trying to protect authors from? This sort of interleaving has been allowed in Sass and related tools for many years, with the exact same "pretend all the properties are together at the top" behavior, and never caused notable issues.

@SebastianZ
Copy link
Contributor

For example, if we ever allow {} in a property, and you write it invalidly so it triggers rule parsing, would that kick the "now there's a rule" switch? Does that mean we actually have to forbid ever using {} in properties?

If you mean the property value and I interpret the algorithm correctly, then {} is already allowed by consuming a component value in step 5 of consuming a declaration. And it can't trigger rule parsing because the consumed component value is appended to the declaration's value.

Sebastian

@tabatkins
Copy link
Member

This is in reference to the resolved-on new parsing algo that tries to parse as a decl, then falls back to parsing as a rule if the result wasn't valid. That algo isn't in the Syntax spec quite yet.

@cdoublev
Copy link
Collaborator

cdoublev commented May 11, 2023

That algo isn't in the Syntax spec quite yet.

Should we expect more changes to come (in CSS Syntax 3, at least) after this commit?

color: red; @media { foo: bar } color: green does not seem to parse as I would expect against <declaration-list>, probably because there is no ; after foo: bar. I think } should be provided as a stop token to consume a declaration.

@tabatkins
Copy link
Member

Thanks for spotting that! I hadn't gotten around to updating my own parser to the new text to test it, so I missed that I was mishandling nested constructs.

I believe it's all good now; the three construct consumption algorithms (at-rule, qualified rule, declaration) now all take a "nested" bool, which triggers them to stop early when encountering a top-level unmatched }. This is passed by "parse a block's contents" (which is renamed from "parse a declaration list" since it definitely returns more than just declarations).

@cdoublev
Copy link
Collaborator

cdoublev commented May 12, 2023

<declaration-list> is defined with consume a list of declarations and the procedure to parse the input of CSSStyleDeclaration.cssText is defined with parse a list of declarations. Both procedures are removed.

I do not mind waiting for an update to your parser to validate your updates. This would prevent me from polluting this issue with implementation details.

That said, <declaration-list> could be renamed to <statement-list> and could be parsed with consume a list of statements.

This would allow using <declaration-list> for when a list of declarations (strict) is required (eg. keyframe rule, @font, etc). But I do not know if it would be backward compatible to apply it to existing rules, and your intent may even be to preserve this flexibility to accept declarations/rules within any rule, for future extensibility.

@tabatkins
Copy link
Member

I've done the parser update, but had to stop at end of day before I could get my testing framework updated to the new data structures. Later today I'll have it working. ^_^

But I do not know if it would be backward compatible to apply it to existing rules, and your intent may even be to preserve this flexibility to accept declarations/rules within any rule, for future extensibility.

Yes, all rules already handled at-rules in their blocks even if they only accepted declarations validly, so that's staying, and at that point there's not really any reason to continue having a parsing split. All blocks are parsed the exact same way now in Syntax, accepting all three kinds of constructs (at rules, qualified rules, declarations).

My plan on reshuffling the productions is just to define one generic production, and then probably some sub-productions that automatically imply certain restrictions on what's valid so you don't have to say it in prose. But they won't change the parsing behavior.

@LeaVerou
Copy link
Member

Unfortunately, it appears that Chrome shipped before we could decide on this, and now authors are already blogging about the "gotcha".

Hopefully it's not too late to change this.
I think we should really try to avoid any rewriting that changes order of declarations.

Since we’re already resolved that nested MQs will wrap their contents with an implicit & {}, perhaps we can do the same thing with any declarations after the first nested rule, as @emilio proposes in the OP?

@tabatkins
Copy link
Member

I still feel very strongly that we should not change this, and the current behavior is the best. Again, this is the exact behavior that Sass (and I suspect other preprocessors) have had for a decade+ already, and it has not been a problem there. (Largely because people just don't write that code - they put their declarations first, then their nested rules.) I don't think we should try to add more behavior for something that has proven itself to not be a problem in practice.

MQs wrap all naked properties in an & {}; style rules obviously cannot do this, so the behavior would be inconsistent either way. As the spec is currently written, style rules and MQs are each internally consistent with a single behavior for all naked properties. I think it would be a (probably minor) bad thing for style rules to have two behaviors, depending on relative ordering of rules and declarations.

@kizu
Copy link
Member

kizu commented Jun 16, 2023

While I would like to see a world where the order would stay as written, I think I agree with Tab on that this is the behavior that was implemented in all preprocessors: less, sass and stylus, at least some CSS-in-JS solutions (tested in styled-components, couldn't find a good playground to share), and nested PostCSS plugin (can be tested here), lightningCSS (but it polyfills the current spec).

Given this was done literally everywhere, I'd say it is not worth it to change this.

@css-meeting-bot
Copy link
Member

css-meeting-bot commented Jul 19, 2023

The CSS Working Group just discussed [css-cascade] [css-nesting] Figure out whether we're fine with "shifting up" bare declarations after rules.

The full IRC log of that discussion <bramus> topic: https://github.com//issues/8738
<bramus> emilio: right now in nesting, when you have mixed declearations and nested rules, the current behavior of nesting (and sass?) is to pull all the declarations up which may feel a bit weird if you dont know this happens
<bramus> … the source order changes
<bramus> … dont know where we ended up in this discussion
<bramus> … but i think it is unfortunate
<bramus> … if ppl are fine with no change, so am i
<bramus> Rossen_: tab’s last comments were suggesting that
<fremy> Oh, thank you @emilio for filing this
<bramus> TabAtkins: yes
<bramus> Rossen_: remarks?
<bramus> fremy: thank you emilio for filing.
<bramus> … would love if we could fix this but understand that implementations are shipped
<bramus> fantasai: i think this is very confusing. On the plus side it is not common authors will do this, where they have the same specificity as the ??? and they are modifying the same property. so authors will not commonly run into this
<bramus> … when they do, the fact that it applies out of order is very confusing
<bramus> … we dont have to have this problem
<bramus> … in terms of options, we can say that when you are interleaving, you have to make additional ampersand rules to have them maintain their position
<fremy> q+
<bramus> … seems stragithforward but are concerned that we should fix the cascade bc it is confusing
<miriam> q-
<bramus> TabAtkins: main arg for no change is that this behavior is the same as what preprocessors do since they supported it
<bramus> … it has not been a problem for any of them as far as we can tell
<bramus> … for sass we have a maintainer confirming that
<bramus> … so we dont need to worry about it I think
<bramus> … it is not an issue in practice, so we should not come up with complicated solution
<bramus> … MQs and non-style rules have a different behavior when nested than style rules are
<bramus> … style rules put all their props in the style as ??? if it were up front
<myles> q?
<bramus> s/???/
<bramus> … and mqs in similar put all their ?? in nested style rules
<Rossen_> ack TabAtkins
<bramus> … we could run into the issue that knowing when nesting has begun depends on non; it is not ituitive what to define well when we have started nested
<bramus> … a rule that is unknown or mistyped can change that interpretation
<bramus> … in an unpredictable way
<bramus> …. to keep consistent behavior lets avoid that problem entirely, and data from preprocessors has shown it is not a problem
<bramus> fremy: the last argument seems valid
<Rossen_> ack fremy
<bramus> … the examples emilio gave is not about nesting selectors but nesting an at-media rule, which I can do
<bramus> … i feel example is convincing enough
<bramus> … is implementation of sass/less supporting this use case?
<bramus> … if they dont, then we are making things quite ???
<bramus> TabAtkins: yes, i believe they do
<bramus> … throw it into a sass playground and it will output the euiqvalent compliled code with media shifted out
<bramus> … the two decls will be grouped together
<Rossen_> q?
<bramus> s/grouped/combined
<bramus> fremy: in this case I dont feel strongly enough that we should
<bramus> emilio: so fixing interleaved decls inside a grouping rule by wrapping in &-rule is easy
<bramus> … fixing style rules by adding nested style rules is not too hard but … I guess we could fix it
<bramus> … dont feel too strongly either way
<TabAtkins> yup, today emilio's examples compiles to "the div with both properties, followed by the @media holding a div holding the conditional props"
<ntim_> q+
<bramus> Rossen_: so then the proposed resolution is to close no change
<ntim_> q-
<bramus> fremy: did anybody look into ???’s comment?
<bramus> Rossen_: we did,
<myles> q?
<fremy> s/???/Lea Verou/
<bramus> jensimmons: we do not like the proposed resolution. it does not make sense for authors right now. they expect for the later styles to apply
<bramus> … i agree that a non-complicated thing is a goal, bu tshould not be what we have now
<ntim_> +1
<bramus> … sass is a guiding principle, but we should follow how CSS has always worked: source order
<bramus> … ntim has a proposal
<bramus> ntim_: emilio wrote it in the issue : wrapping in &
<miriam> Here's the example in a Sass playground, for reference (sorry it's a long url): https://sass-lang.com/playground/#MTFkaXYlMjAlN0IlMEElMjAlMjBjb2xvciUzQSUyMGdyZWVuJTNCJTBBJTIwJTIwJTQwbWVkaWElMjAod2lkdGglMjAlM0UlMjAwKSUyMCU3QiUwQSUyMCUyMCUyMCUyMGNvbG9yJTNBJTIwcmVkJTNCJTBBJTIwJTIwJTIwJTIwYmFja2dyb3VuZCUzQSUyMHJlZCUzQiUwQSUyMCUyMCU3RCUwQSUyMCUyMGJhY2tncm91bmQlM0ElMjBncmVlbiUzQiUwQSU3RA==
<bramus> astearns: just for my clarification: that solution will allow later rule to override the earlier ones?
<bramus> [multiple]: yes
<bramus> plinss: i presume for wrapping ??? in another declaration block it becomes another rule in the OM?
<bramus> TabAtkins: yes
<bramus> plinss: is the & still required these days?
<bramus> TabAtkins: yes, you need a selector
<bramus> … in terms of raw css syntax parsing, omitting ths eelector will give you a rule with empty prelude and that will fail to parse as valid style rule
<oriol> q+
<bramus> TabAtkins: further issue as explained in thread. anything we do that distinguished behavior on before/after nested rule means we have to define the switch for 'we are past a nested rule'
<emilio> q+
<bramus> … cant be ??? to cause the syntax trigger
<bramus> … we dont need that concept if we dont do this
<bramus> … it will potentially change the cascade
<bramus> emilio: why would you have problem to just use valid nested rule as trigger?
<bramus> TabAtkins: bc a new rule that does not exist in older browser that understands nesting will throw away tgha tnew rule and conclude tha tnesting hasnt started yet
<bramus> emilio: and the declarations will ?? and that seems fine
<bramus> TabAtkins: and the OM changes
<bramus> emilio: ????
<bramus> emilio: like the OM changes, but it also changes when you itnroduce new rules.
<bramus> … you end up with different ??? list
<Rossen_> q?
<bramus> fantasai: if we have style rule with interleaved rules and declarations. lets say 3 decls at top, at-rule, and 2 decl at bottom
<SebastianZ> s/???/length/
<bramus> … old browser will have 5 decls appear in .style of that stylerule
<bramus> … if we wrap it in ??? then in a new browser you would get first 3 decls in ./style. then at-media in .cssRules followed by - followed in it - a new style rule with last 2 decls
<bramus> TabAtkins: not, but that is closely related.
<dbaron> ScribeNick: dbaron
<SebastianZ> s/???/an :is() rule/
<Rossen_> ack oriol
<TabAtkins> specifically, `::before { & {color: red;}}` does *not* apply red to ::before in today's spec
<dbaron> oriol: about wrapping decls in an &: with two elements, the & can't represent 2 elements. If you have a declaration, garbage, and a declaration -- if later a new feature parses the garbage as a nested rule, then the following declaration would be grabbed and not work.
<TabAtkins> and causing implicit wrapping would trigger that problem as well
<matthieudubet> (is oriol mic working ? the sounds seems far away)
<dbaron> oriol: while I agree the current behavior is not great, and would be a good oargument for chosoing option 4, I'm leaning more towards what tab is saying -- keeping current behavior
<emilio> ack emilio
<Rossen_> q?
<matthieudubet> (yes that was better at the end with the new mic)
<dbaron> TabAtkins: I forgot about that -- I agree that kills it harder. You can't nest if your parent rule has pseudo-elements. That's the behavior of :is(). If you tried to do this and parent selector applied styles to pseudo-elements, the implicit wrapping would just drop these declarations on the floor. That's broken at a fundamental model level right now.
<dbaron> plinss: that goes away if we get rid of the :is() behavior, right?
<dbaron> TabAtkins: yes
<ntim_> q+
<dbaron> TabAtkins: my proposed resolution is still to close with no change; I think current spec is right.
<dbaron> TabAtkins: but we should see if objection from Apple is still standing
<Rossen_> ack ntim_
<dbaron> ntim_: I wonderif we can translate to a ? rule without using &, just using the selector text.
<ntim_> div { color: green; } @media (width > 0) { div { color: red; background: red; } } div { background: green }
<dbaron> TabAtkins: no, b/c that's a relative selector
<dbaron> TabAtkins: those nested rules are "div div"; it's still relative to the parent rule
<dbaron> hober: siblings, not nested
<dbaron> myles: there's 3 sibling rules in that example
<dbaron> TabAtkins: that would mean treating nesting as a preprocessor directive that rewrites into a different rule structure
<dbaron> TabAtkins: we haven't been pursuing that approach since very early on in nesting. I'd like to reject trying to rewrite style things. That makes many things more complicated, like for how @media nesting works in rules. But the more we diverge the OM and the model underneath from the syntax authors write, the worse it is.
<dbaron> fantasai: I think I'm not comfortable resolving on no change -- it doesn't sound like we have consensus -- but we don't have a clear counterproposal worked out. I think TabAtkins brought up many useful concerns with emilio's path. We should take some time outside the meeting to review minutes and try to think through the psosibilities. Might be down to these 2, but maybe something else. And see if we can address relevant concerns, or at
<dbaron> ... least have a better understanding of what they are
<dbaron> [mic out of battery]
<jensimmons> +1 to what Elika just said
<dbaron> TabAtkins: If you think it's valuable to more exploration, you're welcome to. I don' t think it's going to work, but you're welcome to.
<dbaron> fantasai: Sounds like there's an aciton item for people with concerns about current proposal to come up with a counter proposal.
<dbaron> Rossen_: No rule that we can't reopen issues.
<dbaron> fantasai: I object to resolving when this many people don't like the direction.
<dbaron> TabAtkins: As long as it's not a drastic change (like turning it into a rewriting rule), unlikely there will be compat concerns if it changes relatively soon, given that this is a rare code pattern. But not indefinitiely, and limitations on how far that reaches.
<dbaron> Rossen_: So let's move on to the next issue. And once Apple or anyone else has a better proposal, bring it back.
<astearns> fwiw I have already seen authoring advice against mixing rules and declarations this way, so if people follow that advice it lessens the compat concerns

@LeaVerou
Copy link
Member

LeaVerou commented Oct 4, 2023

I thought it was very odd that the WebKit poll seems to be favoring Option 1 (shifting up), so I posted a couple more polls:

which so far seem to be showing a very different picture with more than 3 out of 4 expecting no shifting. There are even people incorrectly explaining what happens (1 2)

For the minority that expects the current behavior, it often seems to be due to misconceptions around how specificity works (1 2 3 4 5). Others mentioned that while not shifting feels more natural, shifting has been beaten into them by existing implementations (e.g. 1 2 3 ).

There is also this older poll but it's phrased as a quiz, which influences the data, as people expect the result to be weird. Even so, it shows an even split between shifting and no shifting.

I think the current behavior is extremely unintuitive, and 10 years down the line we will regret opting for consistency with today’s preprocessors over predictable, natural behavior. In fact, we even have a TAG principle advising against this exact thing: Prioritize usability over compatibility with third party tools.
I'd even vote for entirely disallowing declarations after rules over the current behavior, as it would give us more time to figure this out.

I’m quite concerned that wording seems to influence the result so much, as it indicates we don't yet have a good picture of author expectations. Perhaps we need more data here. Maybe an MDN short survey would help?

Agenda+ to resolve to temporarily disallow declarations after rules while we try to get better data here, so that we don't get stuck in a situation where we can't change the behavior due to web compat.

@LeaVerou LeaVerou closed this as completed Oct 4, 2023
@LeaVerou LeaVerou reopened this Oct 4, 2023
@mdubet
Copy link

mdubet commented Oct 4, 2023

FWIW we (WebKit) were also very surprised by our poll results (it actually changed over the weekend, the first days were Option 2 winning 70/30).

Disallowing declarations after rules seems the "worst" solution because it means we have to determine the "a rule have been parsed" trigger which could cause some compatibility issue if we extend what correctly parse as a rule in the future. However, I'm not sure what would be the risk if we allow declarations after rules like now and just follow the cascade so last one wins?

@romainmenke
Copy link
Member

Might be good to resurface/link the CSSOM aspects of this as I don't see any mention of those in this issue.
If I recall correctly it was an issue for CSSOM if declarations and rules could be interleaved.

(maybe there is a clever way to resolve those?)

@LeaVerou
Copy link
Member

LeaVerou commented Oct 4, 2023

Disallowing declarations after rules seems the "worst" solution because it means we have to determine the "a rule have been parsed" trigger which could cause some compatibility issue if we extend what correctly parse as a rule in the future.

Could you please elaborate on that? Not sure I follow what you mean by "a rule has been parsed trigger".
FWIW I don't think disallowing declarations after rules is a good long-term solution; I'm only proposing it so that we don't back ourselves into a corner while we deliberate and time passes.

However, I'm not sure what would be the risk if we allow declarations after rules like now and just follow the cascade so last one wins?

I think the argument against that is inconsistency with preprocessors (and perhaps the existing Nesting implementations? Though usage of these in the wild right now is effectively nil, and even smaller where this changes things). Not sure if there's any other counterargument.

Might be good to resurface/link the CSSOM aspects of this as I don't see any mention of those in this issue. If I recall correctly it was an issue for CSSOM if declarations and rules could be interleaved.

(maybe there is a clever way to resolve those?)

From what I remember, we resolved that by resolving that bare declarations can be wrapped with & { ... }, though I can't find that right now.

@tabatkins
Copy link
Member

Could you please elaborate on that? Not sure I follow what you mean by "a rule has been parsed trigger".

Yeah, I've elaborated on this in the past when Emilio suggested disallowing properties after rules.

So, this is a parsing switch. In theory parsing switches are doable; we explored a few of these earlier when working thru Nesting options. But the switch needs to be reliable - the earlier discussion about @nest; being the switch worked, because we know exactly what to look for and don't expect that to change in the future.

But if the parsing switch is "a rule of some kind is seen", that's hard. The most obvious interpretation of that is "a valid rule of some kind is seen" - that's well-defined, but it means that authors can see unexpected differences in parsing behavior if the rule in question is supported in some browsers but not others, as older browsers will throw out the rule and continue allowing declarations, while newer browsers will see it and disallow declarations. (And consider: an invalid selector makes the style rule invalid, and we do new selectors all the time.)

So ideally we define invalid rules as also triggering this. But then we open up the full syntax space of what an "invalid rule" actually is. Is foo bar:baz {...}; an invalid rule? Or is it a new property syntax? We can define something for this, but it means restricting our future evolution capabilities, to a larger extent than what we've already done for Nesting.

@b-strauss
Copy link

FWIW we (WebKit) were also very surprised by our poll results (it actually changed over the weekend, the first days were Option 2 winning 70/30).

Have the results been restricted? I remember seing opposite numbers yesterday.

@jensimmons
Copy link
Contributor

jensimmons commented Oct 5, 2023

I watched the results of the poll very carefully in the first couple days. It was consistently 37-39% for Option 1, and 61-63% for Option 2.

It's very telling when you hit enough votes (like 50 or 100) and the results stabilize. As more and more votes came in, the results did not change.

Sept 28 at 2:50pm ET.
Screenshot 2023-09-28 at 2 50 18 PM

Sept 29 at 12:50pm ET.
Screenshot 2023-09-29 at 12 50 02 PM

Then over the weekend, another 1800 votes came in, with an overwhelming preference for Option 1. Almost three times as many votes came in over the weekend? Long after we stopped promoting the article on social media? With a radically different result?

That's a bot.

So we decided to close the survey and post the last known results from before the traffic pattern became highly suspect.

You can see a similar preference for Option 2 in Lea's survey: https://mastodon.social/@leaverou@front-end.social/111177156433448874

@mdubet
Copy link

mdubet commented Apr 16, 2024

If we define the @nest as:

  • mostly/exclusively used to automatically wrap declarations
  • mostly invisible to user in the serialisation
  • not having a specific behaviour (just using the behaviour/specificity of the parent style rule)

Do we actually need to define it ? Wouldn't extending the API of a CSSGroupingRule to allow a list of intermixed CSSStyleDeclaration and CSSRule works ?

@emilio
Copy link
Collaborator Author

emilio commented Apr 16, 2024

Kind of? How would you expose that? What would CSSGroupingRule.cssRules do? What would happen if you remove the middle rule in @tabatkins's example above? Would the CSSStyleDeclarations get magically merged? Seems less straight-forward / more questions, at the very least...

@LeaVerou
Copy link
Member

LeaVerou commented Apr 16, 2024

I would prefer to serialize the @nest always, because authors can write things like:

.foo {
  color: blue;
  @nest {
    color: red;
  }
}

And I'd prefer not to make the choice of whether to serialize or not dependent on whether a non-@nest rule has been seen yet.

Sure they could, but we don’t design APIs based on the 1% of cases. Far better to have reasonable behavior in the 99% of cases and magic in the 1% than the other way around, even if the magic case in the former seems more magic.

Also @mdubet makes a good point: is there value in authors actually writing @nest? If not, what if they can't, and it basically becomes a CSSOM implementation detail?

@mdubet
Copy link

mdubet commented Apr 16, 2024

@emilio Either CSSGroupingRule.cssRules returns a list of CSSRule + CSSStyleDeclaration or we use a new CSSGroupingRule.elements or something else.

It's really the same idea, except that we use CSSStyleDeclaration instead of a potential @nest. But it has the advantage of not being explicitly writeable by a user (you can't write distinct consecutive CSSStyleDeclaration blocks in CSS - but you can if you really want with CSSOM).

(For the example above, merging the two CSSStyleDeclaration would makes more sense indeed)

@tabatkins
Copy link
Member

Sure they could, but we don’t design APIs based on the 1% of cases. Far better to have reasonable behavior in the 99% of cases and magic in the 1% than the other way around, even if the magic case in the former seems more magic.

Basically my opinion is that the more magic we have for this, the worse it is. There's a minimum amount of magic we need for usability's sake (automatically wrapping the declaration in this new rule when parsing), but beyond that I strongly prefer for everything to be as normal and unsurprising as possible. Every bit of magic is another sharp corner we have to be aware of and design around, forever, vs something predictable and boring.

Importantly, pay attention to the use-case you're talking about - when does an author serialize a CSS rule and manually examine the results? That seems... super rare to me. It's the sort of thing I, personally, have only ever done to debug things. Putting aside the question of whether it's better to serialize the implicit rule or not, does this case occur often enough that it's worth doing anything custom at all? In my opinion, definitely not.

[talk about using CSSStyleDeclaration]

Same point, really - is it worth doing anything more complicated than just "here's a new type of at-rule, it acts like you'd expect"?

@LeaVerou
Copy link
Member

I don't understand why you think it's acceptable to magically add @nest, but not magically remove it. They seem like two sides of the same coin to me, and if anything doing one without the other is inconsistent. Having @nest magically appear in my code when I try to roundtrip it seems way weirder than seeing a manually authored @nest disappear in the same case. And it's still unclear to me what is the value of allowing authors to write @nest at all. If there is none, then the case of an author writing @nest and seeing it disappear if they try to roundtrip will be exceedingly rare.

Importantly, pay attention to the use-case you're talking about - when does an author serialize a CSS rule and manually examine the results?

I believe certain types of dev tools do this to at least some degree.

There's a minimum amount of magic we need for usability's sake (automatically wrapping the declaration in this new rule when parsing), but beyond that I strongly prefer for everything to be as normal and unsurprising as possible.

Are you saying that the expected frequency with which something is encountered doesn't factor in the API design choices you make?

Every bit of magic is another sharp corner we have to be aware of and design around, forever, vs something predictable and boring.

Wait, I thought we were talking about author ergonomics, when did it become about our convenience?

@astearns
Copy link
Member

In the interests of resolving this 90-comment issue, would it be OK to open a new issue on the details of @nest, or do we need to figure it all out to make any progress?

@LeaVerou
Copy link
Member

In the interests of resolving this 90-comment issue, would it be OK to open a new issue on the details of @nest, or do we need to figure it all out to make any progress?

I think we can probably resolve on the general proposal, then discuss serialization in a separate issue.

@mdubet
Copy link

mdubet commented Apr 17, 2024

Same point, really - is it worth doing anything more complicated than just "here's a new type of at-rule, it acts like you'd expect"?

More complicated for browser engine indeed, but less complicated for CSS users (no need to learn about a @nest rule which will maybe create some confusion with the nesting selector & ..etc)

@emilio
Copy link
Collaborator Author

emilio commented Apr 17, 2024

More complicated for browser engine indeed, but less complicated for CSS users

Not sure I agree. This requires to expand the API surface of CSSOM quite a bit, or changing APIs in ways that are confusing / inconsistent (like not returning rules from .cssRules), or unclear...

If you're not inspecting stylesheets via script, it doesn't matter either way. If you do, then IMO the @nest rule is easier to deal with than something new / more complicated. It just works like a style rule.

Given that, and that this is somewhat of a time bomb which we probably want to fix sooner rather than later, I tend to think that the simplest / less magical solution is best.

  • It's implementable in a very straight-forward / performant way in all engines.
  • It raises less open / follow-up questions.
  • It's (IMO) not worse for authors than the current state of things or alternatives.

@tabatkins regarding the proposal, I'm assuming:

  • @nest doesn't need to be a grouping rule (though I guess it doesn't matter much either way).
  • With "style rules" in your proposal you mean style declarations, presumably, right?

@andruud
Copy link
Member

andruud commented Apr 17, 2024

Fully agree with @emilio.

I initially thought that we should serialize back to bare declarations for outer cssText since it's shorter, nicer, and basically "why not". But the round-trip issues and other edge cases convinced me the other way.

@LeaVerou You said that we should consider another name for @nest, would that mitigate the situation? If the name better explains what it does/is, maybe its presence will feel less offensive? @this? @self? @wart?

changing APIs in ways that are confusing / inconsistent (like not returning rules from .cssRules)

Yes, let's avoid this, please. Getting creative about the OM structure is even more intrusive than getting creative about its serialization.

@css-meeting-bot
Copy link
Member

css-meeting-bot commented Apr 17, 2024

The CSS Working Group just discussed [css-cascade] [css-nesting] Figure out whether we're fine with "shifting up" bare declarations after rules, and agreed to the following:

  • RESOLVED: Tab specs this by representing @nestin the CSSOM, with an inline issue about@nest as a syntactic construct that authors could write being something we might want to get rid of
The full IRC log of that discussion <emilio> TabAtkins: So, background... current spec for nesting allows you to interleave declarations with rules
<emilio> ... the way it's handled in the syntax spec is that all the declarations get collected together
<emilio> ... losing the order information relative to the rules
<emilio> ... this appears to cause author confusion
<emilio> ... particularly if you repeat the same declaration a few times between things
<emilio> ... it also means that whenever we introduce mixins, the ability to make it work with nesting doesn't work well
<emilio> ... so thought was, can we instead interpret declarations in between rules as if they were wrapped in another rule
<emilio> ... it interacts nicely with cssRules etc
<emilio> ... andruud added use counters and it seems it can probably be made
<lea> q?
<lea> q+
<emilio> ... I proposed the change in the comment from the agenda
<kbabbitt> q+
<lea> q-
<lea> q+
<emilio> ... basically collect declarations into a new type of atrule (tentative name `@nest`)
<emilio> ... `@nest` doesn't do anything other than grouping the declarations
<emilio> ... This is slightly different to wrapping the parent selector
<emilio> ... So it improves our behavior when nesting declarations inside media queries
<emilio> q+
<emilio> ... everyone seemed happy with it, only q was about serialization which we'd move to a different issue
<emilio> ... authors can write it but it's not particularly useful for them to do so
<emilio> astearns: before going to the queue, wanna say that we only need to resolve on the approach
<astearns> ack kbabbitt
<emilio> kbabbitt: a little uncertain about how this interact with the OM
<kbabbitt> .rule1 { font-family: serif; color: blue; }
<emilio> ... was playing with an example
<kbabbitt> .rule2 { font-family: serif; @media (...) { ... } color: blue; }
<emilio> ... if I do setProperty("color", "red")
<emilio> ... does it know how to dig inside @nest?
<emilio> ... or ends up not having an effect
<emilio> TabAtkins: the later
<emilio> kbabbitt: that's a bit concerning
<emilio> ... in this world setProperty I added an @media and suddenly that doesn't work anymore
<emilio> TabAtkins: I agree you can run into potentially confusing situations with this
<emilio> ... but it's not incompatible with current usage
<matthieud> q+
<astearns> ack lea
<TabAtkins> scribe+ TabAtkins
<TabAtkins> emilio: It is unfortunate that that woudl stop working, but it's also a tradeoff that seems hard to avoid
<TabAtkins> emilio: In the ame example, if you set color in both locations, and you wanted to *keep* both color declarations (which is what needs to happen here), it becomes..
<TabAtkins> emilio: Like, maybe you can discard the previous somehow. But if you want to keep the altter decl you need a place to store it.
<TabAtkins> emilio: So it seems hard to avoid unless we do even more invasive changes to the OM, like letting you represent both declarations and rules directly
<TabAtkins> emilio: Not convinced that's worth it
<TabAtkins> emilio: I also think we generally discourage mutations in CSSOM when they affect stylesheets.
<TabAtkins> emilio: Doing so has pretty bad perf implications, it dedups shared/cached stylesheets.
<TabAtkins> emilio: So not sure it's worth rejiggering to "fix" this case
<TabAtkins> emilio: If you wrapped the decl in a *different* rule, it would also not be changed by setProperty()
<TabAtkins> kevin: to be clear, in my example the decl isn't in the @media
<TabAtkins> emilio: Right, just saying that it's in an @nest, and if it were any other rule it woudlnt' be surprising
<TabAtkins> kevin: just concerned that adding an @media in the middle of a rule causes the beahvior to change
<TabAtkins> matthieud: In the issue we discussed using cssstyledeclaration, and that woudl avoid the issue
<TabAtkins> emilio: That wouldn't fix this, you'd need the exact same logic to find the "correct" cssstyledeclaration to mutate
<TabAtkins> lea: what if we have the proeprty there but it's not enumerable
<TabAtkins> lea: Like a property that gives you all the decls, and a non-enumerable getter that gives you non-interleaved properties
<TabAtkins> lea: So normally you'd get the first set, but if you want to read a specific proeprty you'd just all of them from @nests
<TabAtkins> emilio: What if you have multiple?
<TabAtkins> emilio: I agree that this setProperty() breakage is somewhat surprising/unfortunate. I don't see a great way of avoiding it.
<TabAtkins> lea: I think we could design it to minimize the surprise
<TabAtkins> q+
<TabAtkins> lea: but we can't do this during the call
<dbaron> Is the behavior either with or without this change bad enough that CSS style guides will generally say not to use the interleaving feature (i.e., say to put all the declarations first)?
<TabAtkins> lea: reading properties should work, and there's an unambiguous value for each property, reading them shoudl give you correct
<TabAtkins> That's gonna be what I'm gonna say when I get to my q entry
<TabAtkins> emilio: I don't think it's worth complicating the OM substantially. The implicit rule is already magic, I don't want more.
<TabAtkins> emilio: This is pretty fundamental to what we're dsicussing. If we want the latter rules to act liek rules...
<lea> +1 for the proposed solution in general. Even though I do feel pretty strongly about the serialization issue, I think it's far better than the current situation _regardless_ of how serialization ends up working, so I think we should try to implement it asap before the current behavior starts being depended on. Far easier to change the serialization than how interleaved rules and declarations behave.
<dbaron> s/+1 for/lea: +1 for/
<lea> To summarize the serialization issue, these rules are added magically to author code, so I think when serialized they should also be removed magically, so that author code can roundtrip better. This would mean somewhat surprising behavior in the extremely rare cases where authors actually wrote an @nest rule, but far less surprising in the huge majority of cases where authors did not actually write it.
<lea> But perhaps more fundamental to the design of this feature, if authors have no reason to write it, should they even be able to? If not, that entirely solves the serialization issue, since we never need to handle it and it becomes a CSS OM detail rather than an actual CSS syntax feature.
<lea> We may also want to consider adding a new CSS OM property to get *all* declarations, including the interleaved ones.
<emilio> s/But/lea: But
<emilio> q+
<lea> dbaron: I don't think so, especially if we design the CSS OM so that it works in the largest possible set of cases
<dbaron> s/dbaron:/dbaron,/
<emilio> emilio: You'd still need to come up with something to return from CSSNestRule.cssText or what not
<astearns> ack emilio
<emilio> ... I think I'd rather avoid trying to hide the fact that we implicitly add this rule
<emilio> ... otherwise this kind of complexity proliferates and causes tons of edge cases everywhere that are bad for both authors end browser developers
<astearns> ack matthieud
<emilio> matthieud: In general I don't think we need @nest
<emilio> ... because we don't want authors to actually use it
<emilio> ... it's just an implementation detail
<emilio> ... all these serialization questions would go away if we didn't put it on the spec
<emilio> ... one solution would be to be able to use multiple CSSStyleDeclarations
<emilio> ... I think modifying CSSOM is a better solution
<emilio> q+
<lea> q+
<emilio> ... adding a new at-rule that we don't want authors to be using is a good solution
<emilio> ... I think it's pretty fundamental to this approach
<emilio> ... I think the question is do we add a new atrule or not
<emilio> ... the setProperty issue is less of an issue
<dholbert> emilio: [missed scribing]
<dholbert> scribe+ dholbert
<dholbert> emilio: I think the at-rule is the most obvious path forward to fix this
<dholbert> ... I think it's weird but it's the most obvious weirdness that we don't have to propagate to everywhere else
<dholbert> ... I don't see how a different approach fixes the annoyances it'd introduce, without massive redesign work
<lea> emilio you said an @rule can be implemented quickly. Would a CSS OM rule type that doesn't correspond to an actual syntactic rule take longer to implement?
<emilio> s/[missed scribing]/Multiple CSSStyleDeclarations don't fix any of the weirdness that @nest introduces
<lea> q?
<emilio> ack emilio
<astearns> ack TabAtkins
<emilio> lea: somewhat? I don't think we have a precedent for anything like that
<emilio> TabAtkins: what's being brought up is about what's more convenient or predictable when reading and modifying the OM
<emilio> ... when is that done, compared to writing CSS
<emilio> ... tool writers do
<lea> qq+ to reply to Tab
<emilio> ... but for anything that's not doing that specific thing, the most I've seen anybody do is copying stylesheets around
<matthieud> q+
<emilio> ... so actually modifying the OM is really rare
<emilio> ... so any complexity we introduce there needs to be weighted against the relatively small audience
<emilio> ... also, when I do this kind of thing I value more consistency / predictability
<dbaron> (does writing of polyfills tend to touch the OM, or just reparse everything from scratch?)
<emilio> ... vs. convenient APIs
<lea> dbaron, depends on the polyfill and how closely it tries to follow the actual syntax
<emilio> ... I think the @nest proposal keeps consistency for CSSOM
<emilio> ... anyways, let's resolve on keeping styles where they are
<emilio> ... I need to write _something_
<emilio> ... can't go "do this in an unspecified way"
<emilio> ... unless anybody has a strong objection right now my plan is to put the proposal on the spec, open to change
<emilio> ack lea
<Zakim> lea, you wanted to react to TabAtkins to reply to Tab
<astearns> ack lea
<lea> Lea: Tab, it's true that tool writers are a tiny minority of authors, but breaking tools has huge impact on authors, since a lot more authors _use_ tools than _write_ tools.
<lea> Lea: To reply to Emilio’s earlier point, I think there is an underlying design principles issue. I don't think avoiding magic at all costs should be a goal; the right magic can make a huge usability difference, at the cost of implementation complexity. Magic becomes a problem when it's unpredictable for users, and/or when users cannot opt-out of it. Neither seems to be the case here (at least if we design this right).
<lea> Lea: There one thing that is crucial to understand, emilio you said an @rule can be implemented quickly. Would a CSS OM rule type that doesn't correspond to an actual syntactic rule take longer to implement? Cause I agree we should contain the weirdness, not making this part of CSS syntax is a way to do that. Cause if authors _can_ write it, they will.
<dholbert> emilio: replying directly to that... then, you need to figure out all the APIs that are on CSSRule that... people expect to be able to call Stylesheet.insertRule, rule.cssText
<dholbert> emilio: if that rule can't be written by authors, what does that do?
<dholbert> lea: what is the reason to do insertRule in that case?
<dholbert> emilio: if you're writing a tool that wants to move rules around. that's the consistency argument that TabAtkins was talking about
<TabAtkins> Yup, having *one* object that acts differently from everything else inside of .cssRules is generally bad (and needs a fairly significatn justification to break)
<dholbert> emilio: if I want to filter the set of rules on a stylesheet and put them on another stylesheet, it seems reasonable that you'd be able to call insertRule with the cssText of every stylesheet you care about, but that doesn't work
<dholbert> lea: why does that need @nest { ...declarations...}?
<dholbert> emilio: that's not a rule
<dholbert> TabAtkins: that today would fail to parse. Would be a potential compat impact if we were to make that suddenly valid
<dholbert> matthieud: why would we use insertRule to insert declarations anyway?
<dholbert> emilio: you'd use it to insert a nested rule
<astearns> ack matthieud
<dholbert> lea: we do want to be able to use CSSOM to insert interleaved rules, so that does seem like a problem
<emilio> matthieud: I agree CSSOM is a very small audience
<emilio> ... we should not introduce new complexity in the CSS spec
<emilio> ... for that
<emilio> ... only modifying CSSOM is less invasive
<astearns> ack fantasai
<emilio> ... if you create new @-rules, then we need to explain it to all authors
<emilio> fantasai: wondering if it'd make sense to use the nesting at-rule concept in CSSOM but only in the OM, and stripping it out for serialization, and also have the first rule be represented in the same manner?
<lea> To summarize my position: 1. Ideally, @nest should only exist in the CSS OM, and insertRule() should accept raw decls 2. If that's not possible, then serialization should strip @nest 3. But even if @nest has to be serialized, that's still better than hoisting.
<emilio> ... so it'd be in .style and .cssRules[0].style
<emilio> TabAtkins: duplicating them would be inconsistent
<emilio> TabAtkins: @page does have declarations and rules
<emilio> matthieud: font-feature-values could take various at-rules and then a nested declaration
<emilio> q+
<astearns> ack emilio
<dholbert> emilio: the other option, a bit less weird perhaps, is:
<dholbert> emilio: so right now, we're using this trick of a style rule with the & selector
<dholbert> emilio: the idea behind @nest or whatever the name ends up being, would effectively be wrapping it in a style rule with a magic selector that represents the parent
<lea> q+
<dholbert> emilio: @nest is conceptually a style rule with a magic selector that doesn't get wrapped
<dholbert> emilio: we could define such magic selector somehow
<dholbert> emilio: would that make people less worried about the OM shenanigans?
<dholbert> TabAtkins: that wouldn't fix the problems that were brought up
<kbabbitt> q=
<kbabbitt> q+
<dholbert> emilio: it would behave like @nest, and like bare declarations in @media. So less breaking change than what we've been discussing
<dholbert> TabAtkins: every issue discussed has been from being in a rule of *any* kind
<dholbert> emilio: it doesn't happen if it's a style rule... I guess we need to explain the new selector
<dholbert> TabAtkins: that sounds even more magical
<dholbert> emilio: just wondering if it would be more amenable. But I think @nest is the best solution, given the constraints. Just curious if people would prefer putting the magic in the selector list rather than the at-rule. But I don't mind either way
<astearns> ack lea
<lea> Lea: Do note that this does not need to be set in stone, we can implement it as a hidden CSS OM rule type to minimize API surface and fix the immediate problem, and figure out how things like insertRule() would work later — and if we can't, we just promote it to a regular rule. But it's much harder to go the other way
<astearns> +1 to start with the least user-facing change
<emilio> kbabbitt: I do agree that we need some kind of a solution, and I think @nest is better than the status quo
<emilio> kbabbitt: IIUC what emilio was proposing was making @nest explicitly with some sort of selector, it feels less magical
<emilio> ... it doesn't solve the setProperty issue but it makes it so that if you have to confront that it becomes obvious
<fantasai> +1 to lea
<lea> +1 to astearns
<ydaniv> +1 to both
<emilio> astearns: does it sound reasonable to make @nest {} as a magic thing that authors can't write?
<emilio> lea: wasn't it what matthieud proposed?
<emilio> matthieud: I was proposing using CSSStyleDeclaration
<lea> s/astearns: does it sound reasonable to make @nest {} as a magic thing that authors can't write?/astearns: does it sound reasonable to make @nest {} as a magic thing that authors can't write for now?/
<emilio> ... but this sounds a bit in the middle
<emilio> TabAtkins: if we make @nest parse invalid we can't round-trip
<emilio> ... anything we do here has significant complexity that makes corners of stuff behave weirdly
<emilio> astearns: your point was that this wasn't affecting authors much
<lea> Yes, the idea was there would be no @nest in serialization for now
<emilio> TabAtkins: it does because we need to define @nest to not serialize
<emilio> lea: the idea is to make @nest not serialize
<dholbert> emilio: I agree with TabAtkins for what it's worth
<emilio> ... if we settle on @nest then we switch to serialize it
<lea> straw poll?
<oriol> +1 to Tab and emilio
<dholbert> emilio: doing this in two steps that can break code seems unnecessary
<emilio> TabAtkins: really strongly against extra magic other than in parsing
<emilio> ... unless there's a great argument for that, but I haven't heard one
<emilio> ... the more changes we make here the more potential there is for compat risk
<kbabbitt> ack kbabbitt
<emilio> TabAtkins: I'm close to objecting to things that put lots of magic in the CSSOM
<emilio> ... the less magic is CSSStyleDeclaration
<emilio> lea: I think it's great, we have that object already
<dholbert> emilio: I disagree. how can you make CSS Rules return something that isn't a CSS Rule?
<dholbert> lea: cssstyledeclaration is the least surprising option. it already exists and fits this perfectly
<dholbert> emilio: I disagree. Every time we introduce an @ rule, it causes things that [...]
<dholbert> lea: can we resolve that TabAtkins can scribe whatever makes him happy, and we see if we can resolve on it?
<emilio> lea: can we resolve on TabAtkins writing whatever he wants and prioritizing next steps on follow-up calls?
<dholbert> TabAtkins: sounds good to me
<lea> s/and we see if we can resolve on it?/and prioritize resolving on the additional issues ASAP, next week if possible?/
<emilio> fantasai: we'd like not to introduce @nest as an author-facing constructs
<lea> might be useful to do the straw poll anyway to get a sense of what people think, even if it doesn't guide the resolution
<emilio> ... if this is a temporary step towards that then fine but...
<emilio> astearns: Proposed: Let tab spec whatever, but note that user-facing @nest is something we want to get rid of
<emilio> objections?
<emilio> RESOLVED: Tab specs @nest, with a note about user-facing stuff being something we might want to get rid of
<lea> TabAtkins please add mention the unresolved issues in the draft?
<lea> s/add mention/mention/
<fantasai> s/user-facing stuff/@nest rules as a usable syntactic construct/

@tabatkins
Copy link
Member

To reiterate my points a bit from the call:

  • Fixing this problem in any reasonable way will affect (positively) approximately all CSS authors (by making properties in MQs/etc work better), and unblock CSS from evolving in certain ways that we want it to (adding mixins).
  • The issues brought up in the latter half of this thread are exclusively about the CSSOM, specifically reading and modifying it. I, personally, have never once read or modified the CSSOM in my entire webdev career; I've only ever done so to write WPTs.
  • So, to a first approximation, these issues affect nobody. ^_^ More precisely, the only audience I know of that actually reads/modifies the CSSOM in non-trivial ways (more than just, say, serializing an entire stylesheet and putting it somewhere else) are CSS tool authors. I'd estimate there to be a few hundred people in this category, across the entire world; a thousand on the extreme end of the possible range. This audience is also going to be significantly more experienced with coding in general.
  • None of the issues listed here are about a use-case being difficult or impossible, which is usually what we can use to justify solving a use-case for such a small audience. They are convenience features.
  • And the convenience itself is arguable in most cases. They're mostly about making the OM more consistent in some ways, but every single one comes at the cost of making it less consistent in other ways.

So in summary, past the necessary complexity/magic that's at the core of the issue (grouping interleaved declarations in some way so they don't merge with the initial set of decls), any further complexity we add is, as far as I can tell, solely to add minor convenience to some use-cases for a miniscule audience of already-experienced developers. Thus, none of them justify the spec and impl complexity cost they would impose.

I'm more than happy to be shown wrong on this regard - if you have an example of some change to my base proposal that would either give the "CSS tool authors" audience a large benefit (especially if it unblocks something that is currently hard or impossible), or that would give the "CSS general audience" even a mild benefit, or that would give some third audience I'm not aware of a benefit that is appropriately weighted against its size, then I'd joyfully consider it.

@astearns
Copy link
Member

But let’s not make this a permathread to go over all the alternatives and options. This issue was specifically about changing the “shifting up” behavior, which we have resolved to undo (to great author benefit). Please open new issues on specific problems or alternative solutions so that we can have more focused discussion about them.

@mdubet could you open an issue on the idea of using multiple CSSStyleDeclarations instead of a new @nest rule?

@LeaVerou would it make sense to open a new issue to explore how far we could go in making things browser-internal and not exposed to developers?

@kbabbitt if you think it is possible to solve the setProperty() problem while keeping the non-shifting behavior, please open a new issue

(and anything else I have missed, please open a new issue)

@yisibl
Copy link
Contributor

yisibl commented Jul 15, 2024

@LeaVerou It looks like @nest has been removed from the specification again right? This history I think needs an article detailing it.

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

No branches or pull requests