-
Notifications
You must be signed in to change notification settings - Fork 686
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] Another problem with mixing declarations and rules - error recovery #8349
Comments
I think this is only an issue if CSS authors expect nested CSS to have aspects of graceful degradation / progressive enhancement. But I don't think they do. /* no one writes nested CSS with this mental model : */
.foo {
/* base styles */
&:hover {
/* enhancement */
}
} CSS authors are aware that any use of nesting will result in a broken and unusable result in older browsers without native support.
Nested CSS might break down in weirder ways than some might expect but I don't think how it breaks is particularly important. It is also why I spend so much of my time crafting tools to transpile features like these for older browsers. Authors who need to support older browsers and want to use nested CSS can use tools like these (as they do today). |
I disagree. CSS has had forward compatible parsing and graceful degradation since day one. It's a core feature of the language. Of course authors should not expect nested rules to work in older browsers, but they will not (and should not) expect them to break other aspects of the stylesheet. Nothing should get ignored except the nested rules. I do accept that due to the nature of nesting, most authors will either use a preprocessor to generate the stylesheets they server to all clients (which makes our entire nesting feature irrelevant), serve different stylesheets to newer clients than older ones (which is fragile, ask any browser vendor how much trouble UA sniffing has caused), or will simply use nesting and not care/test on older browsers. Some authors may not care about older browsers, but we need to. We can decide to just live with this (see option 3 above), but if so, we need to make that decision with our eyes open and we also need to give the proper advice to authors on what to expect and what best practices to use in their stylesheets. |
Yes, but this can only go so far.
The same would be true for
They should and do expect the rendered page to be fully broken. Exactly how it breaks at a tokenizer/parser level is something we care about, but CSS authors and end users do not. I am only talking about nesting here, for many other features forwards compat is absolutely critical
This seems fine to me.
|
I fail to see how " div {background: green}
@layer {}
p {background: red} On browsers that don't support background: green;
.nested {}
background: red; On browsers that don't support nesting, |
absolutely true, at a tokenizer/parser level. My comments are specifically about CSS author expectations. The way nested CSS is written today doesn't look anything like these artificial examples. .component {
/* few or no declarations, "component" is mostly just a wrapper, "@scope" doesn't exist yet */
color: cyan;
.element-a {
color: blue;
&:hover {
color: pink;
}
@media screen {
color: green;
}
/* maybe some more deeply nested elements ... */
}
/* more elements ... */
/* no declarations at all */
} All the important bits are thrown out. No one is using div {background: green}
@layer {}
p {background: red} Much more common to see this : @import url('bootstrap.css') layer(bootstrap);
/* overrides on top of bootstrap here */ No one expects their overrides on top of bootstrap to result in a working page when the import is ignored. The entire foundation is gone. In this aspect nesting and |
Yes, Romain is making the right point. While an Nesting is in exactly the same boat. Your nested rules will simply get thrown away in older browsers. There's no way to work around this; you have to either buy in to Nesting (accepting that your stylesheet will be broken in older browsers) or avoid it entirely. And if you're buying in, the error-recovery change doesn't matter; it'll work correctly in the browsers that your stylesheet works in, and it'll be extremely broken in older browsers no matter what. This is just how adding newer syntax features works. It's happened in the past, and it'll happen in the future; Nesting is perfectly normal in this regard. It's awkward during the transition period but stops being a problem as usage numbers advance. We certainly prefer designing features that can gracefully fallback instead, but if sometimes isn't an option. All that said, this particular issue only occurs if you put properties after rules, which is a bad practice anyway. Put all your properties at the top, before any rules, and you'll be fine. Putting them later gives you absolutely nothing except possible confusion. |
Of course new functionality won't work on old browsers. This issue is about the old functionality that follows it.
That's not true in general. The
Again, not necessarily, e.g. here I would be fine with losing the cosmetic shadow on hover, losing .grid {
&:hover { box-shadow: 0 0 5px }
display: grid; /* won't work on old browsers */
grid-template-columns: repeat(auto-fill, 100px); /* won't work without display: grid */
}
I can't recall a single new feature that had this effect. It's not the case for property values, property names, selectors nor at-rules. When these things are not recognized, they don't affect the next declaration or rule. The closest thing that comes to mind is selector lists, where a single invalid branch invalidates the others, but it still only affects that rule, and this behavior has caused lots of frustration.
1st time I have seen this described as bad practice. https://drafts.csswg.org/css-nesting/#mixing just says "all three can be arbitrarily mixed". If it's bad practice then the spec should warn authors. |
I'm not advocating for some workaround that will make nested rules work in an older browser, and of course I'm aware that a stylesheet that's naively written for a browser that supports nesting isn't going to give a good result in an older browser. That's not my point.
My point is that while stylesheets will break in older browsers, they should break in a predicable way so that anyone who wants to provide fallback behavior for older browsers can. This is a fundamental aspect of CSS as you're well aware. Saying 'it's partly broken, so who cares how broken" is not generally the right answer. The behavior of swallowing the first declaration after a nested rule will be surprising to most authors.
The transition period is important. If we don't provide a functional mechanism to support both older and newer browsers, it will hurt the uptake of the feature until the vast majority of browsers support it. Of course due to the nature of nesting an 'all-or-nothing' approach probably makes the most sense for most authors. To this end, we need to make sure that nesting is feature detectable so that authors can include both smaller stylesheets that use nesting, and larger ones that don't for older clients. (I expect preprocessors to generate both from a single source.) That doesn't mean that we should ignore fallback behavior, CSS isn't versioned.
Agreed, but if this is the approach that we take (as listed as option 3 above), at a minimum we need to document putting properties first as best practice and explain to author what will happen if they don't. We just spent a large part of a meeting discussing how mixed rules and properties work, and no one said "don't do that". We also need to take this aspect into consideration when we consider alternatives. |
…perties and nested rules willy-nilly. #8349.
Yes, one can imagine a scenario in which
I didn't say we'd added new parsing constructs that invalidates the next property, I said we've added features that are all-or-nothing - either you only care about browsers that support them, or you don't use the feature at all, because it's used in a wide-ranging and integral way that isn't realistically fall-back-able.
An older version of the spec did more explicitly warn against it. Having a warning put back in is probably appropriate, just for reasons of readability. (Added now.)
In the (likely exceedingly rare) cases someone does want to write a stylesheet using nesting while providing some sort of content for those without, the rule is: put your properties first.
Right, we've said if often enough that it didn't particularly bear repeating, plus "don't do it" isn't a valid response to "what should the behavior be when someone does it". |
Another viable approach is to put a And to be clear, I'm not raising this as a showstopper, I'm just saying that it needs to be considered, and the WG as a whole needs to decide to just live with it or not. If we do decide to live with it (which I'm ok with, but not thrilled about), we do need to make authors aware of the issue and provide guidance. That's why I offered that as an option in the first place. If we wind up in a place where we reconsider the |
Yup, that works, it's just not necessary if you do the more readable thing in the first place. ^_^ I'd prefer not mentioning it in the spec, since there's a more preferable option in the first place. |
It might be worth mentioning anyway. We need to be careful with the lookahead algorithm, if it's possible for an invalid rule to be considered as a declaration then it could trigger declaration error recovery, which will also swallow all the following nested rules. Putting a |
Right, that's part of why I prefer the current spec's behavior wrt #8251 right now; by tilting toward parsing anything unknown as a rule, you get safer error recovery. The proposed infinite lookahead algo (#7961 (comment)) also leans in this direction. Of the three ways to trigger property parsing, the first ("starts with a dashed-ident") is "unsafe" but not a realistic confused-for-selector scenario; the second ("starts with |
I agree that we should bias towards rule recovery, where we'll get into trouble is if we ever add something like: new-property:something { something } color: red; and the part of the declaration after the Where we need to be really careful are the cases where we use lookahead to tell if something is a rule or property, and guess wrong because it's invalid either way. Falling into declaration error recovery by mistake could be really bad.
Agreed, but one of the options here is to make properties after rules invalid, turning 'best practice' into 'mandatory practice', which would make all the spec'd behavior of mixed rules and declarations moot. (I'm not in favor of this approach, fwiw, because older browsers will only skip the first declaration after a rule, not all of them, so it's still inconsistent behavior.) |
Yup, that exact pattern is documented in my lookahead parsing comment as a design restriction we'll have to abide by - non-custom properties will still be able to start with a {}-block (allowing ideas like explicit shorthands or whatever), but wouldn't be able to contain a top-level {}-block after the start.
Agreed, thus my analysis in my previous comment. So long as CSS abides by the design restriction, one of the possible "parse like a declaration" results is safe, and the other two, while still "unsafe" (that is, they'll eat following rules at the same level, until they see a |
Note that the problem I was trying to highlight is actually the reverse. If a new, non-custom property has a |
Yeah, in theory that's possible, if exceedingly unlikely; we haven't put colons into any property yet, and that's the minimum that would be needed. We could potentially pre-commit ourselves to not do that (only use {}-blocks at the top level of a property as the whole value), but I think in practice that'll be the case anyway. |
Just to quickly lay out the error-recovery things again, to make it clear without having to dive thru an issue:
For (1), putting decls after your rules is already not a great practice for readability; if authors put all their decls first (like the spec recommends) then there's no additional problem. (And in any case, all the rules being dropped is already gonna result in a broken stylesheet, so further breakage is fairly insignificant.) For (2), these restrictions were more or less already guaranteed; neither are syntax space we've ever even thought about exploring. For (3), this is only theoretically a problem for non-HTML host languages, where it might be possible to have a dashed-ident as an element name. In HTML, tho, this is illegal - custom element names are required to start with an ASCII alpha character in the first place. So |
I'm not sure if this has been discussed before or not, but I just realized there's another issue with both option 3 and the lookahead approaches to nesting. Error recovery.
There are different behaviors for error recovery with declarations and rules, an invalid declaration consumes everything up until the next unenclosed
;
, where invalid rules consume everything up until the next unenclosed;
or block{}
.Consider the following example:
What color is the background of the
div
? In a browser that doesn't support nesting, it'll be green, in a browser that does, it'll be red. This is a problem.Having a
&
prefix doesn't fix this. Lookahead doesn't fix this. AFAICT the only thing that does is a full at-rule prefix, e.g.@nest
(even a bare@
doesn't fix it) because this causes the browser to shift back into rule recovery mode when it was expecting a delcaration.I see a few paths forward here:
;
after nested rules. I think this is fragile.;
between the last rule and the first following declaration if they want compatibility with older browsers.I'm open to other suggestions...
The text was updated successfully, but these errors were encountered: