alphagov / govuk-frontend

GOV.UK Frontend contains the code you need to start building a user interface for government platforms and services.
https://frontend.design-system.service.gov.uk/
MIT License
1.16k stars 319 forks source link

Sass deprecation warning for slash as division #2238

Open colinrotherham opened 3 years ago

colinrotherham commented 3 years ago

Hello 👋

It's been a while since I was on govuk-frontend issues 😆

Description of the issue

We've just noticed Sass deprecation warnings popping up for latest sass@1.34.0: https://sass-lang.com/d/slash-div

DEPRECATION WARNING: Using / for division is deprecated and will be removed in Dart Sass 2.0.0.

Recommendation: math.div($button-shadow-size, 2)

More info and automated migrator: https://sass-lang.com/d/slash-div

   â•·
35 │     padding: (govuk-spacing(2) - $govuk-border-width-form-element) govuk-spacing(2) (govuk-spacing(2) - $govuk-border-width-form-element - ($button-shadow-size / 2)); // s1
   │                                                                                                                                             ^^^^^^^^^^^^^^^^^^^^^^^
   ╵
    node_modules/govuk-frontend/govuk/components/button/_index.scss 35:141  @content
    node_modules/govuk-frontend/govuk/tools/_exports.scss 29:5              govuk-exports()
    node_modules/govuk-frontend/govuk/components/button/_index.scss 1:1     @import
    node_modules/govuk-frontend/govuk/components/_all.scss 6:9              @import
    node_modules/govuk-frontend/govuk/all.scss 6:9                          @import

With the "fix" being a breaking change for older versions of Sass, we might need to look into running the Sass migrator before we import any *.scss files: https://github.com/sass/migrator#readme

Steps to reproduce the issue

Importing any GOV.UK Frontend *.scss files using / for division with sass@1.33.0 and up.

See release notes: https://github.com/sass/dart-sass/releases/tag/1.33.0

36degrees commented 3 years ago

Hey @colinrotherham, long time no see! Thanks for raising this.

This is a tricky one, as we currently support LibSass and Ruby Sass alongside Dart Sass, and math.div (as part of the wider module system) is not available in either. As such, I can't see any way to update the code to work with Dart Sass 2.0.0 (and to silence the deprecation warnings) without also removing support for compilers other than Dart Sass.

Ruby Sass has been deprecated for a while and I don't believe sees much use nowadays, but we wouldn't want to break support for LibSass without giving plenty of notice, and we'd need to treat it as a breaking change.

We've previously been aiming for the October 2022 date to move onto the module system (and drop support for LibSass and Ruby Sass in the process) but if this sort of thing keeps happening we may have to bring that forward… I don't suppose anyone's been able to find any sort of timeline for Dart Sass 2.0.0's release?

Related, we should probably migrate to using Dart Sass as our 'primary' compiler so that we spot these sorts of things a bit earlier. I'll create an issue for that.

There's also a relevant discussion over on Twitter Bootstrap that's worth a read: https://github.com/twbs/bootstrap/issues/34051

36degrees commented 3 years ago

@lfdebrux has pointed out that Dart Sass 1.34.0 makes it possible to silence warnings from dependent stylesheets with a new --quiet-deps flag, which may help in the short to medium term.

colinrotherham commented 3 years ago

Thanks @36degrees, yeah I couldn't think of a non-breaking fix either 😬

Cheers for spotting that flag though!

36degrees commented 2 years ago

Worth noting that Bootstrap have ended up introducing their own custom divide Sass function (and replacing e.g. / 2 with * 0.5 where appropriate) to avoid this issue – see https://github.com/twbs/bootstrap/pull/34245 and https://github.com/twbs/bootstrap/pull/34386