Migrate to dart sass

[joelanman]

What

Switch to using the Dart Sass compiler to compile the Sass in the prototype kit, rather than Node Sass.

Why

LibSass and Node Sass, which is built on top of LibSass, are both deprecated:

We recommend all existing LibSass users make plans to eventually move onto Dart Sass, and that all Sass libraries make plans to eventually drop support for LibSass.

We’re no longer planning to add any new features to LibSass, including compatibility with new CSS features.

LibSass and Node Sass will continue to be maintained indefinitely on a best-effort basis, including fixing major bugs and security issues and maintaining compatibility with the latest Node versions.

There are also currently known vulnerabilities in Node Sass.

Who needs to know about this

Joe

Further detail

Timebox - 2 hours

Done when

• Switch to Sass
• Check it doesn't break anything

[kellylee-gds]

Timebox to couple hours to do the switch and monitor it.

[36degrees]

I've got the prototype kit compiling using dart sass, but it is significantly slower. On the master branch, with node-sass, compilation takes around a second:

govuk-prototype-kit on  master [$] is 📦 v9.10.1 via ⬢ v12.13.1 took 4s 
➜ npx gulp sass      
[09:14:07] Using gulpfile ~/Code/govuk-prototype-kit/gulpfile.js
[09:14:07] Starting 'sass'...
[09:14:08] Finished 'sass' after 1.18 s

govuk-prototype-kit on  master [$] is 📦 v9.10.1 via ⬢ v12.13.1 
➜ npx gulp sass
[09:14:12] Using gulpfile ~/Code/govuk-prototype-kit/gulpfile.js
[09:14:12] Starting 'sass'...
[09:14:13] Finished 'sass' after 1.07 s

govuk-prototype-kit on  master [$] is 📦 v9.10.1 via ⬢ v12.13.1 
➜ npx gulp sass
[09:14:15] Using gulpfile ~/Code/govuk-prototype-kit/gulpfile.js
[09:14:15] Starting 'sass'...
[09:14:16] Finished 'sass' after 1.07 s

With dart-sass, it's more like 7-8s:

govuk-prototype-kit on  switch-dart-sass [$] is 📦 v9.10.1 via ⬢ v12.13.1 took 10s 
➜ npx gulp sass
[09:11:39] Using gulpfile ~/Code/govuk-prototype-kit/gulpfile.js
[09:11:39] Starting 'sass'...
[09:11:46] Finished 'sass' after 6.98 s

govuk-prototype-kit on  switch-dart-sass [$] is 📦 v9.10.1 via ⬢ v12.13.1 took 8s 
➜ npx gulp sass
[09:11:49] Using gulpfile ~/Code/govuk-prototype-kit/gulpfile.js
[09:11:49] Starting 'sass'...
[09:11:57] Finished 'sass' after 7.46 s

govuk-prototype-kit on  switch-dart-sass [$] is 📦 v9.10.1 via ⬢ v12.13.1 took 9s 
➜ npx gulp sass
[09:12:31] Using gulpfile ~/Code/govuk-prototype-kit/gulpfile.js
[09:12:31] Starting 'sass'...
[09:12:39] Finished 'sass' after 7.83 s

I'm not sure we can accept this change with that sort of performance penalty. Making changes to the styles in a prototype would go from near-enough instantaneous to having a significant delay – long enough I think to be frustrating, and possibly to cause confusion (e.g. users being unsure whether their change has 'worked').

There are also currently known vulnerabilities in Node Sass.

As far as I can tell at least some of the high vulnerabilities are incorrectly listed by Snyk as affecting all versions, when in fact they have been fixed:

SNYK-JS-NODESASS-540974
According to NVD the linked CVE-2018-11695 this only affects LibSass up to and including libsass 3.5.2, so this vulnerability should not affect node-sass 4.9.0+ which uses libsass 3.5.4.

SNYK-JS-NODESASS-540956
According to NVD the linked CVE-2018-11697 affects up to and including libsass 3.5.4, so this vulnerability should not affect node-sass 4.11.0+ which uses libsass 3.5.5.

SNYK-JS-NODESASS-540996
According to NVD the linked CVE-2018-11698 affects up to and including libsass 3.5.4, so this vulnerability should not affect node-sass 4.11.0+ which uses libsass 3.5.5.

I've contacted Snyk to flag this for their attention.

Some of the other valid vulnerabilities listed appear to fixed in libsass 3.6, which should land in node-sass 5.0 – a pre-release for which came out 2 days ago.

More broadly, generally Sass vulnerabilities rely on a malicious user being able to edit the Sass to cause the compiler to do unexpected things. If a malicious user is able to modify the Sass in the prototype then it's highly likely they can edit other code in the prototype as well - in which case they could already cause damage, without needing a vulnerability in the Sass compiler to exploit.

I think we should remain on node-sass until the work on the Sass embedded protocol lands (or something else changes which gives dart-sass close enough performance to that of node-sass), updating to node-sass 5.0 as soon as it's generally available.

[joelanman]

great investigation @36degrees thanks!

[36degrees]

I thought I'd been testing dart-sass with Fibers as recommended when using dart-sass but I'd stashed those changes 🤦🏻

However, even with Fibers compilation still takes 5s or more:

govuk-prototype-kit on  switch-dart-sass [$!] is 📦 v9.10.1 via ⬢ v12.13.1 took 5s 
➜ npx gulp sass                            
[09:51:09] Using gulpfile ~/Code/govuk-prototype-kit/gulpfile.js
[09:51:09] Starting 'sass'...
[09:51:14] Finished 'sass' after 5.14 s

govuk-prototype-kit on  switch-dart-sass [$!] is 📦 v9.10.1 via ⬢ v12.13.1 took 7s 
➜ npx gulp sass
[09:51:16] Using gulpfile ~/Code/govuk-prototype-kit/gulpfile.js
[09:51:16] Starting 'sass'...
[09:51:21] Finished 'sass' after 5.18 s

govuk-prototype-kit on  switch-dart-sass [$!] is 📦 v9.10.1 via ⬢ v12.13.1 took 6s 
➜ npx gulp sass
[09:51:23] Using gulpfile ~/Code/govuk-prototype-kit/gulpfile.js
[09:51:23] Starting 'sass'...
[09:51:29] Finished 'sass' after 5.56 s

[36degrees]

I've pushed a branch with my changes here, for future reference:
https://github.com/alphagov/govuk-prototype-kit/tree/switch-dart-sass

[joelanman]

Just to record the fact that there is an executable version of Dart Sass which is significantly faster. However it probably isn't workable for us as you can't run it in places like Heroku or Glitch

https://sass-lang.com/dart-sass

[edwardhorsford]

I was previously on a project where compile times took similar times (rails) - it's workable, but also damn annoying. I suspect for novice users there's more chance as @36degrees says that they may think something has gone wrong.

Like @36degrees says, as compilation only happens at build, I'd assume you'd need control of the repo to take advantage of any security vulnerabilities - which should greatly mitigate the short term risks.

[36degrees]

Removing the IE8 stylesheets also improves compilation times – bringing us down to ~3s – but that's true with node-sass as well where compilation times are reduced to ~500ms.

For context, govuk-frontend compiles (on Travis) in 0.386s using LibSass, or 1.487s using Dart Sass. Which seems to tally with the times we're seeing, given in the sass task we're compiling 4 stylesheets every time and the majority of their Sass comes from govuk-frontend.

[joelanman]

@36degrees I wonder if we need to compile IE8 for prototyping? Or at least maybe it should be off by default? Is that hard?

[joelanman]

The Financial Times has the faster executable as an npm package:

https://twitter.com/JakeDChampion/status/1374415348288475141

https://www.npmjs.com/package/@financial-times/sass

[lfdebrux]

This was done in #1269, closing.