New deprecations in Sass 1.79

mbabker commented 1 month ago

Prerequisites

[X] I have searched for duplicate or closed issues
[X] I have validated any HTML to avoid common problems
[X] I have read the contributing guidelines

Describe the issue

In Sass 1.79, a number of color-related functions are deprecated, to include color.red(), color.green(), and color.blue(), with the suggested replacement being the new color.channel() function.

Reduced test cases

Compile Bootstrap's SCSS with Sass 1.79

What operating system(s) are you seeing the problem on?

macOS

What browser(s) are you seeing the problem on?

Chrome

What version of Bootstrap are you using?

v5.3.3

julien-deramond commented 1 month ago

Hey @mbabker Yeah, saw that today in our Bootstrap fork at work. Let's keep this issue open for when we'll bump the sass dependency in our repo. In the meantime, I'll try to analyze how to fix the issue. Hopefully, it will be possible. Don't hesitate if there's still no PR yet and you have ideas (/cc @twbs/css-review)

In the meantime, folks having these warnings have different options:

pin their sass version waiting for a fix on our side (e.g. npm i sass@1.77.6 --save-exact).
silence the warnings as described here: https://sass-lang.com/documentation/breaking-changes/legacy-js-api/#silencing-warnings

Edit with really quick analysis: Using sass:color won't probably be possible as Bootstrap 5 still needs to support node-sass. Based on that, several scenarios are possible that need to be discussed. Let's try first to check if it would be possible to fix that without using sass:*. If not, we'll need to decide what and how to do next.

klunker commented 1 month ago

Deprecation Warning: The legacy JS API is deprecated and will be removed in Dart Sass 2.0.0. More info: https://sass-lang.com/d/legacy-js-api

Deprecation Warning: red() is deprecated. Suggestion: color.channel($color, red, $space: rgb) More info: https://sass-lang.com/d/color-functions

When we can wait for update?

thet commented 1 month ago

I've stumbled over this issue too and started a PR - however the tests do not yet pass and any help is wanted.

Meanwhile, I will just silence the deprecation warnings as described here: https://sass-lang.com/documentation/breaking-changes/legacy-js-api/#silencing-warnings

ghiscoding commented 1 month ago

Edit with really quick analysis: Using sass:color won't probably be possible as Bootstrap 5 still needs to support node-sass.

I was quite surprised to see that Bootstrap still support node-sass when it's been tagged as EOL and the repo is now archived, even if the EOL did happen this year, it was however announced a long time ago. Will that be removed only in Bootstrap 6? I don't see any mention of that in the BS6 project. I'm also assuming BS6 probably won't come for another couple years, so will node-sass really be supported that long even though it's EOL?

julien-deramond commented 1 month ago

Will that be removed only in Bootstrap 6? I don't see any mention of that in the BS6 project. I'm also assuming BS6 probably won't come for another couple years, so will node-sass really be supported that long even though it's EOL?

That's the question indeed. The original plan was to drop node-sass support in v6 because it's a breaking change for some users. And yes, v6 is clearly not coming anytime soon, as development hasn't really started yet.

I haven't fully thought through all the options, but here are the ones I'm considering:

Fix the issue with a workaround, since we can't currently use sass:color. This could involve re-implementing an equivalent for the deprecated functions. Of course, there's always a risk that future changes in Sass will render node-sass unusable on our side. And clearly, they have perfectly the right to do so, as node-sass is EOL, and announced a long time ago.
Delay the fix until the problem is resolved in Bootstrap (whatever the version). Meanwhile, users would need to either pin their Sass dependency or ignore warning messages. This approach also gives us several options for when to drop node-sass: 2.1. Drop node-sass support in a v5.3.x release. This would be unfair to some users still relying on it, and frankly, I’m not a big fan of this option. 2.2. Drop node-sass support in v5.4. While it would be a breaking change, clear communication could help. However, this also means that any fixes for the v5.3.x series would need to be addressed before removing node-sass. 2.3. Stick with the original plan to drop node-sass in v6. This has always been the long-term goal, but with v6 far off, it's worth considering if we need a more immediate solution.

For now, I’m inclined to try option 1. If that doesn’t work, we'll need to weigh our alternatives carefully.

(/cc @twbs/css-review)

louismaximepiton commented 2 weeks ago

Hey there,

I tried my best finding a workaround with Sass but the only one I found was to introduce some new functions in Bootstrap overriding the ones from Sass itself. The solution is available in #40925. It's only a draft to see the impacts so I put not so much efforts in testing and documentation.

That said, I still don't think it's a good solution to do it this way because:

It's not the duty and goal of Bootstrap to maintain this kind of parsing functions.
It's very probably a source a errors and problems for future us if we introduce this kind of code.
It's an open door to every contribution wanting to add some new color spaces.
What if Sass still deprecate some other functions, I think we won't be able to introduce these functions along the way.

   2.1. **Drop `node-sass` support in a v5.3.x release.** This would be unfair to some users still relying on it, and frankly, I’m not a big fan of this option.

Definitely not a big fan too.

I can't decide if we should rather drop node-sass for 5.4 or for v6.

julien-deramond commented 2 weeks ago

Thanks a lot, @louismaximepiton, for the research that confirms my initial thoughts: it doesn't seem possible to fix this in v5.3.x solely on our side.

My two cents on this.

I don't think it makes sense to release a v6 just to remove Node Sass. If that's part of v6, we'll be waiting a long time for the fix. Plus, there's no guarantee Sass won't release a v2 in the meantime, leaving us stuck on v1.78 for ages, just to support something that's been deprecated for quite a while.

I understand that removing Node Sass would be a breaking change, even in a minor version, so we'd need to finalize all pending patches—especially those related to color modes—within v5.3.x. After that, we could roll out a v5.4, merging everything that's ready, including dropping Node Sass support. Then, I imagine the next focus would shift to v6.

This way, Node Sass users wouldn’t lose significant features and still benefit from the v5 entirely, as they'd remain on v5.3.x instead of moving to v5.4, which likely wouldn't include any critical changes anyway, or big new features or improvements.

Thoughts @twbs/css-review, @mdo, folks, Node Sass users?

ghiscoding commented 2 weeks ago

@louismaximepiton @julien-deramond It might actually be possible to create a polyfill by creating a custom method that will check if the SASS function you're calling exists (color.channel()) if so use it or else use an older equivalent SASS function (lighten()). That is exactly what Angular-Material did in this old PR fix warnings related to division operator, so it might actually be possible to try this out (I didn't have time to create a new PR and I don't have node-sass, neither want to install, to try this out though, so...

Below was the Angular-Material polyfill for the old SASS math.div() warning, the project no longer has this code in their codebase since they released new major release bumping minimum SASS version but when this polyfill was implemented, nobody complained of having issues with it (they surely have thousands of users), so it's worth giving it try!?

@use 'sass:math';
@use 'sass:meta';
@use 'sass:list';

// Private polyfill for the `math.div` function from Sass to be used until we can update the
// minimum required Sass version to 1.34.0 or above.
// TODO(crisbeto): replace with `math.div` eventually.
@function private-div($a, $b) {
  @if (meta.function-exists('div', 'math')) {
    @return math.div($a, $b);
  }
  @else {
    @return $a / $b;
  }
}

// Private polyfill for the `list.slash` function from Sass to be used until we can update the
// minimum required Sass version to 1.34.0 or above.
// TODO(crisbeto): replace with `list.slash` eventually.
@function private-slash($a, $b) {
  @if (meta.function-exists('slash', 'list')) {
    @return list.slash($a, $b);
  }
  @else {
    @return #{$a}#{' / '}#{$b};
  }
}

Side note, I also just checked if node-sass actually has the meta.function-exists and they do here), so I'm assuming a similar polyfill for the new SASS function is probably possible!

louismaximepiton commented 2 weeks ago

Hello, thanks for your contribution to the topic.

We've been trying this feature and it sounds like an unsolvable issue. We are only human, so maybe we've been missing something in here.

Here are all our findings: All the @use "sass:**"; get in the generated CSS when it's generated via node-sass, not blocking but not very clean. Know that conditional @import or @use aren't possible. It seems like we can't use meta.function-exists because it's not supported by node-sass so we need to use function-exists which only take one parameter so we can't test for functions inside modules (or we don't know the syntax). We couldn't find any function in Dart Sass not inside a Dart Sass module and not in Node Sass.

For Angular it worked because they were likely supporting 2 different versions of Dart Sass which is different and unfortunately not applicable to our issue.

ghiscoding commented 2 weeks ago

@louismaximepiton well ahh at least you tried :) So in that case, I guess that v5.4.x would be a nice time to drop node-sass support then, I'm all in favor of that (though I guess it means that a fix is still a couple months away?). A big thanks for giving it a try.

karolyi commented 1 week ago

for webpack-dev-server, you can ignore warnings with the following configuration:

  const devServerOptions = {
    hot: true,
    // host: '0.0.0.0',
    host: 'localhost',
    port: 3000,
    compress: true,
    historyApiFallback: true,
    allowedHosts: 'all',
    client: {
      overlay: {
        errors: true,
        runtimeErrors: true,
        warnings: false
      },
      progress: true,
    },
    headers: {
      'Access-Control-Allow-Origin': '*',
    },
    static: {
      directory: path.join(_myDirName, 'frontend', 'src', 'assets'),
    },
  }

the important parts are in the client->overlay section. this way, webpack will not flood your dev site with modals.

mbabker commented 1 week ago

In good news, dart-saas 1.80 also has options to silence these new deprecations, see https://sass-lang.com/blog/import-is-deprecated/#controlling-deprecation-warnings for more on that.

twbs / bootstrap