bootstrap icon indicating copy to clipboard operation
bootstrap copied to clipboard

Drop `node-sass` support

Open julien-deramond opened this issue 1 year ago • 3 comments

[!WARNING]
This PR is heavily draft. It prepares the modifications for when we'll drop node-sass support

Context

Sass has begun deprecating certain features (see https://github.com/twbs/bootstrap/issues/40962 and https://github.com/twbs/bootstrap/issues/40849), which currently generate warning messages during compilation with Sass.

The first major breaking change for us came in Sass 1.79.x, where the blue(), red(), and green() functions were deprecated. These functions were replaced by the color.channel() function, which is not supported by node-sass, and where we can't use any workaround on our side.

As a result, addressing these warnings requires us to drop node-sass support.

Bootstrap v6 development has not yet started, which raises an important consideration: Dart Sass v2 might be released before Bootstrap v6. If that happens, it may become necessary to remove node-sass support in the final Bootstrap v5.x release rather than waiting for v6. This is because Bootstrap v6 will likely introduce a significant set of new features, whereas a v5.x release can focus on resolving compatibility issues. Dropping node-sass in the last v5.x release ensures that all v5 content remains accessible to node-sass users while aligning with Dart Sass's latest updates.

Even if it is clearly a breaking change, for node-sass users, with the right communication, I think this could be an acceptable solution for everyone.

It'll also mean that for everyone, cascading the node-sass removal, Bootstrap users will need to use @use instead of @import, but also @use <color;math;map;etc.>.

Description

This PR drops node-sass support and bump the sass dependency to the latest release.

Following these changes, users will need to use @use instead of @import, and use @use Sass color, math, map, etc. modules.

Sub-tasks

  • [x] Drop dedicated node-sass GitHub workflow
  • [x] Replace RGBA with rgba
  • [x] Bump sass → 1.79.6
    • [x] Deprecation Warning: blue() is deprecated. Suggestion: color.channel($color, "blue", $space: rgb) More info: https://sass-lang.com/d/color-functions (same thing for red() and green()) → 18161453f0bda40e5ddce56ac0bc27b7ee6ecea7
  • [ ] Bump sass → 1.80.x
  • [ ] Bump sass → 1.81.x
  • [ ] Bump sass → 1.82.x
  • [ ] Bump sass → 1.83.x
  • [ ] Revert https://github.com/twbs/bootstrap/pull/41283 and https://github.com/twbs/examples/pull/585
  • [ ] Revert https://github.com/twbs/examples/commit/e9cb7cc03531064e9b43f4c88593b95a8e4bc78c
  • [ ] Check the diff between dist/css/bootstrap.css from the main branch and this one
  • [ ] Check if we need to do something about rfs repository (and vendor dir here)
  • [x] Documentation build locally, with the docs job, and with Netlify
  • [ ] Check the documentation to remove node-sass specific content (if any)
  • [ ] Add somewhere in the documentation that Bootstrap doesn't support node-sass anymore
  • [ ] After the merge of this PR, update the examples repository

Type of changes

  • [x] Breaking change (fix or feature that would change existing functionality)

Checklist

  • [x] I have read the contributing guidelines
  • [x] My code follows the code style of the project (using npm run lint)
  • [ ] My change introduces changes to the documentation
  • [ ] I have updated the documentation accordingly
  • (N/A) I have added tests to cover my changes
  • [x] All new and existing tests passed

Live previews

Related issues

Closes https://github.com/twbs/bootstrap/issues/40962 Closes https://github.com/twbs/bootstrap/issues/40849

julien-deramond avatar Dec 19 '24 07:12 julien-deramond

@julien-deramond Sass color functions don't round particular values in RGB colors (Please check this - https://github.com/sass/dart-sass/issues/2456). You chose to round values in the following functions:

@function to-rgb($value) {
  @return math.round(color.channel($value, "red", $space: rgb)), math.round(color.channel($value, "green", $space: rgb)), math.round(color.channel($value, "blue", $space: rgb));
}
@function luminance($color) {
  $rgb: (
    "r": math.round(color.channel($color, "red", $space: rgb)), // stylelint-disable-line scss/at-function-named-arguments
    "g": math.round(color.channel($color, "green", $space: rgb)), // stylelint-disable-line scss/at-function-named-arguments
    "b": math.round(color.channel($color, "blue", $space: rgb)) // stylelint-disable-line scss/at-function-named-arguments
  );

  @each $name, $value in $rgb {
    $value: if(divide($value, 255) < .04045, divide(divide($value, 255), 12.92), nth($_luminance-list, $value + 1));
    $rgb: map-merge($rgb, ($name: $value));
  }

  @return (map-get($rgb, "r") * .2126) + (map-get($rgb, "g") * .7152) + (map-get($rgb, "b") * .0722);
}

I recommend avoiding math rounds in those functions, as suggested by the Sass creators. It requires modifying the luminance function like this. The math.round is only required in nth()

@function luminance($color) {
  $rgb: (
    "r": color.channel($color, "red", $space: rgb), // stylelint-disable-line scss/at-function-named-arguments
    "g": color.channel($color, "green", $space: rgb), // stylelint-disable-line scss/at-function-named-arguments
    "b": color.channel($color, "blue", $space: rgb) // stylelint-disable-line scss/at-function-named-arguments
  );

  @each $name, $value in $rgb {
    $value: if(divide($value, 255) < .04045, divide(divide($value, 255), 12.92), nth($_luminance-list, math.round($value + 1)));
    $rgb: map-merge($rgb, ($name: $value));
  }

  @return (map-get($rgb, "r") * .2126) + (map-get($rgb, "g") * .7152) + (map-get($rgb, "b") * .0722);
}

mrholek avatar Dec 27 '24 19:12 mrholek

The thing with dart sass and Hugo is that it requires extra steps from each user on their OS to install it.

For example, on my Windows VM, this branch right here won't work. Hence the use of hugo-bin. We should find a solution that works IMHO.

Also, since this is a breaking change, we should make it in a major version bump, which I don't mind if it's just the node-sass removal.

XhmikosR avatar Jan 15 '25 11:01 XhmikosR

@julien-deramond @XhmikosR I just finished the migration in my Bootstrap-based project, and I'm able to generate almost the same CSS as is available now. The only differences are in color: hex colors are now rgb, and the precision of the rgb colors is different than before.

Almost everything works like before. I can use whole library and override variables:

@use "bootstrap" with (
  $accordion-padding-x: 20rem
);

I can use particular components:

@use "alert";

But the problem occurs when I want to import a particular component and override its variables:

@use "alert" with (
  $accordion-padding-x: 20rem
);

Then I have the following error:

Error: This variable was not declared with !default in the @used module.
2 │   $accordion-padding-x: 20rem

What will you do with specific component variables? It's not possible to use certain components with custom variables. The only solution I came up with is to define component-related variables directly within the components. What do you think about it?

mrholek avatar Jan 19 '25 13:01 mrholek

Closing this one. Superseded by https://github.com/twbs/bootstrap/pull/41512.

julien-deramond avatar Jul 05 '25 10:07 julien-deramond