-
Notifications
You must be signed in to change notification settings - Fork 682
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-nesting-1] Syntax Invites Errors #7834
Comments
[ @LeaVerou, @jensimmons, @bradkemper, @tabatkins, @mirisuzanne and I discussed this problem today; this comment is a summary of the discussions.] The following options outline the available syntax space for nested style rules:
The first option has the problems outlined above. Regardless, once we're in rule-parsing mode, in order to make nested style rules compatible with style rules elsewhere, the desire is to allow both:
In terms of syntactic triggers for a rule-parsing mode, the following options were considered:
After the trigger, parsing would continue in rule-parsing mode and not in declaration-parsing mode, so &-prefixing would no longer be required for subsequent nested style rules. Forwards compatibility considerations:
Backwards compatibility considerations:
The recommended option from the discussions today was to allow as a trigger both:
Pros/cons of triggering on &-prefixed rules: Pros/cons of triggering on Pros/cons of triggering on new delimiter: Agenda+ to discuss |
Pulling out for clarity, the suggested change is:
Taking the following example Sass from the original Nesting issue: .main-nav {
display: flex;
ul {
list-style-type: none;
li {
margin: 0;
padding: 0;
}
a {
display: flex;
padding: 0.5em 1em;
}
}
nav& {
display: block;
}
} You'd write this in the new proposal as one of the following: .main-nav {
display: flex;
& ul {
list-style-type: none;
& li {
margin: 0;
padding: 0;
}
a {
display: flex;
padding: 0.5em 1em;
}
}
nav& {
display: block;
}
} or .main-nav {
display: flex;
@nest;
ul {
list-style-type: none;
@nest;
li {
margin: 0;
padding: 0;
}
a {
display: flex;
padding: 0.5em 1em;
}
}
nav& {
display: block;
}
} Which you use is up to preference. |
I think this proposal introduces several issues. A person needs to have seen the parsing switch to be able to understand the code. In a reasonably large file this might be dozens of line above, way out of view. From their perspective the code looks indented as if from a conditional rule. A syntax highlighter might not be parser based and will have difficulty with this. Overal I think this solves some writing issues but negatively affects readability. Diffing code is a good example I think : margin: 0;
padding: 0;
}
- & a {
+ & b {
display: flex;
padding: 0.5em 1em;
}
vs. margin: 0;
padding: 0;
}
- a {
+ b {
display: flex;
padding: 0.5em 1em;
}
|
I don't think that's actually true in practice. The code means the same thing either way. The switch just changes if that code is valid or not. And that switch is only needed where you change from declarations to nesting. In a reasonably large file, you can tell what's valid from context. If you're looking at nested selectors, then you're past the point of the switch, and you can continue using nested selectors. If you're looking at declarations, then you should see a switch as you scroll down to nested stuff. If you are adding nested stuff right after declarations, you need to add a switch. If you're on either side of that switch already, and it's off-screen - it's pretty clear which side you are on from context. |
Part of the implication of this proposal is that you never mix back-and-forth between nested rules and declarations:
|
Right, that is my main hesitation about this proposal. We get some good copy-and-paste behavior at the expense of a different copy-and-paste problem, where people who regularly paste new declarations at the end of an existing block may not get what they expect. |
One thing I'd like to point out is that in the code examples @tabatkins posted, using However, in real nesting use cases, the first few rules are often (though of course not always) specifying variations of the base rule, e.g. section {
declarations;
&.main {
declarations;
}
h1 {
declarations;
}
p {
declarations;
}
...
} so the parsing switch often comes naturally, whereas with Note that this is exactly how the above code would have been written in Sass (which was designed without our parsing constraints and thus is IMO the most natural syntax for this, so the closer we can get, the better). |
a) Interleaving declarations and rules makes code harder to read, you now have to hunt down the entire rule with all its descendants to understand how the base element is styled, so I'm fine disallowing that. |
I'm a bit concerned about the implications of this for editing of style sheets. In particular, it introduces cases where deleting a rule (which has the initial Maybe it's something we can live with given all the constraints here, but I just wanted to point out explicitly what this implies about removing rules from style sheets in the process of editing them. |
maybe we should split out concerns/feedback into separate issues? @romainmenke said :
@mirisuzanne said :
The case I had in mind was this : @media (min-width: 300px) {
/* a lot of css */
margin: 0;
padding: 0;
}
a { /* "a" is just "a", no nesting */
display: flex;
padding: 0.5em 1em;
}
/* a lot more css */
} vs. .my-component {
/* a lot of css */
margin: 0;
padding: 0;
}
a { /* "a" is ".my-component a" */
display: flex;
padding: 0.5em 1em;
}
/* a lot more css */
} I've always seen the required Maybe I am alone in thinking that the more verbose syntax of the current draft is actually a good feature :) |
I initially had this concern too. I think in practice, I would probably add the The point is, you could always continue to prefix each line with a |
Wish I was invited to this call. |
But you could still be that verbose if you want to. You could still proceed each rule with an And actually, you could still write @nest multiple times and in multiple places between rules if you wanted to (switching to a mode you are already in). So I don't see why you couldn't write it in the previous sustains if you wanted, and it should still parse the same, I think. This also means you could write the rules in a SASS compatible way. |
Also reading a lot of statements on Twitter that this change would allow you to copy/paste from Sass. But that would not be true as I understand it. Sass and the nesting specification would still be different for complex selectors. .a .b {
/*
might need a preceding `@nest;`
or a might need to be `@nest .c &` depending on final syntax
*/
.c & {
color: green
}
} sass : .c .a .b { /* "a" is a descendant of "c" */
color: green;
} current draft : .c :is(.a .b) { /* "a" and "c" might be the same element, or one might be an ancestor of the other */
color: green;
} I think it is misleading to present this change to the specification as "compatible with Sass". |
Do note that it's far easier to debug all your nested rules suddenly not being applied, than the current situation where it's per-rule and thus leaving out a necessary
This is orthogonal to nesting syntax, and is true regardless of how the nested rules are specified. I think what people on Twitter are rejoicing about is that this makes it easier to migrate from Sass. The edits required are now O(N) on the number of rules with children, not O(Nk) where k is the number of child rules per rule, and in practice even fewer as often the first nested rule starts with |
I think it's pretty strange and confusing to have different requirements for the first nested rule vs the others just to avoid a single character in some cases. Also agree with the above points about making editing (eg inserting or reordering rules) more difficult. Better to just be consistent about it IMO. I think, just like variables, people coming from SASS etc just aren't used to the syntax yet. Once it becomes standard and everyone starts using it, it'll become second nature. |
There are, for sure, tradeoffs and issues with every option we've considered here. All of that is well documented in the proposal above. They all introduce potential footguns for authors, and they all come with caveats at the edges - issues that will impact some code styles more than others. From my perspective, the priority of this proposal was a flexible and forgiving syntax. In the most common cases, it just works - and authors can migrate from existing code (both Sass and the PostCSS polyfill) easily. It's also possible to copy/paste that code to the root of the document, or into a scope rule, etc. The implications change slightly in those different cases, but in each one the code makes sense - and has the expected behavior. From there, authors can choose to make various improvements, based on their preferred code styles. This was also very clearly true of the existing syntax - where some authors may choose to require Even though I wouldn't rely on it for large projects, I still think it makes sense to allow that flexibility as far as possible, so that most things will just work. It's unfortunate that as far as possible stops with the first We are not going to achieve a perfect syntax here that everyone loves. But we can achieve one that has the flexibility to handle most of what authors throw at it. |
I understand that for professional developers flexible and forgiving is not always the priority we choose. Most of us use linters to make our code less flexible and less forgiving. We can still do that! But I believe flexible and forgiving is actually a pretty good guiding ideal for CSS itself, even if many of us will use tooling to enforce more consistent code styles. |
To add to Mia's excellent responses above, it is actually fairly common for the last thing in a sequence to have more forgiving syntax, for example:
I'd argue the last n-1 things having more forgiving syntax than the first one is a fairly similar pattern to the first n-1 things having the stricter syntax and the last one having the more forgiving syntax. Also, there are two ways to frame this:
I wonder if some of the people against this proposal would be more amenable to the second framing? |
If viewing it as an error-correction why couldn't it be inserted whenever it encounters a non-literal token? That way it would only require an extraneous |
I'd actually be fine with that. |
I think the important thing is to be consistent. Requiring Sure, lint rules can be invented, but why create this confusion in the first place? IMO, if we're even talking about a lint rule being needed to disable a new language feature, it's probably not a good idea. Especially since the only benefit is saving a single character per rule. Perhaps users of SASS will need to re-learn things, but the majority of CSS developers have never used SASS so I don't think this is a good argument. Plus, the nesting syntax already works differently in other ways as @romainmenke mentioned, so re-learning will be necessary either way. Clarity and consistency is far more important than SASS compatibility in my opinion. |
I think it is also important to challenge the opinions that sparked this.
This is not solved by this syntax change. It can only be resolved by giving
This is untrue. Regex will never be sufficient for migrations from Sass to nested CSS because complex selectors work fundamentally different. This is also a one time event and should not be used to motivate a lasting language design.
The same is true for I am really trying to see the upsides of this proposal and have begon working on some sample code that compares certain aspects. Doing this so that I can get a feel for this version of nesting. https://github.com/romainmenke/nesting-proposal-changes-7834 At the moment however I am really struggling with this. Correction I initially overlooked the addition of relative selectors which enables This comment has been edited to strike through the incorrect statements. |
That only solves it one way: you can then paste from a nested context to
Nope, under this proposal
Both
This sample code demonstrates my earlier point that for a lot of code, no non-optional ampersands will even need to be included. |
@romainmenke I took a stab at converting your sample code following this proposal to sample code with a mandatory This is the result:.block {
color: green;
box-sizing: border-box;
height: auto;
padding-left: 1.25rem;
padding-right: 1.25rem;
position: relative;
width: 100%;
z-index: 1;
@nest;
@media (min-width: 48rem) {
padding-left: 2rem;
padding-right: 2rem;
}
@media (min-width: 80rem) {
padding-left: 3rem;
padding-right: 3rem;
}
@media (prefers-color-scheme: dark) {
color: lime;
}
&:hover {
outline: 2px solid currentColor;
}
&.block--orange {
color: orange;
@nest;
@media (prefers-color-scheme: dark) {
color: yellow;
}
}
.block__element {
align-items: center;
display: flex;
flex-direction: column;
justify-content: center;
right: 2rem;
text-align: center;
top: 50%;
transform: translate(-5px, calc(-50% - 1.625rem));
z-index: 2;
@nest;
@media (min-width: 48rem) {
right: 3rem;
}
@media (min-width: 80rem) {
right: 4rem;
}
.block--orange & {
text-decoration-color: black;
@nest;
@media (prefers-color-scheme: dark) {
text-decoration-color: white;
}
}
}
} Is this preferable? Is it more understandable? |
No this is, in my personal opinion, much worse. :) But in these examples
This coding style makes it highly unlikely that you will ever need to use
Not saying that. I am concerned about a parser switch that appears once. |
Yeah, I've been thinking thru the parsing implications, and this should be doable. I need to be a little careful, because people today rely on "put some random symbol at the start of your property to 'comment it out'" and that needs to be preserved, but I believe I can handle this very reasonably in the spec. Currently the parsing rules for style blocks are (ignoring the ending conditions and the one bit that's for the current Nesting spec):
To accommodate this new bit, I'd instead have:
Then "consume an ambiguous rule" is identical to the existing "consume a qualified rule", except it'll fail early if it encounters a top-level semicolon, raising a parse error and returning nothing. I believe it's okay in practice to defer a "parse this as X or fail" decision until later in the stream, while "parse this as X or as Y" needs to be know which with small, finite lookahead (which is why "just do it like Sass" is problematic for our impls). Note, tho, that this will slightly tie our hands in the future - we'll never be able to change the property syntax to start with a non-ident (like doing additive CSS by writing |
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.
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 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. |
We don't "normalize" serialization for other relative selectors, like in :has(); this should be the same.
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.
The section that defines the nesting selector still talks about the |
PostCSS plugin changes are mostly done : https://www.npmjs.com/package/@csstools/postcss-nesting-experimental |
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. |
Chromium as of 109.0.5394.0 is updated with the new syntax. |
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 Something like this: @nesting
.my-class {
nested-tag {}
.nested-class {}
} |
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).
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). |
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
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). |
There are a lot of assumptions and personal opinions being thrown around here. Your own poll showed that developers prefer to write the |
This has already been addressed, I think by @fantasai: The poll showed that about half of developers (52%) want to include Also, if you read the comments, there was some confusion about what an optional I understand that you feel strongly about this, but right now none of the proposals considered by the group involves a mandatory |
For starters, I personally like the syntax of proposal 3 on principle. 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. 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 will miss I do have strong opinions about a parser switch. 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 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. |
It should be noted that this poll has significantly fewer votes. The difference between them is also very subtle.
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. |
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. |
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?
|
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 (parcel-bundler/lightningcss#340) as well, and will ship soon behind a flag, but if more changes are expected I will hold off. |
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. |
The CSSWG resolved on Option 3 as described in these minutes. |
@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) |
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. |
Edit: 👉🏼 UPDATED SUMMARY TABLE OF SYNTAX PROPOSALS 👈🏼 (from #7834 (comment) )
As mentioned in #7796, there are some problems with the currently-proposed nesting syntax:
@scope
, which invites a lot of copy-paste errors when transferring code among these contexts&
within all the selectors in a list even beyond syntactic disambiguation (instead of allowing relative selectors) is annoying to type, easy to forget, and makes automated (and manual) conversion of existing code much more difficult (requires selector parsing rather than regex).&.foo
and& .foo
are easily confusable (both for reading and for typing), and so long as the latter is required for nested relative selectors this will be a commonly encountered problemThe text was updated successfully, but these errors were encountered: