feat(pv-scripts): replace using node-sass with (dart-)sass #181
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.
Switch do dart-sass instead of node-sass for processing .scss files
Dart Sass is currently the active implementation of sass and gets new features. It is stable for a long time now, and imo it makes sense to switch to it.
Pros
width: min(100px, 10vw);
instead of weird workaround via interpolations:#{"width: min(100px, 10vw);"}
Cons
/
for division. e.g.There is a sass-migrator cli tool which can help automatically update the codebase.
But once sass-loader can use the native Dart Sass executable, this would be less relevant (see Option to use the DartVM executable rather than the pure-js version of dart-sass. webpack-contrib/sass-loader#774).
Breaking?
Technically it would probably compile successfully our code bases so it isn't a breaking change. But that will generate a lot of deprecation warnings regarding the usage of slash for division.
see https://sass-lang.com/documentation/breaking-changes for possible breaking changes
User preference
It is also possible to allow the user to choose between these two sass implementation by:
1- property in
pv.config.js
2- sass not being a direct dependency of pv-script, and the user has to install
node-sass
orsass
themselves. (similar to CRA which we have its error message already in formatWebpackMessages.js#L79== Closes issue(s) ==
== Changes ==
== Affected Packages ==
pv-scripts