search

w3c / csswg-drafts

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

[css-nesting-1] Syntax Invites Errors #7834

Closed fantasai closed 1 year ago

fantasai commented 1 year ago

Edit: 👉🏼 UPDATED SUMMARY TABLE OF SYNTAX PROPOSALS 👈🏼 (from https://github.com/w3c/csswg-drafts/issues/7834#issuecomment-1282776786 )

As mentioned in #7796, there are some problems with the currently-proposed nesting syntax:

devongovett commented 1 year ago

Great examples. I still think this syntax is very confusing as soon as you get past the most basic of cases.

romainmenke commented 1 year ago

I still think this syntax is very confusing as soon as you get past the most basic of cases.

I agree.

I think we should also be careful with statements about rarity of patterns because the entire future of CSS and all future authors have to be taken into consideration. Something rare today might become common tomorrow and even if it remains rare, small numbers * infinity is still infinity...

jonathantneal commented 1 year ago

If this helps, @sesse:

Could we formulate clearly when & is inserted and when it's not...

As far as I understand it, each selector in a nested style rule would be evaluated like this:

This also matches the behavior in Sass.

With Nesting:

.yay {
  .lol {}
}

With Implications, Without Nesting:

.yay {}
.yay .lol {}

With Nesting:

a {
  :hover {}
}

With Implications, Without Nesting:

a {}
a :hover {}

As far as I understand it, valid nested at-rules would continue matching the same element.

With Nesting:

section {
  @container style(--yay: lol) {}
}

With Implications, Without Nesting:

section {}
@container style(--yay: lol) {
  section {}
}

... in a way that doesn't require lookahead to specify what is a valid rule?

I’m not sure if lookahead is relevant here, because qualified rules are not determined by semantic validity. The browser can know something is a rule, but it won’t evaluate the selector until the curly brace has started reading the block.

/* this stylesheet will give the document a green background, but it will not give it any border */

%@#!WHOA-NELLY!#@%, :root { border: 100px solid; }
/* ☝️ that is a qualified rule, from the moment "%" is read */
/* ☝️ when it reaches the opening curly brace, the selector is evaluated */
/* ☝️ it will not apply the rule if it does not semantically understand "%@#!WHOA-NELLY!#@%" */
/* ☝️ this is because regular selectors “not forgiving” */

:is(%@#!WHOA-NELLY!#@%, :root) { background: green; }
/* ☝️ that is also a qualified rule, from the moment ":" is read */
/* ☝️ it will apply, even if the it does not semantically understand "%@#!WHOA-NELLY!#@%" */
/* ☝️ this is because, within :is(), selectors are “forgiving” */
LeaVerou commented 1 year ago

I'm trying to get a handle on proposal 3, more specifically this idea of implicit & insertion (which I really think is a bad idea, but people seem to feel very strongly about it). Could we formulate clearly when & is inserted and when it's not, in a way that doesn't require lookahead to specify what is a valid rule? For instance:

Not an editor of Nesting, but since option 3 was my proposal, I'll make an attempt to answer some of these.

  • .foo { .bar { … }} — seemingly this should be interpreted as & .bar

Yes, it would be & .bar, with &.bar requiring the &, just like Sass.

  • .foo { .bar & { … }} — this should not be interpreted as & .bar &, because there's already a & in there (it's nest-containing)?

Correct.

  • .foo { > .bar & { … }} — however, this should be interpreted as & > .bar &, because it starts with a combinator? Or should this be a parse error? (Why?)

That would be & > .bar &, yes.

  • .foo { :hover { … }} — This should be & :hover and not &:hover? But I've seen examples where people seem to assume it's the latter.

Yes. Again, it's descendant by default, just like preprocessors. For &:hover the & is mandatory.

  • .foo { :is(&) { … }} — Should this be interpreted as & :is(&) or just :is(&)? I would assume the latter, since it's nest-containing, but you may have to dig pretty deep to find the &.

Yes, it's :is(&), not & :is(&). Digging deep to find the & is fine, what we're trying to avoid is having to dig deep to find whether it's a rule or a declaration.

  • .foo { :is(.bar, !#&/) { … }} — Is this nest-containing (which then decides whether there should be a & in front? There's an & in there, but it's dropped in forgiving selector parsing.

I'd file a new issue for this.

And finally:

  • .foo { .bar { … }} — how do we serialize this?
  • .foo { & .bar { … }} — how do we serialize this?
  • .foo { > .bar { … }} — how do we serialize this?
  • .foo { & > .bar { … }} — how do we serialize this?

I suppose it's fine to serialize with the implicit &.

  • .foo { .bar &, .baz { … } } — I assume this should be interpreted as .bar &, & .baz?

Yes.

  • .foo { :is(.bar &, .baz) { … } } — but this should be interpreted as :is(.bar &, .baz), so not consistent with the previous one?

I'd file a separate issue for that one too.

  • .foo { > .bar, .baz { … } } — and this should be & > .bar, & .baz?

Yes.

  • .foo { > .bar, + .baz { … } } — and this should be & > .bar, & + .baz?

Yes.

Alohci commented 1 year ago

I was wondering how .foo div { .bar &, .baz, > .fred { .qux & { … } } might expand. I think it might go something like this:

.foo div { .bar &, .baz, > .fred { .qux & { … } }

    for each complex selector (cs) in a nested selector list         if (cs) starts with a combinator (except space) imply '& ' precedes it         otherwise if (cs) contains a '&' do nothing         otherwise imply '& ' precedes (cs) .foo div { .bar &, & .baz, & > .fred { .qux & { … } }

    Substitute from inside to out, wrapping each substitution in :is() .foo div { .qux :is(.bar &, & .baz, & > .fred) { … } .qux :is(.bar :is(.foo div), :is(.foo div) .baz, :is(.foo div) > .fred) { … }

I think this is consistent with LeaVerou's answer above

dbaron commented 1 year ago
  • .foo { .bar { … }} — how do we serialize this?
  • .foo { & .bar { … }} — how do we serialize this?
  • .foo { > .bar { … }} — how do we serialize this?
  • .foo { & > .bar { … }} — how do we serialize this?

I suppose it's fine to serialize with the implicit &.

I think it would be good to file a separate issue for the serialization question as well.

tabatkins commented 1 year ago

The spec is now updated to Option 3, after a quick hack session with @argyleink; we haven't touched the CSSOM section yet, but the syntax stuff is all done (both in Nesting and in Syntax).

Re: serialization of selectors, we shouldn't do any magical insertion or dropping of stuff. The spec defines how to interpret selectors, but there's no need to "normalize" them to any particular form.

sesse commented 1 year ago

The spec is now updated to Option 3, after a quick hack session with @argyleink; we haven't touched the CSSOM section yet, but the syntax stuff is all done (both in Nesting and in Syntax).

Blink changes are also in review, although they've been done independently, so I'll need to check whether I diverged from the spec at some point. There seems to be still some issues to file to get the interpretation of e.g. :is() 100% nailed down.

Re: serialization of selectors, we shouldn't do any magical insertion or dropping of stuff. The spec defines how to interpret selectors, but there's no need to "normalize" them to any particular form.

This feels odd. We're already normalizing selectors; in particular, for whitespace. Also, of course we (implementors) need to do magical insertion of stuff; that's the only way we can understand what these nested selectors mean and match against them. If & > .a and > .a were to serialize differently, we would probably still store the &, but have some sort of “this & was added implicitly” bit solely for the point of serialization. Which feels a bit pointless, but isn't difficult per se (we already do it for relative selectors).

I guess maybe we're coming from different sides here; I see the spec talks about everything being relative selectors now, except that sometimes they're not (and you don't know until after you're done parsing them). I don't intuitively interpret “.foo” standalone as a relative selector, and I think losing the “interpret & as :is(…parent list…)” is a bit sad (the new text just says “any relative selectors are relative to the elements represented by the nesting selector”, which sort of makes the :is() equivalence a secret and its specificity less than obvious?). But if you do define most of those selectors as relative, I understand that you wouldn't want to convert them on serialization.

tabatkins commented 1 year ago

We're already normalizing selectors; in particular, for whitespace. Also, of course we (implementors) need to do magical insertion of stuff;

We don't "normalize" serialization for other relative selectors, like in :has(); this should be the same.

I guess maybe we're coming from different sides here;

If you're familiar with Sass or many other "nested CSS" preprocessors, the syntax as the spec now describes is pretty much exactly what they do. (Just with the "no starting with an ident" restriction.) If you're not familiar with those tools, yeah, it might not be the most obvious at first, but it seems to be a very popular and understandable syntax since it's been around for so many years and copied in so many tools.

I think losing the “interpret & as :is(…parent list…)” is a bit sad (the new text just says “any relative selectors are relative to the elements represented by the nesting selector”, which sort of makes the :is() equivalence a secret and its specificity less than obvious?).

The section that defines the nesting selector still talks about the :is() equivalence, rather explicitly.

romainmenke commented 1 year ago

The spec is now updated to Option 3, after a quick hack session with argyleink; we haven't touched the CSSOM section yet, but the syntax stuff is all done (both in Nesting and in Syntax).

PostCSS plugin changes are mostly done : https://www.npmjs.com/package/@csstools/postcss-nesting-experimental This should make it possible for authors to experiment more easily with the updated nesting proposal and with relative selector syntax.

sesse commented 1 year ago

Blink changes are also all but done, but we figured out that the syntax changes break several tests (including ACID2), so it can't go in yet.

sesse commented 1 year ago

Chromium as of 109.0.5394.0 is updated with the new syntax.

mrleblanc101 commented 1 year ago

Why not adopt the SCSS syntax entirely ?

I know it could cause parsing issue of existing CSS, but we already had a similar problem with HTML5. So we simply created a new Doctype to handle that.

Why not just add a declaration at the top of the file to tell the browser to parse this file differently ? We already have something like this with @charset "UTF-8"

Something like this:

@nesting 

.my-class {
  nested-tag {}
  .nested-class {}
}
sesse commented 1 year ago

Why not adopt the SCSS syntax entirely ?

As has been discussed in the thread already, and also noted in the draft standard, the case of “nested-tag {}” would be ambiguous to parse without increasing the parser's lookahead (either explicitly or by implicit means, such as restarts).

I know it could cause parsing issue of existing CSS, but we already had a similar problem with HTML5. So we simply created a new Doctype to handle that.

The word “simply” is not one that comes to mind if you're suggesting maintaining two entirely separate CSS parsers indefinitely (both in the spec and in code).

LeaVerou commented 1 year ago

Why not adopt the SCSS syntax entirely ?

Blink implementers refused to consider it. @emilio said it could have been feasible for Gecko. See #7961 where I spent a fair bit of my time trying to come up with a solution that involves only a minimal performance hit (the backtracking would be limited to only element:pseudo nested selectors, which are fairly rare in nested stylesheets). This was the response from @sesse :

I don't share this goal; I would like us to just get the syntax (any syntax) specified, out the door and be done with it. So no, I won't be spending resources trying to bend our minds around how we tokenize CSS to this avail; I can NAK what is a non-starter for us, and that's what you'll get.

He also mentioned having to give up some performance optimizations to go that route, without responding to my question asking what they are so it could be more widely examined if they'd really need to go.

But hey, at least the syntax we (thus far) went with allows for the syntax to be relaxed in the future (perhaps once it becomes obvious to everyone how much that would increase developer ergonomics).

devongovett commented 1 year ago

There are a lot of assumptions and personal opinions being thrown around here. Your own poll showed that developers prefer to write the & in every selector: https://twitter.com/leaverou/status/1579902585540345857. Yet this was pushed through anyway. It does not seem like feedback is being taken seriously, both from implementors and from authors.

LeaVerou commented 1 year ago

There are a lot of assumptions and personal opinions being thrown around here. Your own poll showed that developers prefer to write the & in every selector: twitter.com/leaverou/status/1579902585540345857. Yet this was pushed through anyway. It does not seem like feedback is being taken seriously, both from implementors and from authors.

This has already been addressed, I think by @fantasai: The poll showed that about half of developers (52%) want to include & in every selector, and half (48%) do not. Making it optional caters to both; the first group can still include & in every selector. Making it mandatory only caters to one of the two groups.

Also, if you read the comments, there was some confusion about what an optional & would be, many assumed it would be optional in every case, including things like &.foo, &:hover, .foo & etc and wanted to change their vote when they realized that was not the case. This was clarified in the subsequent poll.

I understand that you feel strongly about this, but right now none of the proposals considered by the group involves a mandatory &, so this is not a productive debate, and I'm not going to engage in it further. Perhaps we can agree that if & is optional, it's better if it's optional in every case where selectors begin with & and a combinator? (I do not think it's so much better that it's warrants moving to a postfix syntax, but if we could get it to be optional in every case and keep a nested syntax, that would be ideal).

romainmenke commented 1 year ago

For starters, I personally like the syntax of proposal 3 on principle. I would like it even more if we could omit & everywhere. It is just more consistent.

I like it because it gives authors more options and from an author perspective this is always a good thing :

I do not have strong opinions about nesting itself. For me nesting is a minor feature and should not dominate the language.

This for context because it has been pointed out that I apparently have strong feelings about all this and that I oppose the syntax change. I do not. I've spend the past weekend making sure there is a PostCSS plugin for it. I am fully on board with the resolution to switch to proposal 3.

I will miss @nest blocks but not that much. I thought they were an elegant "catch all" solution for writing nested CSS. Something you truly do not have to think about, it just works.

I do have strong opinions about a parser switch. This was only part of proposal 2.

I also have strong opinions about ignoring side-effects and downsides and I prefer having this made clear and explicit.

This as a prelude to say that I don't have an issue with the substance of this thread. I do however have an issue with how this all played out.

I don't think it is ok to have a one sided discussion were one groups arguments are always immediately discarded.

I definitely don't think it is ok to call out individuals and mark them as solely responsible for why we can't have a certain syntax.

I think we can and must do better. These issues are not important enough for all this drama, none are.

devongovett commented 1 year ago

This was clarified in the subsequent poll.

It should be noted that this poll has significantly fewer votes. The difference between them is also very subtle.

I understand that you feel strongly about this, but right now none of the proposals considered by the group involves a mandatory &

The previous syntax (option 1) was exactly this.

LeaVerou commented 1 year ago

The previous syntax (option 1) was exactly this.

I said "considered by the group", not "in existence". My understanding is that it's down to 3 vs 4 right now, and all other proposals are off.

devongovett commented 1 year ago

Exactly my point. This was pushed through, despite significant pushback, by a few members of the working group with very strong opinions.

I'll back out of the discussion now because it doesn't seem to be going anywhere, but I am disappointed by the process here.

pyronaur commented 1 year ago

Exactly my point. This was pushed through, despite significant pushback, by a few members of the working group with very strong opinions.

I'll back out of the discussion now because it doesn't seem to be going anywhere, but I am disappointed by the process here.

This is why I don't even attempt to participate in the discussion.

If Scss syntax can't be used, I don't care what the outcome is, I'll still use a preprocessor anyway.

If efficiency is what browsers care about, maybe a new "efficient syntax" needs to be created instead and we should all use prepeocessor of choice to compile to that.

cdoublev commented 1 year ago

The spec is now updated to Option 3, after a quick hack session with @argyleink

Can you please clarify why consume a style block's content looks for an identifier or a functional notation to consume a declaration? Is it to anticipate a future declaration syntax?

  • <ident-token>
  • <function-token>
  • <function> Reconsume the current input token. Initialize a temporary list, initially empty. As long as the next input token is anything other than a <semicolon-token> or <EOF-token> , consume a component value and append it to the temporary list. Consume a declaration from the temporary list. If anything was returned, append it to decls .
LeaVerou commented 1 year ago

@cdoublev See https://github.com/w3c/csswg-drafts/issues/7834#issuecomment-1283019419

devongovett commented 1 year ago

Should this issue be closed, or is something still outstanding here? I see the spec was updated already in d30c9f6. I have implemented the changes in Lightning CSS (https://github.com/parcel-bundler/lightningcss/pull/340) as well, and will ship soon behind a flag, but if more changes are expected I will hold off.

tabatkins commented 1 year ago

Note just as a general rule that anything is subject to change until a browser ships it to their stable channel and it starts being used by a non-trivial number of pages, so that changing it would cause too much breakage.

That said, I personally do not believe it will change further from where it currently is.

fantasai commented 1 year ago

The CSSWG resolved on Option 3 as described in these minutes.

LeaVerou commented 1 year ago

@tabatkins do you happen to know what happened to the Markdown file of proposals? I just looked it up for a TAG discussion and it appears to have been deleted (I could only find it in the history, here)

tabatkins commented 1 year ago

Yeah, I deleted it earlier this month; it didn't seem like it was still needed. If you want to preserve it as an artifact, feel free to do so.