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

[gulp-core-build-sass] Consider dart-sass instead of node-sass #1666

Closed
JakeStanger opened this issue Dec 24, 2019 · 6 comments · Fixed by #2682
Closed

[gulp-core-build-sass] Consider dart-sass instead of node-sass #1666

JakeStanger opened this issue Dec 24, 2019 · 6 comments · Fixed by #2682

Comments

@JakeStanger
Copy link

JakeStanger commented Dec 24, 2019

Currently gulp-core-build-sass uses the node-sass package. This has a few disadvantages compared to dart-sass (or the node sass package built from it):

  • dart-sass is the primary Sass implementation. This means it recieves fixes and features often a long time before libsass and subsequently node-sass.
  • dart-sass is a full JavaScript implementation. node-sass has C++ bindings to libsass which have to be compiled. This means node-sass has lower cross platform compatibility and is more prone to error when installing dependencies.
  • dart-sass should be almost entirely compatible with the node-sass API so very little work, if any, would be required.

Due to the fact it is implemented in JS rather than C++, dart-sass is generally slower at compiling stylesheets. Personally I would rather the advantages of reliability and one less package with bindings over the slight extra compile time, however I believe this should be discussed.


For clarity:

  • node-sass is the currently shipped package, which has bindings to the C++ library libsass.
  • sass is the node package built from the Dart package dart-sass.
@iclanton
Copy link
Member

iclanton commented Jan 6, 2020

Performance is a major concern for gulp-core-build-sass, which is why we originally decided to go with node-sass. Are there any bugs in node-sass that you're seeing?

How much slower is dart-sass at this point? I'll admit I haven't looked at the compiler options in a while.

@JakeStanger
Copy link
Author

JakeStanger commented Jan 13, 2020

There are no bugs I have encountered, but there are missing features (such as the new @use modules system).

In terms of performance, the sass CLI tool at least actually seems to be faster than the node-sass one. I threw office-ui-fabric-react at it and the difference was around10x. Of course, node probably has a lot of startup overhead so it's not a particularly fair test. I'm afraid I haven't had time to do much more investigation than that at the moment.

@KevinTCoughlin
Copy link
Member

From SPFx side, I am investigating potential regression of token interpolation: SharePoint/sp-dev-docs#5131. As well as a long-standing bug with current build implementation where shorthand properties are not interpolated properly.

I will report back if node-sass is responsible for either of the above. If so, it may inform the above move from node-sass to dart-sass.

@StfBauer
Copy link

@KevinTCoughlin I was always under the impression the token as in theme token interpolation not part of the SASS build. Instead it just integrates the strings.

@JakeStanger
Copy link
Author

An update to this: the node-sass package is now considered deprecated and is in a maintenance state (albeit indefinitely) as per the npm page. The official recommendation is to move to Dart Sass.

@LeopoldLerch
Copy link

Pls think of replacing node-sass with dart-sass, as new features are and will not be implemented in node-sass anymore. This is already true for "@use", which is the recommendation to use instead of "@import" for various reasons (and "@import" import is deprecated as well).
In Order to keep developers being able to use modern techniques, the move should be done soon (dart-sass should be an compatible replacement for node-sass anyway).

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

Successfully merging a pull request may close this issue.

5 participants