-
Notifications
You must be signed in to change notification settings - Fork 331
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
Migrate GOV.UK Frontend build pipeline to use Dart Sass #3164
Conversation
b05d11f
to
8a6c2e3
Compare
8a6c2e3
to
740ed0f
Compare
740ed0f
to
f38df2e
Compare
I'm not seeing changes to Also wondering what we want to do about all the deprecation notices… it's potentially useful to be aware of new ones, but there are ones we can't fix that create quite a lot of noise in the terminal output… 😬 |
@36degrees Update 1: I've kept the source import
For deprecations we can silence the log output for tests and/or development quite easily Update 2: Found nice fixes for all our deprecation warnings in: |
f38df2e
to
ccd1395
Compare
ccd1395
to
46b9159
Compare
13e3be4
to
d5903b3
Compare
46b9159
to
0ec9e7a
Compare
0ec9e7a
to
6094d28
Compare
Diff for
|
42cd467
to
8686050
Compare
8686050
to
360cd32
Compare
Codes changes look good to me. 🙌🏻 Regarding the underline, it's be neat to have a fix if adjusting |
360cd32
to
d5769e8
Compare
As per our discussion offline: we're going to merge this as it only affects dist and folks using dart-sass are already seeing these changes. Additionally, we'll make sure we tackle #2743 soon. |
Dart Sass uses a higher 10dp precision
Includes minor whitespace (and rule ordering) changes unique to Dart Sass as shown in tests
Exactly matches log output from Dart Sass https://github.com/sass/dart-sass/blob/main/lib/src/logger/stderr.dart Also includes compiler deprecation warnings
d5769e8
to
c1c4c13
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good to me
Use the new `silenceDeprecations` feature introduced in Sass 1.74.0 and remove the custom logger that was implemented to filter them out. The logger was introduced in 81f59d9 and ec547be as part of #3164. As mentioned in the first commit message, the output otherwise exactly matches the log output from Dart Sass. I’ve also manually verified this by comparing the output from the custom logger with an empty array for `supressed` and using the default logged with an empty array for `silenceDeprecations`. There’s nothing else using `chalk` which was introduced as a dependency in 81f59d9 so uninstall that too.
This PR swaps "Node Sass"
node-sass
with "Dart Sass"sass
sass-embedded
There are some minor output differences to be aware of:
@media
queries for expanded style1.42857
1.4285714286
to_upper_case
to-upper-case
Other changes in preparation have been moved to:
expanded
output style #3193Plus this swap has previously been looked at in an older PR:
Closes: #2239