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

Upgrade Sass to 1.80.6 (and fix as many deprecations as possible) #1064

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

frankieroberto
Copy link
Contributor

This upgrades Sass from 1.77.6 to 1.80.6.

The new version introduces a lot more deprecation warnings ahead of Sass 2.0.0 – so I’ve tried to fix as many as possible.

A lot of the warnings are from the vendored Sass-mq however. I've not touched that at all as it feels a bit icky to edit a vendor file (although it has had some code style tweaks over the years).

The GOV.UK Design System team are considering removing the sass-mq dependency altogether. However there was a 6.0.0 release of it last week which should resolve the deprecations.

The other remaining deprecation warnings are about use of @import - however moving away from is bigger job (changing every instance of @import to @use and namespacing all the imported functions).

Checklist

`darken()` has been deprecated in the new version of sass due to support for new colour spaces - see https://sass-lang.com/documentation/breaking-changes/color-functions/

It seems however that we already had defined colours for the 3 button hover background colours using `shade()` but which were not being used. This switches to using them. There are some slight differences however, so we should check the new colours.

Also change to use the namedspaces functions to avoid warnings about global functions beind deprecated.
@paulrobertlloyd
Copy link
Contributor

paulrobertlloyd commented Nov 10, 2024

The other remaining deprecation warnings are about use of @import - however moving away from is bigger job (changing every instance of @import to @use and namespacing all the imported functions).

From the Sass docs:

You can even load a module without a namespace by writing @use "<url>" as *.

In my limited experience trying to move over to using @use, this means you can do the following:

- @import 'core.scss'
+ @use 'core.scss' as *

This then means you don’t need to use namespaced variables. However, the Sass documentation goes on to say:

We recommend you only do this for stylesheets written by you, though; otherwise, they may introduce new members that cause name conflicts!

Which is to say this may be a short term fix, but longer term, a breaking change could introduce an nhsuk namespace for all mixins, functions and variables owned by nhsuk-frontend.

@frankieroberto
Copy link
Contributor Author

@paulrobertlloyd ooh nice, I hadn’t spotted that. It certainly could be a short-term option, and then we could gradually switch to namespaces, starting with the most-reused functions and mixins and so on?

@paulrobertlloyd
Copy link
Contributor

paulrobertlloyd commented Nov 12, 2024

Trying to resolve these deprecation warnings in my prototype, one thing with the above approach (@use "<url>" as *) is that it seemingly causes conflicts if 2 SCSS files export a variable, mixin or function with the same name. As this project shares some names with GOV.UK Frontend, I suspect there’d be clashes if you were to import anything from GOV.UK Frontend.

This module system seems nice ’un all, but get the feeling it’s an all or nothing kinda of affair. 😕 Definitely warrants more investigation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants