Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(pv-scripts): replace using node-sass with (dart-)sass #181

Merged
merged 5 commits into from Apr 26, 2024

Conversation

mbehzad
Copy link
Contributor

@mbehzad mbehzad commented May 5, 2022

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

  • allows usage of modern CSS syntax. For example width: min(100px, 10vw); instead of weird workaround via interpolations: #{"width: min(100px, 10vw);"}
  • Better modularization possibilities
  • node-sass binaries are tied to the node version which means changing node version is coupled to the need of re-downloading node-sass binaries. etc.

Cons

  • not being able to use / for division. e.g.
// node-sass
width: 100% / 3;


// dart-sass
@use "sass:math" as *;

width: div(100%, 3);

// or use the browser to do the job:
width: calc(100% / 3);

There is a sass-migrator cli tool which can help automatically update the codebase.

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 or sass themselves. (similar to CRA which we have its error message already in formatWebpackMessages.js#L79

== Closes issue(s) ==

== Changes ==

== Affected Packages ==
pv-scripts

use the current immplementation of sass to compile scss files

BREAKING CHANGE: see https://sass-lang.com/documentation/breaking-changes for migration from
node-sass to dart-sass
… feature/node-sass-to-dart-sass

# Conflicts:
#	packages/pv-scripts/package-lock.json
… feature/node-sass-to-dart-sass

# Conflicts:
#	packages/pv-scripts/package-lock.json
… feature/node-sass-to-dart-sass

# Conflicts:
#	packages/pv-scripts/package-lock.json
@friewerts friewerts merged commit 02fd2b3 into develop Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants