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

Add support for Dart Sass #672

Closed
nex3 opened this issue Mar 23, 2018 · 9 comments
Closed

Add support for Dart Sass #672

nex3 opened this issue Mar 23, 2018 · 9 comments

Comments

@nex3
Copy link
Contributor

nex3 commented Mar 23, 2018

Dart Sass is in Release Candidate phase now, with a full release probably coming next week. 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.

Right now, this package has a compiler field that can be configured, like so:

var sass = require('gulp-sass');
sass.compiler = require('dart-sass');

However, because gulp-sass includes Node Sass in its dependencies, this still requires users to get Node Sass even if they want to use Dart Sass—which is often a big pain, since that can involve building C code. One way around this would be to remove Node Sass as a dependency and require users to pass in the version of the compiler they want to use, either using the existing API or using something like

var sass = require('gulp-sass')(require('dart-sass'));
@faizanalvi
Copy link

faizanalvi commented Apr 29, 2018

hey, I am only missing Gulp-sass file here in this list. I am new to wordpress So i don't know how to add this particular file into my this list.Please guide me. windows 10. Thankx
capture

@mattdsteele
Copy link

mattdsteele commented May 8, 2018

Want to bump and continue discussion. Dependency-wise, being able to avoid a download of node-sass would be a big plus unless the user opts-in.

Based on the conversation in sindresorhus/grunt-sass#278, I'm not sure there's a solution without a drawback:

  • Setting up dart-sass and node-sass as peerDependencies will trigger warnings for whichever is missing
  • Setting optionalDependencies will trigger a download, and fail if the node-sass binary is missing; which isn't great either
  • We could declare neither but that's not really a great DX

Any other options?

@xzyfer
Copy link
Collaborator

xzyfer commented May 8, 2018 via email

@mattdsteele
Copy link

I've gone ahead and forked to gulp-dart-sass and published a proof of concept to npm: https://www.npmjs.com/package/gulp-dart-sass

A few features aren't working (namely around sourcemaps) but hopefully this is enough for folks to begin using.

@nex3
Copy link
Contributor Author

nex3 commented May 20, 2018

I'd really like Dart Sass to be a first-class implementation, and naming its package gulp-dart-sass while the package named gulp-sass only supports Node Sass doesn't really frame it that way. I'd prefer to see a solution where users explicitly decide which implementation to use, possibly using the pattern I outlined above, even if that does increase the first-time setup friction somewhat.

We see relatively few issues with regards to installation of node-sass.

@xzyfer You and I may be hearing from different people, but I've heard a lot of noise around people who have had bad Node Sass installation experiences.

nex3 added a commit to nex3/gulp-sass that referenced this issue May 24, 2018
Rather than always loading Node Sass, this now requires users to pass
in either Dart Sass or Node Sass to the require() call.

Closes dlmanning#672
@nex3
Copy link
Contributor Author

nex3 commented May 24, 2018

I've created #694 as a potential implementation of this, using the require('gulp-sass')(require('sass')) method of customization.

@xzyfer
Copy link
Collaborator

xzyfer commented Jun 8, 2018

@mattdsteele could you please update the links in the package.json and readme to point at the correct repository? We've started receiving issues we're not suited to resolve.

@mattdsteele
Copy link

@xzyfer Removed references to gulp-sass in the readme.

package.json was already referencing my repo for repository and bugs, let me know if there's a different link you'd like updated.

@xzyfer
Copy link
Collaborator

xzyfer commented Jun 8, 2018

Thanks :)

xzyfer pushed a commit that referenced this issue Oct 16, 2018
Rather than always loading Node Sass, this now requires users to pass
in either Dart Sass or Node Sass to the require() call.

Closes #672
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

4 participants