Replace node-sass package with sass
#61722
Conversation
jportner
force-pushed
the
replace-node-sass
branch
from
c998827
to
1940e9e
Compare
jportner
added
chore
release_note:skip
| outputStyle: 'nested', | ||
| outputStyle: 'compressed', |
There was a problem hiding this comment.
While Dart Sass supports a JavaScript API that's fully compatible with Node Sass, I had to make this change:
- Only the
"expanded"and"compressed"values ofoutputStyleare supported.
Reference: https://github.com/sass/dart-sass#javascript-api
| sass.render( | ||
| { | ||
| file: src, | ||
| fiber: Fiber, | ||
| }, |
There was a problem hiding this comment.
I made this optional change to boost performance:
Note however that by default,
renderSync()is more than twice as fast asrender()due to the overhead of asynchronous callbacks. To avoid this performance hit,render()can use thefiberspackage to call asynchronous importers from the synchronous code path.
Reference: https://github.com/sass/dart-sass#javascript-api
Note: we could change this to use the renderSync function instead, then we wouldn't need fibers here. However, we would still need fibers for the kbn-optimizer package.
|
@elasticmachine merge upstream |
This results in better performance for `sass.render()` by calling asynchronous importers from the synchronous code path.
The `sass` module has a known limitation that prevents it from running with the default Jest test environment of "jsdom". Needed to create a separate Jest test config to allow the sass-related unit tests to execute properly.
While this approach works just fine in practice, it doesn't seem
to execute properly in a Jest test environment. The `renderSync`
function obviates the need for `fibers`, so I used that instead
and it's now testable.
Note: snapshots needed to be updated because we are using a
different sass outputStyle ("compressed" instead of "nested").
jportner
force-pushed
the
replace-node-sass
branch
from
5b67dcc
to
73306fc
Compare
💔 Build Failed
Failed CI Steps
Build metricsHistory
To update your PR or re-run it, just comment with: |
Overview
In this PR, I changed all of our usage of
node-sassto(dart-)sassinstead. The latter is the reference implementation of sass; it is more well maintained, has lightweight dependencies, and doesn't rely on C bindings*.Performance
node-sassis generally faster thansassthat is run as native JavaScript. However, based on some anecdotal manual testing, I didn't notice much of a performance difference for Kibana.Before changes
After changes
* Note: I did include
fibersin a couple of consumers to speed up async rendering, but this comes with prebuilt binaries instead of relying onnode-gyp. See #61722 (comment).