sass / dart-sass

The reference implementation of Sass, written in Dart.
https://sass-lang.com/dart-sass
MIT License
3.9k stars 352 forks source link

Update to mimic new CSS nesting appears to break use of @include #2292

Closed Kurohyou closed 1 month ago

Kurohyou commented 1 month ago

Started up a project today using SASS 1.77.8.

Got a host of deprecation warnings that my scss was going to become nonfunctional when scss updates to mimic the css nesting behavior.

However, the sections of code that are triggering this deprecation warning are using @include to include code from other mixins. I have typically put all of my includes at the top of a given style declaration like so:

@mixin baseHeader{
  @include baseText;
  color:var(--fontColor);
  display: block;
  white-space: nowrap;
  margin-top: 0px;
  margin-bottom: 0px;
  font-weight:normal;
}

However, according to the deprecation warnings, because the mixin that is included contains other nested scss, this will break when the update occurs.

It seems to me that this change will also break most uses of include. Previously, you could include your mixins anywhere and they would work. Now you will have to consider whether the mixin has any nested scss in it to place it accordingly, however this doesn't guarantee that it will work. Consider what happens if you have several mixins that each have direct style declarations and nested styling:

@mixin nested-1{
  font-weight: bold;
  > *{
    font-style: italics;
  }
}
@mixin nested-2{
  font-size: 120%;
  > *{
    text-decoration: underline;
  }
}
.final-use{
  border: 1px solid black;
  @include nested-1;
  @include nested-2;
}

According to the deprecation warnings, this will no longer work because it parses out to:

.final-use{
  border: 1px solid black;
  font-weight: bold;
  > *{
    font-style: italics;
  }
  font-size: 120%;
  > *{
    text-decoration: underline;
  }
}

This essentially means that if you are using mixins, you now need to wrap everything in the mixin in&{} so that you can guarantee it will work in any and all situations in which the mixin might be called.

nex3 commented 1 month ago

This essentially means that if you are using mixins, you now need to wrap everything in the mixin in&{} so that you can guarantee it will work in any and all situations in which the mixin might be called.

Yes, this is accurate. There's unfortunately no other way to ensure that your stylesheet is forwards-compatible with the new CSS semantics, which we have to support for CSS compatibility. I understand that this requires a considerable amount of change for mixins, but our hands are pretty much tied here. We absolutely cannot break CSS compatibility, and given that there's going to be a potentially-breaking change the only responsible course of action is to warn people that it's coming.