Replace node-sass with Dart Sass #1269
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-1269
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-1269
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-1269
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-1269
govuk-design-system-ci
requested a deployment
to
govuk-prototype-kit-pr-1269
govuk-design-system-ci
requested a deployment
to
govuk-prototype-kit-pr-1269
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-1269
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-1269
|
not sure about showing warnings for Frontend when our users can't do anything about them, and the custom Logger adds a lot of code as opposed to using the documented Dart Sass silent option |
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-1269
lfdebrux
reviewed
Mar 31, 2022
nataliecarey
force-pushed
the
replace-node-sass
branch
from
9e49433
to
02c43f0
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-1269
joelanman
changed the title
lfdebrux
force-pushed
the
replace-node-sass
branch
from
02c43f0
to
58a932e
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-1269
lfdebrux
force-pushed
the
replace-node-sass
branch
from
58a932e
to
2b9d8a3
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-1269
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-1269
lfdebrux
reviewed
Mar 31, 2022
__tests__/spec/sanity-checks.js
Outdated
Comment on lines
146
to
147
| console.error('Found an error') | ||
| console.error(err) |
There was a problem hiding this comment.
We can probably get rid of these lines right?
When I make the test fail by breaking the CSS I get a nicely formatted stack trace in the error output.
|
Is it worth adding a comment to the tests saying 'we don't know why this works, but it's the only thing we could find that did'? |
|
@lfdebrux Re: comment I'd rather leave it without that comment, at some point we'll either use the |
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-1269
nataliecarey
force-pushed
the
replace-node-sass
branch
from
153af35
to
6318c7b
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-1269
joelanman
reviewed
Mar 31, 2022
gulp/sass.js
Outdated
| @@ -23,7 +23,7 @@ gulp.task('sass-extensions', function (done) { | |||
|
|
|||
| gulp.task('sass', function () { | |||
| return gulp.src(config.paths.assets + '/sass/*.scss', { sourcemaps: true }) | |||
| .pipe(sass({ outputStyle: 'expanded' }).on('error', function (error) { | |||
| .pipe(sass.sync({ outputStyle: 'expanded', logger: sass.compiler.Logger.silent }, false).on('error', function (error) { | |||
There was a problem hiding this comment.
just checking what the false here is for? The other calls on this page don't have it
nataliecarey
force-pushed
the
replace-node-sass
branch
from
6318c7b
to
4d83f6d
Compare
govuk-design-system-ci
temporarily deployed
to
govuk-prototype-kit-pr-1269
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #1000
This replaces node-sass with Dart Sass. Node-sass is deprecated and does not officially support M1 Macs. Dart sass works on M1 Macs. Other than fixing the Prototype Kit on M1 Macs, there should be no change for users.
This PR also replaces division symbol
/in Prototype Kit sass files withmath.divas/is deprecated in Dart Sass.