-
Notifications
You must be signed in to change notification settings - Fork 352
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
mixed_decls
produces too much noise for developers with mixins
#2276
Comments
I again examine the patch change. It seems that the patch just reflects a new warning and behaves the old way, in this case I strongly recommend to provide a option to silence the warning with (all files / only mixins). No one is expecting his log to contain 100+ warnings which can not be silenced Also, I think dart team should reconsider, what's the best behavior for mixins. |
They will though. But specification doesn't tell if we can rewrite rules after @apply. If we can, then this warning is even missleading. |
fully agree with @Mister-Hope in here and don't see any particular reason this breaking change was introduced in the first place. |
It's ok to add such hint to help people migrate to the future v2 smoothly, but the biggest thing is that sass not only support nesting selectors, but also mixins, and the latter is a huge problems to all users. |
Moreover, the deprecation message is too long (probably 20+ lines each), and with 100+ warnings, you can hardly fine other warnings and errors in console when integrating into a vite/webpack project. Imagine previously your build log is clean with only <50 lines, and now it's more than 2000 lines. That is super annoying. For me I am building tools like vuepress and vuepress-theme-hope (both >2k stars), I am not an end-point user which I can pin the deps easily. Downstream users are suffering this with my current code and I can only give them instructions pinning sass to an earlier version through advanced solutions like |
These warnings can be silenced with |
Unfortunately, I could find no assurances that the "upcoming change" or the "future release" will only arrive with v2. If that is the case, it has better been made explicit. Alternatively, a few therapy recommendations would be fitting. |
This could be a nice workaround at the moment, but further improvement about mixins should still be discussed :) |
Let me address a few different issues here: When and where should we debate this change?If you want to debate upcoming changes to Sass, you need to do so when those changes are out for public review, not after they are approved and released in the language. We take great pains to make sure users have the chance to have a say in the design process of the language, but you have to actually participate in that process if you want your voice to be heard. This proposal was out for review from 30 April until 18 June, and we made public announcements on X and Mastodon soliciting comments. We will very occasionally make changes to a feature after it's approved and landed, but the burden of proof for making adjustments is much higher. If you want a say in how Sass evolves, you need to participate in the discussions as they're happening, not come in with complaints after the fact. Okay but is this change necessary?Yes. It is a core design principle of Sass that any valid plain CSS that appears in a Sass file should have the same meaning as it does in plain CSS. Even if this particular application of that principle annoys, I guarantee that the principle as a whole is necessary to make Sass approachable and comprehensible. The moment a user can't rely on standard CSS semantics, they have to second-guess everything in a Sass file, which makes it impossible to learn or use with any confidence. The use of mixins here is not an exception to this point. Mixins may not be in CSS yet, but they are transparent to CSS: writing styles in a mixin that you include should always behave the same as writing those styles directly. This is called referential transparency and is another core design principle (not just of Sass but of most programming languages). If moving a chunk of re-used styles into a mixin caused other declarations to be ordered differently, refactorings that should be safe start to have unpredictable and dangerous side-effects. That's something we have to avoid. But shouldn't it be a bigger version bump?It's generally our philosophy that it's valuable to release deprecation warnings as soon as possible. The role of a deprecation warning is to give developers time to prepare for a breaking change, and the more time you have to prepare the better. This means we don't want to hold them back for major version bumps. That said, looking over the semver spec it does say:
...so in keeping with that we will in future release new deprecations as minor version bumps rather than patch bumps. The warnings are too noisy!As @ntkme points out, you can silence specific deprecation warnings in Sass. If you're confident that the new ordering won't affect your styles, go ahead and silence this warning and you won't ever need to think about it again. Unless you pass When will the breaking change happen?Dart Sass's compatibility policy states:
As such, the release that removes these deprecations and begins emitting interleaved declarations in-place will be no earlier than 9 October 2024. I can't give a more specific date than that because the details depend on developer bandwidth and team prioritization which is always difficult to predict. |
From your message, I understand your team do great work try to make some comming changes to everyone before landing. But definitely there are a lot of developers like me are relying a lot of packages, so I believe there are a lot of People like me, which only have time to review major changes with some common packages we use while they are in beta.
So if I understand everything well:
Then unless developers choose to add |
In an upcoming version of Sass, the default will change and mixed declarations will be emitted in the order they were written, rather than being hoisted to the top of the style rule. See https://sass-lang.com/d/mixed-decls for an example of the difference. This change is unlikely to cause style breakages because nested rules generally have higher specificity anyway, but it's not impossible so we need to release a deprecation warning in order to give developers time to prepare. I understand that this is annoying. It would be much simpler for everyone—including me—if CSS kept the original semantics for mixed declarations, which matched Sass's. But we have to deal with the world as it is, and given our core design principles there's really only one path forward. |
You are not answering my question directly. Thanks for the reply, and your reply answered most of my questions and I believe should solve others concern. But, one more thing that should be important to most developers. In the new version are the current issues I am reporting doing the new way without warnings, or I still must opt-in If this is the former way, then we can tell users to hide this warning in the next coming three or maybe more months, But it is the latter way, I think I still need to take time to add the This is confusing me because the standard css does not require |
I can't get this option to work, here's an extract from my webpack config:
|
https://webpack.js.org/loaders/sass-loader/#api You have to use |
Thanks @ntkme! It's |
Once the breaking change is released, there will be no need to write It's standard when producing a deprecation message to provide a way of writing styles that will work the same in both older versions and newer (in this case upcoming) versions of the language. That's what The reason plain CSS doesn't have a similar migration plan is that plain CSS nesting is new enough that there's a negligible corpus of existing users who might be broken by this change. The same is not true for Sass, which has had the existing nesting semantics for almost two decades. |
Hi, @nex3, webpack solution should be fine as sass-loader can opt-in to use the new compile API. I was wondering if there is any workaround to silence the warnings in Vite, see https://github.com/vitejs/vite/blob/main/packages/vite/src/node/plugins/css.ts#L2148. Vite is still using an old one, and this issue just leaves as is for quite a long time: vitejs/vite#14689. So does it mean telling users to fix sass to 1.77.6 is the only way for downstream developers before anyone pickup vitejs/vite#7116 |
mixed_decls
produces too much noise for developers with mixins
The main issue in here is if native CSS is structured incorrectly (and in this case it's still quite questionable!) it won't break the whole website builds. |
I edited my first comment and add a conclusion, and I hope it helps newcomers to quickly review what is going on here. @nex3 If would be great if you can check if there are issues in it. |
The modern JS API has been available for two and a half years, so it's disappointing to see major frameworks still not supporting it. You can also silence specific warnings by implementing a custom logger, which is supported in the legacy API. You can build in logic to such a logger to filter out specific deprecation warnings, including this one. |
@nex3 could you please reopen this thread as the issue is still ongoing and quite active and hasn't been resolved yet. |
While deprecation warnings are annoying, they are not breaking change. They are just telling you to be prepared for an upcoming breaking change. As of now, the output behavior has not been changed. It's really up to the frameworks authors and end users to either update the sass code or just silence the warnings. Introducing a new deprecation in a patch release was a mistake by semantic versioning definition, but honestly I don't think the situation will be any better if it was properly introduced in a minor release as most of the users have dependencies locked down at major version. In my opinion, the most frustrating part of the issue is that sass already provided updated infrastructure in new JS API with deprecation API for fine-grained control of deprecation warnings, but the web bundlers make it difficult (webpack) or impossible (vite) to be used. The gap in user experience between standalone sass and third party integration of sass is problematic, and need to be addressed by those bundlers. Meanwhile, I do think there are things we can improve for the deprecation process. Lots of people complained about communications despite this change has been announced via multiple channels ahead of time. Here are some ideas to make it better:
|
I like those ideas, but I'm concerned that they might take a lot of effort from us in exchange for potentially minimal value. Given how slow most downstream users seem to be to make changes even when they are directly contacted (see Vite's 2.5-year-old issue to use the new compiler API), it's not clear to me that a test-ahead scheme would actually make much of a difference. On the other hand, it does come at a certain degree of cost. Not only would it consume precious bandwidth from our already-stretched-thin team, it also means it'll take longer for users who can directly update their stylesheets to see the deprecation warnings, which means that either they'll have less time to update or the new deprecation-blocked features will take longer to land—both of which are user-negative. All that is to say: if any frameworks are interested in proactively working with us to avoid deprecation warnings, I'm definitely interested in developing a process for that, but I'm not optimistic about something fully driven from our end. (It is worth noting that we don't necessarily need to publish snapshot builds; we already have the technology for releasing "future deprecations" which users can opt into on demand.) |
Maybe for all the new deprecations we are going to add, they should be announced and released as "future deprecations" for at least one minor release cycle (or some fixed time) before they are activated, even for the fast-tracking deprecations. - That is to say, no deprecations will be activated immediately at release. Then it's up to framework developers to monitor sass announcements and test new future deprecations before they are activated for the end users, given us a chance to reduce direct impact on end users. |
I'd be all for that if we had any indication that even a few frameworks would actually monitor them. But I don't know that such a thing is realistic, given how downstream consumers have historically ignored any changes that didn't print visible warnings on the command line. |
It seems that, in the specific case of mixins, it is quite painful to write code that doesn't trigger the deprecation warning. For instance, this code: .alert-warning
@include alert
@include warning
background-color: yellow must be rewritten (as far as I understand) as below, in order not to trigger the deprecation warning: .alert-warning
@include alert
&
@include warning
&
background-color: yellow A question : would it be possible for Or to have a separate deprecation warning switch for mixins, so that the mixins case can be silenced/ignored? |
@kemenaran The recommended way to handle those mixins is to update them to use .alert-warning
background-color: yellow
@include alert
@include warning As I explained above, we can't exempt mixins from this because mixins will be included in the eventual change to output order. And separating out a different deprecation name would just make more annoying for users to silence all deprecations in the common case that that's what they want. |
@nex3 could you please just think about this small example in terms of how much code had to be refactored across billions of projects because of this change that is forced by a preprocessor that still compiles everything in CSS.... |
@maksymmamontov I'm sympathetic to that, but again our hands are tied here for the most part. We can't just diverge from CSS here; it may not seem like a big deal right now when CSS nesting is new and not yet widely used, but in five or ten years when developers have thoroughly internalized CSS's semantics as the logical way for nesting to work it would be a huge problem for Sass to work differently. Really, if you're unhappy with this breaking change, you should take it up with the CSS working group—we're just following their lead. |
This approach works, I was able to silence several hundred warnings without adding a single &{…} block. However, every change I made had to do with the placement of root declarations relative to nested blocks. Even when the nested blocks contained a '&', the order of these blocks relative to sibling blocks did not seem to affect the deprecation warnings at all. I now assume that eliminating all warnings makes my SCSS fully ready for the Dart Sass adaptation to plain CSS nesting. |
Works around deprecations of mixed declarations as documented at https://sass-lang.com/documentation/breaking-changes/mixed-decls/ by re-ordering where possible, including allowing mixins to be included later in declarations where possible, generally following the recommendations at sass/dart-sass#2276 where declarations are not being overridden by the mixin, or ordering needed to override something from within the mixin. There are still warnings from within foundation-sites to address, and some stylelint rules temporarily to disable where `& {}` workarounds are needed, usually from one of the two override cases.
I've just faced this issue now and thinking about from the perspective of how it Let's say I have this code: @mixin form_input {
color: red;
&:disabled {
color: gray;
}
}
textarea.large.green {
height: 300px;
@include form_input;
color: green;
} If I understand correctly it produces this CSS: textarea.large.green {
height: 300px;
color: red;
color: green;
}
textarea.large.green:disabled {
color: gray;
} So here color is repeated, but that's a small overhead for the convenience of writing my code this way. My problem is that right now the above code says: use all the default styles from my mixin, but override textarea.large.green {
height: 300px;
color: green;
@include form_input;
} It no longer works, because textarea.large.green {
height: 300px;
color: green;
color: red;
}
textarea.large.green:disabled {
color: gray;
} But if I leave it the way it is (aside from the warnings until this change goes live) I am now producing a CSS that not only duplicates the textarea.large.green {
height: 300px;
color: red;
}
textarea.large.green:disabled {
color: gray;
}
textarea.large.green {
color: green;
} Indeed this is a fictive example, but I have some mixins that are quite large, they define many styles and I want to occasionally override some of those. If the selector is also long (not like in the example above), which can happen, than I'm bloating my final CSS with the long selector being repeated, which can be significantly more useless overhead in terms of css size than just overriding a property. Another thing I quite liked and used 1000s of times in my codebases is to put overrides beside what is being overridden for improved readability. This naturally resulted in code where styles are interleaved with subselector override styles and media queries. This is a huge benefit as opposed to CSS, that I can see every potential override for a style together. This becomes pretty much impossible now, unless I'm willing to have CSS output that repeats the same selector over and over again. S.g. like this: .some-element {
color: red;
&.green {
color: green;
}
&.blue {
color: blue;
}
background-color: white;
&:hover {
background-color: black;
}
&:disabled {
background-color: gray;
}
min-width: 800px;
@include for-tablet {
min-width: 400px;
}
@include for-mobile {
min-width: unset;
}
} Very easy to see certain aspects defined together. But now I have to rewrite these to: .some-element {
color: red;
background-color: white;
min-width: 800px;
&.green {
color: green;
}
&.blue {
color: blue;
}
&:hover {
background-color: black;
}
&:disabled {
background-color: gray;
}
@include for-tablet {
min-width: 400px;
}
@include for-mobile {
min-width: unset;
}
} By looking at the top of the code I am no longer aware that I understand that the window to discuss this change has passed. Fair enough. Although I think it is a bit week argument as I'd suggest that the vast majority of SASS users would not be actively involved with development channels, so it is a bit like in The Hitchhiker’s Guide to the Galaxy:
Anyway, But I'm not here to complain, instead I'd like to suggest to introduce a syntax that keeps the previous way of working. Just like now the warnings can be avoided by explicitly opting for the declaration order output by using Please don't apply this rule in a repeated declaration, but instead hoist it to the top of the selector just like it was in the legacy behaviour. This would not contradict the principle of having any valid CSS compile to identically if copied into an SCSS file, as this syntax would be non-existent in CSS. I'm not a language designer, but it could be similar to So I could still write code with related aspects in one place: .some-element {
color: red;
&.green {
color: green;
}
&.blue {
color: blue;
}
@hoist {
background-color: white;
}
&:hover {
background-color: black;
}
&:disabled {
background-color: gray;
}
@hoist {
min-width: 800px;
}
@include for-tablet {
min-width: 400px;
}
@include for-mobile {
min-width: unset;
}
} without producing repeated selector in the output. The concept needs a bit of thinking through for mixins still, as should the whole mixin be wrapped in EDIT: I've just realised that the warning text says |
I sympathize with you. I know this change feels disruptive, and if I had my way CSS would continue to match Sass's behavior and we wouldn't have to make it. But that's not the world we live in, and we have to make the best of this one. The burden of evidence necessary to add a new language feature is very large, and it's almost never something we do just to save a few bytes in the output (especially since repeated byte sequences are already very amenable to gzip compression). If you're concerned about output size, I recommend looking into the possibility of a post-processing step that combines style rules if it's provably safe to do so. For example, textarea.large.green {
height: 300px;
color: red;
}
textarea.large.green:disabled {
color: gray;
}
textarea.large.green {
color: green;
} is provably identical to textarea.large.green {
height: 300px;
color: green;
}
textarea.large.green:disabled {
color: gray;
} because the second style rule has higher specificity than the third anyway. cssnano might be a good place to look for or request a feature like that.
This isn't "objectively untrue". In your first example, "above the nested rule" means immediately before |
Is it just me or does it feel like CSS is getting dumber in it's process to get "smarter"? I have found Sass/SCSS far superior for many years, and now I feel like Sass is having to dumb itself down because CSS wants to try to reinvent the wheel because they fell way behind in the game and are still eating Sass' dust. |
@nex3 Thank you for your continued support regarding these changes. I really appreciate that you put real thought into the concerns raised, albeit having certain hard limits on what is possible in terms of solutions and/or workarounds. Sure, adding a language feature is not done by a random suggestion in a closed issue. I was just trying to tests the waters if anyone else sees any value in the idea at all. But I can see how a dedicated tool could be better at solving these issues. I assume those probably could also eliminate repeated properties overriding each other, such as Just for fun I'd also note that data transfer size of css is not the only concern, but the time/cpu it takes the browser to match the selectors. I admit I don't know if browsers would first consolidate subsequent identical selectors before starting the matching against the DOM, but the DOM matching procedure can be in some cases a concern. I know that with the powerful hardware we have today it is fine, but Chromium have just introduced a new developer tool to analyse the performance of selector matching, which suggest it is a legit concern for some. I try to believe the code I make uses the least amount of CPU both for speed and for CO2 footprint, so for me it is certainly an interesting aspect. I assume most devs probably don't care at all and their attitude might be the healthier one...
"But is it tho? Is it tho?" :P Imho "a bit tricky, but it's still the best way" and "keep the existing behavior" are still two much different claims. I don't say what you said is not reasonable, and i think all said in this topic is respectable, but I keep up that the statement is untrue and is a bit too generous assuming everyone's usecase will just be fine, all needed is just reshuffle some lines here and there. I have mixins used in mixins used in mixins..., with overrides on multiple levels. I don't know, I might be crazy and my code may be one big bowl of mess, but for me it was superior encapsulation and advanced usage of the available language features. So far SASS was powerful enough to handle it, and now it is not any more. I literally have no way to keep the existing behaviour. Impossible. Hence the statement is untrue. And it downplays the impact of this change, which is somewhat lacking approach in terms of sincerity. Again, I'm not here to fight. I just like to say things as they seem to me. Still, I appreciate your attendance to this issue, and thaks for your suggestion of alternatives. I'll look into them. EDIT: Checked cssnano, it cannot fix it, because it can only merge identical selectors if they are adjacent. Which makes sense because merging them otherwise could be breaking depending on what is in between and how that relates to the DOM. So when SASS outputs these properties in declaration order is basically a lossy transformation and from looking at the output it is no longer possible to determine if the rules are separated for a valid reason or not. So merging them can be an issue. It can only be decided on the source level if the merging is desired and should be done or not. So a Also regarding introducing new language feature: the good news is that the implementation is already there: the current behaviour. So it is not like it'd be something very challenging to implement... I know that's not the key issue, but still. It just feels like a shame to throw away all those logic capable of so much and replace with inferior solution without giving users a way to keep using it... Anyway. I'm going trough the stages of grief. Will stop spamming here eventually at some point.... EDIT2: Although I am willing to put effort into a proposal for |
FWIW, I want to echo @BenceSzalai's concerns. We're running into similar issues on due to this change on a codebase with 25 repositories with about 300 occurrences. We're not quite sure what the correct way forward would be. To give some context, we have a setup where we use Sass mixins like follows (simplified): // block.scss
@use '~sass-mq/mq';
@mixin horizontal-whitespace() {
margin: 1em;
@include mq.mq() {
margin: 0.5em;
}
}
@mixin vertical-whitespace() {
margin: 2em;
@include mq.mq() {
margin: 1em;
}
} .selector {
@include horizontal-whitespace();
@include vertical-whitespace();
margin-top: 1rem;
} The only correct way to handle this, seems to be the following? Is this really the right/best way to do this? .selector {
& {
@include horizontal-whitespace();
}
& {
@include vertical-whitespace();
}
& {
margin-top: 1rem;
}
} |
Which still results in the |
@cascornelissen Here's how I'd recommend writing that for forwards-compatibility: // block.scss
@use '~sass-mq/mq';
@mixin horizontal-whitespace() {
& {margin: 1em}
@include mq.mq() {
margin: 0.5em;
}
}
@mixin vertical-whitespace() {
& {margin: 2em}
@include mq.mq() {
margin: 1em;
}
} @use 'block' as *;
.selector {
margin-top: 1rem;
@include horizontal-whitespace();
@include vertical-whitespace();
} Specifically:
We're not going to add a |
@nex3, thanks so much for the detailed write-up, much appreciated! 🫶🏼 I ended up simplifying the example a bit more so that I could play around with the suggestions in the playground, see this link. The only part that wasn't working with just these changes was the possibility to overwrite whatever was being defined inside the mixin. But, as can be seen in this link, that's still possible with the wrapping of the subsequent rules inside another |
I thought @cascornelissen's code intended the .selector {
@include horizontal-whitespace();
@include vertical-whitespace();
& { margin-top: 1rem; }
} I didn't realize until much reading here and in the CSSWG discussions that the warning was for those who expected/depended on the hoisting behavior. I'm usually putting in my Since I don't need things hoisted, then default CSS nesting should work fine for me and I just need to silence the deprecations, which I've done. Thanks for this discussion thread. Helped a lot. |
Yeah, I think that's why the CSSWG ended up changing the behavior: it looks like @cascornelissen's example would override the margin, but in older CSS specs and the current version of Sass it won't. |
I've found a way to limit the cases in which this warning is emitted to those where the specificity of the style rules may actually overlap. It's out for review in #2342 and should be released in 1.79.0 shortly. |
That's great news, thank you. Given the fact that I have already devised an elaborate system of not triggering this warning, I shall soon be able to simplify things. How can I find out what to simplify, any hints please? |
It's just cases where we can prove that the nested rules have different specificity than the parent rule. |
With 1.77.7 release, my projects have warnings everywhere. I understand this breaking change with nested selectors, but with mixins, this change can be too BREAKING to migrate through a patch.
For example, I write a wrapper mixin and use it in a lot of places:
Now, every usage of this wrapper is throwing warnings unless it's a bare
@include
, because the @include is placed at the top of the page (for overriding), and any declarations after it will trigger warning:I am not satisfied with the dart team about this changes because:
I will suggest the following things:
revert this change with 1.77.8 and release at least a minor (1.78.0)
This is the thing I couldn't stand the most, why should ANY package release any deprecations with patch??????
Hide the warnings for mixins (can provide the behavior opt-in but at least with an option)
There could be too much work for complicated styles with a lot of mixin used. I would suggest the previous behavior, but it seems that there will still be issues with v2.
Edited: Explanations and solutions from the below comments:
Solutions to get rid of the warnings:
If you think your codes are fine with the new behavior, you can bypass this warnings with 2 possible solutions:
For the first one, there is a clear guideline in docs: https://sass-lang.com/documentation/breaking-changes/mixed-decls/
For the second one:
If you are using the API from sass directly:
If you are using webpack, you should also ensure
api
is set tomodern
ormodern-compiler
If you are using Vite, use a custom logger to filter these warnings:
The text was updated successfully, but these errors were encountered: