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

[heft] heft-sass-plugin use deprecated node-sass #3716

Closed
csobj opened this issue Oct 21, 2022 · 2 comments
Closed

[heft] heft-sass-plugin use deprecated node-sass #3716

csobj opened this issue Oct 21, 2022 · 2 comments

Comments

@csobj
Copy link
Contributor

csobj commented Oct 21, 2022

Summary

node-sass is deprecated and no additional sass feature will come.

Details

node-sass (and libsass) used by heft-sass-plugin has been deprecated and is not compatible with the new css or sass feature. The same kind of PR #1666 was resolved before, but it seems that the code using node-sass is restored when refactoring the rushstack-legacy.

There are 3 sass implementation available for now.

  • node-sass: Deprecated. many sass features still are available but nothing new will come.
  • dart-sass(sass): It's known as having some performance overhead because it is pure javascript version converted from dart-sass. If the changing log I'd saw is correct one, rushstack-legacy was choose node-sass because of performance issue. the compilation performance might be critical especially for large scaled codebase. so this decision should be respected also.
  • sass-embedded: This is javascript wrapper around a native dart executable. Because of it depends Dart, It will be much faster especially for large sass compilations generally, but also have relatively limited platform supports which can install Dart runtime. (Windows, MacOS and Linux)

sass-embedded seems suitable for most users but none of options are perfect.

So maybe giving configurable option is the simpliest and ideal.
Or, We can provide heft-dart-sass-plugin or heft-sass-embedded-plugin like we did for webpack5.

Related

@TheLarkInn
Copy link
Member

I agree that the scalability issue with dart-sass could be an eventual problem but I believe having the option for sass-embedded is a good plug-n-play solution here.

@iclanton
Copy link
Member

Closing because @csobj did the work to upgrade us to dart-sass in #3717

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants