sindresorhus / grunt-sass

Compile Sass to CSS
MIT License
1.01k stars 209 forks source link

Add support for Dart Sass #278

Closed nex3 closed 6 years ago

nex3 commented 6 years ago

Dart Sass has been released, and it would be great to add support to this package for choosing between Node Sass and Dart Sass. I'm willing to create a pull request, but I'm not sure what the best design would be.

sindresorhus commented 6 years ago

We could remove node-sass as a dependency here and just tell the user to either install node-sass or dart-sass. We could try requiring both and use the one that exists.

Something like:

let sass;
try {
    sass = require('dart-sass');
} catch (_) {
    sass = require('node-sass');
}
xzyfer commented 6 years ago

I think the gulp-sass approach will be to continue depending on node-sass as the default and accept an optional engine.

const engine = opt.engine || require('node-sass');
...
engine.render(..)

In time we could move towards having both as peerDependencies.

sindresorhus commented 6 years ago

IMHO, either both are dependencies or none are. peerDependencies would not work as you can’t have optional peer dependencies.

nex3 commented 6 years ago

We could remove node-sass as a dependency here and just tell the user to either install node-sass or dart-sass. We could try requiring both and use the one that exists.

Something like:

let sass;
try {
  sass = require('dart-sass');
} catch (_) {
  sass = require('node-sass');
}

This could work, but it does mean that users can't explicitly configure which implementation of Sass they want to use. For example, a user may transitively depend on Node Sass for some entirely different reason (maybe a component uses it for its own CSS) but want to use Dart Sass with Grunt. Or maybe a user wants to use a version of Dart Sass that's installed locally to test out a change they're working on.

I think the gulp-sass approach will be to continue depending on node-sass as the default and accept an optional engine.

const engine = opt.engine || require('node-sass');
...
engine.render(..)

In time we could move towards having both as peerDependencies.

One of the big benefits of Dart Sass is avoiding the installation headaches of Node Sass. If gulp-sass continues to depend on Node Sass, then users still have to pay that installation cost even when they aren't using Node Sass at all.

xzyfer commented 6 years ago

One of the big benefits of Dart Sass is avoiding the installation headaches of Node Sass

I understand. The intent for gulp-sass is not to replace node-sass as the default, but to give people who want to use dart-sass the option to do so.

One thing we can do is make node-sass an optional dependency. This way if node-sass fails to install for some reason the installation process won't bail out. This allows people to add dart-sass as dependency and use it as the engine.

nex3 commented 6 years ago

It's up to you, but I really think users would be happier with no built-in dependency from gulp-sass onto Node Sass. Not needing to worry about installing Node Sass is a very real part of Dart Sass's value proposition.

laurenhamel commented 6 years ago

I came across this thread and wanted to chime in. I too was looking for a grunt plugin that uses Dart Sass instead of the now deprecated Ruby Sass after making the switch. I went ahead a built a similar package to this one, grunt-dart-sass, which uses Dart Sass.

xzyfer commented 6 years ago

Before filing an issue, please make sure you have Updated to the latest Gulp Sass and have gone through our Common Issues and Their Fixes section.

xzyfer commented 6 years ago

One last thing

xzyfer commented 6 years ago

Actually maybe that only updates once published a new version.