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

Dart Sass? (Sass implementation customization) #422

Closed
Grawl opened this issue Oct 27, 2018 · 2 comments · Fixed by #517
Closed

Dart Sass? (Sass implementation customization) #422

Grawl opened this issue Oct 27, 2018 · 2 comments · Fixed by #517
Labels

Comments

@Grawl
Copy link

Grawl commented Oct 27, 2018

sass-loader supports to specify Sass implementation to use: node-sass/dart-sass/sass

// ...
    {
        loader: "sass-loader",
        options: {
            implementation: require("dart-sass")
        }
    }
// ...

If I understand things right, I can customize sass-loader config like this:

Encore.enableSassLoader(options => {
	options.implementation = require('sass')
})

But if I uninstall node-sass and install sass instead, I got an error because Encore.enableSassLoader() and Encore.addStyleEntry are limited to node-sass only

@Grawl Grawl changed the title Dart Sass? Dart Sass? (Sass implementation customization) Oct 27, 2018
@ghost
Copy link

ghost commented Nov 22, 2018

You’re also going to want the fibers option so it plays nice with the way webpack plugins work, because they’re suppose to be able to handle async calls. To illustrate:

Note that when using Dart Sass, synchronous compilation is twice as fast as asynchronous compilation by default, due to the overhead of asynchronous callbacks. To avoid this overhead, you can use the fibers package to call asynchronous importers from the synchronous code path. To enable this, pass the Fiber class to the fiber option

see here

@stof
Copy link
Member

stof commented Nov 22, 2018

We should remove the check about node-sass being installed in case the implementation option is provided., as it is not necessary in that case. That should be an easy fix.

@ProtonScott the callback allows you to configure any option you want.

@Lyrkan Lyrkan added the HasPR label Feb 10, 2019
weaverryan added a commit that referenced this issue Mar 1, 2019
…verryan)

This PR was merged into the master branch.

Discussion
----------

Allow to use Dart Sass instead of Node Sass

This PR closes #422 by allowing `sass` (Dart Sass) as an alternative choice to `node-sass`.

---

The `sass-loader` allows to set the implementation that should be used to process Sass files. It currently accepts either `sass` or `node-sass` (default):

In theory this can already be used by setting the related option when calling `Encore.enableSassLoader()`, but Encore then throws an error asking you to install `node-sass`, even if you already have `sass` installed.

One way to prevent that could be to detect when `options.implementation` is set but in its next version the `sass-loader` will [automatically detect](webpack-contrib/sass-loader@ff90dd6) which implementation is installed and use it accordingly.

So, instead of doing that, this PR allows to define alternative packages in our `feature.js` file, for instance:

```js
const features = {
    sass: {
        method: 'enableSassLoader()',
        packages: [
            { name: 'sass-loader', enforce_version: true },
            [{ name: 'node-sass' }, { name: 'sass' }] // Will allow both `node-sass` and `sass`
        ],
        description: 'load Sass files'
    },
    // ...
}
```

If neither `node-sass` or `sass` is available, the error message will display both choices, and recommend the first one in the `yarn add` advice:

![image](https://user-images.githubusercontent.com/850046/52538612-0338fa00-2d75-11e9-8d89-146fe7ee27eb.png)

Note that for now using Dart Sass still requires the `implementation` option to be set:

```js
Encore.enableSassLoader(options => {
    options.implementation = require('sass');
});
```

This shouldn't be a big issue since the default recommendation is still to use `node-sass`.

Commits
-------

4e90bf8 clarifying comment
1346ee5 Allow to use Dart Sass instead of Node Sass
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants