PLATUI-3157 switch to dart-sass but add test to make sure we can still compile our sass with node-sass #395
Conversation
| @@ -10,19 +10,19 @@ const configPaths = require('../../config/paths.json'); | |||
| const destinationPath = require('./destination-path'); | |||
| const pkg = require('../../package.json'); | |||
|
|
|||
| const errorHandler = (error) => { | |||
| function errorHandler(error) { | |||
There was a problem hiding this comment.
changed this because when it's an arrow function this.once/this.emit won't work (so we get an error in our error handler) const it's capturing the this to be this package I think - where as we want it to be kind of floating and refer to the context it's run in which will be some EventEmitter type thing that has a this.once https://nodejs.org/docs/latest/api/events.html
the iffy thing is that when using an arrow function the whole build blows up straight away when there is any error with css compiling - but without it, it continues on and only fails at the end - might be some tech debt here to generally tidy stuff up in case the first behaviour is what we want
| console.error(e.messageFormatted || e); | ||
| throw e; | ||
| } finally { | ||
| await removeSymLinksIfPresent(); |
There was a problem hiding this comment.
feels like we can probably tidy these tests up a bit t o make them clearer but for now I think it's ok to just duplicate stuff and have that come within some wider tech debt task to tidy up hmrc-frontend build & test config
oscarduignan
changed the title
oscarduignan
changed the title
oscarduignan
changed the title
package.json
Outdated
| "govuk-frontend": "^5.4.1", | ||
| "accessible-autocomplete": "^2.0.4" | ||
| "accessible-autocomplete": "^2.0.4", | ||
| "govuk-frontend": "^5.4.1" |
There was a problem hiding this comment.
I don't know why npm always likes to swap these around in order when I install new stuff
oscarduignan
force-pushed
the
PLATUI-3157
branch
2 times, most recently
from
a3301f7 to
e7ad10f
Compare
|
|
||
| gulp.task('scss:compile-all-govuk-and-hmrc', () => gulp.src(`${configPaths.src}all-govuk-and-hmrc.scss`) | ||
| .pipe(plumber(errorHandler)) | ||
| .pipe(sourcemaps.init()) | ||
| .pipe(sass({ | ||
| includePaths: ['node_modules'], | ||
| includePaths: ['node_modules', 'node_modules/govuk-frontend/dist', 'node_modules/govuk-frontend/dist/govuk/components'], |
There was a problem hiding this comment.
we should add a todo comment in the code explaining why we're adding these extra paths
oscarduignan
force-pushed
the
PLATUI-3157
branch
from
e7ad10f to
e1f77dc
Compare
still maintaining compat with node-sass (libsass)
oscarduignan
force-pushed
the
PLATUI-3157
branch
from
e1f77dc to
1688450
Compare
There was a problem hiding this comment.
the difference is a slight 1px movement of the icon next to account menu - I couldn't recreate this difference in my browser - it is most likely due to gulp-sass rounding to 5 decimal places and dart-sass rounding to 10
| @@ -40,7 +41,8 @@ gulp.task('scss:compile-accessible-autocomplete', () => gulp.src(`${configPaths. | |||
| .pipe(plumber(errorHandler)) | |||
| .pipe(sourcemaps.init()) | |||
| .pipe(sass({ | |||
| includePaths: ['node_modules'], | |||
| includePaths: ['node_modules', 'node_modules/govuk-frontend/dist', 'node_modules/govuk-frontend/dist/govuk/components'], | |||
| quietDeps: true, | |||
There was a problem hiding this comment.
if we don't have quietDeps then we get a load of deprecation warnings from stuff imported from govuk-frontend (I think) https://design-system.service.mod.gov.uk/get-started/silence-deprecation-warnings-in-dart-sass/ think with this we should still get warned about deprecations in our sass, just not in stuff imported from node_modules
migrated to dart-sass without having to change any of the import paths in the sass, so will still compile for people still using libsass (so hmrc teams using sbt-sassify) and also for people who move to dart-sass