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

Move to dart-sass instead of node-sass #1509

Closed
pablomayobre opened this issue Jun 8, 2018 · 5 comments
Closed

Move to dart-sass instead of node-sass #1509

pablomayobre opened this issue Jun 8, 2018 · 5 comments
Labels
💬 RFC Request For Comments

Comments

@pablomayobre
Copy link

pablomayobre commented Jun 8, 2018

🙋 Feature Request

Since late march dart-sass reached the stable release 1.0.0, marking the start of the deprecation of ruby-sass (not libsass though).

Although node-sass is based on libsass and will continue to be updated with the latest patch and fixes, it will take more time for this features to land while dart-sass will receive them immediately and has the benefit of compiling to pure JS without the need of native modules.

Dart-sass is now at version 1.5.1 (as of 8/6/2018) and in nice shape to be used in production. NPM Link

🤔 Expected Behavior

Parcel should be able to detect the sass module instead of node-sass, and maybe fallback to it, if needed, though it may be a good idea to drop it altogether and look forward to #1253

😯 Current Behavior

Parcel incorrectly detects sass and asks for node-sass instead, failing to compile .sass/.scss files

💁 Possible Solution

Luckily sass is API compatible with node-sass so it should be as easy as changing the 2 occurrences of node-sass to sass at package.json line 90 and SASSAsset.js line 16

@DeMoorJasper
Copy link
Member

Dart sass is slower and would require the dart runtime right?

@DeMoorJasper DeMoorJasper added the 💬 RFC Request For Comments label Jul 11, 2018
@pablomayobre
Copy link
Author

pablomayobre commented Jul 14, 2018

Citing Dart Sass performance

Dart Sass on Node lags behind, particularly when many extends are in use. It's still faster than it was last measurement, but it would probably pay dividends to do some JS-specific benchmarking and optimization to try to bring the speed closer to that of the Dart VM. The majority of our users run Dart Sass through JS, so while it's good that they have a path to better performance with the same semantics, improving the baseline performance is important.

So yeah it's slower, but it's pure JS. I also saw mentions of pure JavaScript version here

Also the compiled Dart Sass standalone (which doesn't require the Dart VM) seems to be on par to libsass and may be faster for some test cases

PS: It may be a good idea to ask someone on the Sass team to clarify this stuff

@DeMoorJasper
Copy link
Member

Pure JS isn’t necessarily better, the major issues we currently have with sass are unrelated with it being in C++.
Idk if it’s worth the tradeoff would indeed be usefull to have someone from sass shim a light on this issue

Sent with GitHawk

@sindresorhus
Copy link

@DeMoorJasper You should at least let people be able to choose dart-sass? It has additional benefits like not requiring a native addon.

There are also a myriad of issues with using node-sass with Parcel:

// @nex3

@DeMoorJasper
Copy link
Member

DeMoorJasper commented Aug 3, 2018

@sindresorhus I thought we fixed the freezing issue, that’s why I thought this issue was pretty unnecessary, but now that someone has reported it again I’m pretty sure it’s a good idea to support dart-sass, still unfortunate that they don’t have a solid sass-js or something.

I’ll probably open up a pr for this later today (or link a plugin)

Sent with GitHawk

DeMoorJasper added a commit that referenced this issue Aug 16, 2018
…1847)

Dart sass will prevent parcel from randomly freezing.
It's also a lot stricter, preventing unknown errors or side-effects after compiling/parsing.
Also adds codeFrames, so users get a visual representation of the line the error occurs.

Closes #1836 Related to #1509
devongovett pushed a commit that referenced this issue Oct 15, 2018
…1847)

Dart sass will prevent parcel from randomly freezing.
It's also a lot stricter, preventing unknown errors or side-effects after compiling/parsing.
Also adds codeFrames, so users get a visual representation of the line the error occurs.

Closes #1836 Related to #1509
devongovett pushed a commit that referenced this issue Oct 15, 2018
…1847)

Dart sass will prevent parcel from randomly freezing.
It's also a lot stricter, preventing unknown errors or side-effects after compiling/parsing.
Also adds codeFrames, so users get a visual representation of the line the error occurs.

Closes #1836 Related to #1509
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
💬 RFC Request For Comments
Projects
None yet
Development

No branches or pull requests

3 participants