Skip to content
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

Sass deprecation warning for slash as division #2238

Open
colinrotherham opened this issue May 28, 2021 · 4 comments
Open

Sass deprecation warning for slash as division #2238

colinrotherham opened this issue May 28, 2021 · 4 comments
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation) sass / css tooling
Milestone

Comments

@colinrotherham
Copy link
Contributor

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

@colinrotherham colinrotherham added awaiting triage Needs triaging by team 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) labels May 28, 2021
@36degrees
Copy link
Contributor

36degrees commented Jun 1, 2021

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: twbs/bootstrap#34051

@36degrees
Copy link
Contributor

@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
Copy link
Contributor Author

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

Cheers for spotting that flag though!

tombye added a commit to alphagov/emergency-alerts-govuk that referenced this issue Jul 15, 2021
Brings in major version bumps to:

- @rollup/plugin-commonjs
- @rollup/plugin-node-resolve
- govuk-frontend
- gulp-sass
- jest

Of those, the following require changes to our
code.

Gulp-sass

The gulp-sass API has changed, requiring you to
now set the compiler used in a different way.

The compiler we use, dart-sass, used the
node-fibers module to speed up rendering. Node
itself contains changes from version 16 and up
that mean it can no longer be used.

See this for more detail on node fibers'
deprecation: https://sass-lang.com/blog/node-fibers-discontinued

See this for more detail on the changes needed for
gulp-sass version 5: https://github.com/dlmanning/gulp-sass/tree/master#migrating-to-version-5

Sass

This pins the sass module to v1.32.13 because
versions after that produce warnings from the
GOVUK Frontend Sass code that it looks like won't
be changed for a while (see
alphagov/govuk-frontend#2238).

Jest

Version 27.0.0 of Jest changed the default test
environment from JSDOM (a JS module that simulates
the Document Object Model - DOM) to NodeJS.

Our tests rely on the environment being JSDOM so
we now need to set this in the config file.

See this for more detail on the switch to NodeJS:
jestjs/jest#9874
daviesjamie added a commit to ministryofjustice/bichard7-next-user-service that referenced this issue Jul 22, 2021
The GOV.UK Frontend Design System uses `/` for division in its SASS
stylesheets. Sass is deprecating the use of the `/` operator, in
preference of the `math.div()` function.

There is an open issue about this in the GOV.UK Frontend repo:

alphagov/govuk-frontend#2238

In the meantime, passing the `--quiet-deps` to Sass in the user-service
app means we won't get flooded with warnings when running/building the
app.
@36degrees
Copy link
Contributor

36degrees commented Oct 5, 2021

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 twbs/bootstrap#34245 and twbs/bootstrap#34386

36degrees added a commit to alphagov/govuk-frontend-docs that referenced this issue Oct 5, 2021
We can't currently make the changes in GOV.UK Frontend to resolve the deprecation warnings without also removing support for compilers other than Dart Sass.

However, since Dart Sass 1.35 it's possible to silence deprecation warnings coming from dependencies.

Document how to do this, pointing to the relevant parts of the Sass docs for more information.

See alphagov/govuk-frontend#2238 for more details.
36degrees added a commit to alphagov/govuk-frontend-docs that referenced this issue Oct 14, 2021
We can't currently make the changes in GOV.UK Frontend to resolve the deprecation warnings without also removing support for compilers other than Dart Sass.

However, since Dart Sass 1.35 it's possible to silence deprecation warnings coming from dependencies.

Document how to do this, pointing to the relevant parts of the Sass docs for more information.

See alphagov/govuk-frontend#2238 for more details.

Co-authored-by: EoinShaughnessy <72507742+EoinShaughnessy@users.noreply.github.com>
EoinShaughnessy added a commit to alphagov/govuk-frontend-docs that referenced this issue Oct 18, 2021
We can't currently make the changes in GOV.UK Frontend to resolve the deprecation warnings without also removing support for compilers other than Dart Sass.

However, since Dart Sass 1.35 it's possible to silence deprecation warnings coming from dependencies.

Document how to do this, pointing to the relevant parts of the Sass docs for more information.

See alphagov/govuk-frontend#2238 for more details.

Co-authored-by: EoinShaughnessy <72507742+EoinShaughnessy@users.noreply.github.com>
ntsim added a commit to dfe-analytical-services/explore-education-statistics that referenced this issue Dec 6, 2021
This change removes `node-sass` in favour of `sass` (the Dart
implementation) so that we should no longer get node-gyp errors in the
build pipeline.

Note that we have pinned `sass` at 1.32.13, instead of the latest
version due to `govuk-frontend` using deprecated features. This was
unfortunately causing too much unnecessary noise in the frontend dev
server logs. This should hopefully be resolved by `govuk-frontend` at
some point the future. See: alphagov/govuk-frontend#2238
ntsim added a commit to dfe-analytical-services/explore-education-statistics that referenced this issue Dec 6, 2021
This change removes `node-sass` in favour of `sass` (the Dart
implementation) so that we should no longer get node-gyp errors in the
build pipeline.

Note that we have pinned `sass` at 1.32.13, instead of the latest
version due to `govuk-frontend` using deprecated features. This was
unfortunately causing too much unnecessary noise in the frontend dev
server logs. This should hopefully be resolved by `govuk-frontend` at
some point the future. See: alphagov/govuk-frontend#2238
DanCorder added a commit to DanCorderSoftwire/govuk-design-system-dotnet that referenced this issue Jun 3, 2022
huwd added a commit to alphagov/di-solid-prototype that referenced this issue Aug 1, 2022
Government Frontend is currently supporting LibreSass and Ruby Sass
alongside Dart Sass. math.div is not available in either. So the
deprcation warning / preference does not have a clear mitigation path
until the design system is able to remove these conflicting support
requriements (aimed at October 2022, or so I hear).
Source: alphagov/govuk-frontend#2238

In the meantime, we get a ton of noise everytime we spin up an app.

Theoretically Dart Sass (which we're using here) can silence these
errors since v1.34.0:
https://github.com/sass/dart-sass/releases/tag/1.34.0

Other folks who have encountered this have even left handy PRs we can
:eye:
DanCorderSoftwire/govuk-design-system-dotnet@eb12d75#diff-7bca01eb707ebcc19f259555d952675647510b2155a9a313960a8470ea0f85a6R7

However my attempts to reproduce here have not resulted in any success.
Noting the Dart docs
(https://sass-lang.com/documentation/cli/dart-sass#quiet-deps) I
expected any of the following to have an impact:

- `--load-path=node_modules --quiet-deps`
- `--load-path=node_modules/govuk-frontend --quiet-deps`

But alas no joy. I even tried shuffling them around after the `sass`
command wondering if there was something funky with flag order
precidence.

`--quiet` does change things however.

Not ideal, as I would like to know if we have errors, just not those
inherited from our deps...

On the other hand, if we did have an error right now, not sure we'd see
it through all the noise.

So... The eternal sunshine of the spotless server log? Or overkill?
Or... what's wrong with the real solution anyway?

Thought we could discuss on this PR
Gweaton added a commit to UKGovernmentBEIS/beis-report-official-development-assistance that referenced this issue Jan 3, 2023
This was done automatically using `./bin/rails css:install:sass`

I've added `--quiet-deps` here, as it suppresses deprecation warnings
for the GOV UK frontend library, as is recommended here:

alphagov/govuk-frontend#2238
Gweaton added a commit to UKGovernmentBEIS/beis-report-official-development-assistance that referenced this issue Jan 5, 2023
This was done automatically using `./bin/rails css:install:sass`

I've added `--quiet-deps` here, as it suppresses deprecation warnings
for the GOV UK frontend library, as is recommended here:

alphagov/govuk-frontend#2238
Gweaton added a commit to UKGovernmentBEIS/beis-report-official-development-assistance that referenced this issue Jan 5, 2023
This was done automatically using `./bin/rails css:install:sass`

I've added `--quiet-deps` here, as it suppresses deprecation warnings
for the GOV UK frontend library, as is recommended here:

alphagov/govuk-frontend#2238
Gweaton added a commit to UKGovernmentBEIS/beis-report-official-development-assistance that referenced this issue Jan 5, 2023
This was done automatically using `./bin/rails css:install:sass`

I've added `--quiet-deps` here, as it suppresses deprecation warnings
for the GOV UK frontend library, as is recommended here:

alphagov/govuk-frontend#2238
edavey added a commit to dxw/dfsseta-apply-for-landing-ruby that referenced this issue Mar 15, 2023
As per BEIS RODA:

> This was done automatically using `./bin/rails css:install:sass`
>
> I've added `--quiet-deps` here, as it suppresses deprecation warnings
for the GOV UK frontend library, as is recommended here:
>
> alphagov/govuk-frontend#2238
edavey added a commit to dxw/dfsseta-apply-for-landing-ruby that referenced this issue Mar 15, 2023
As per BEIS RODA:

> This was done automatically using `./bin/rails css:install:sass`
>
> I've added `--quiet-deps` here, as it suppresses deprecation warnings
for the GOV UK frontend library, as is recommended here:
>
> alphagov/govuk-frontend#2238
@36degrees 36degrees added this to the v6.0 milestone Apr 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation) sass / css tooling
Projects
None yet
Development

No branches or pull requests

3 participants