thgh / rollup-plugin-scss

Rollup and compile multiple .scss, .sass and .css imports
MIT License
134 stars 47 forks source link

Remove node-sass from dependencies #65

Closed astappiev closed 3 years ago

astappiev commented 3 years ago

As described in #61, even after moving node-sass to optional dependencies it is still downloaded by npm/yarn.

Moving it to peer dependencies solves the issue. Now users have to specify what package they want to use sass or node-sass in their projects.

astappiev commented 3 years ago

Ok, peerDependencies downloaded too.

So, if we want to avoid node-sass and sass installed at the same time, we have to remove them :/

andreimoment commented 3 years ago

Does this mean that mom install sass --dev is sufficient to replace node-sass with sass?

Or, is there an option to specify the sass engine to be used?

astappiev commented 3 years ago

Yes, you have to install rollup-plugin-scss and sass (or node-sass).

Like one of:

Until this is merged, you can try

andreimoment commented 3 years ago

Thank you for the quick reply.

I already have rollup-plug-in-scss installed and set up - thank you for the additional watch options :)

So if I uninstall node-sass which was auto installed , and install sass (dart sass) the plugin will pick up and use whichever is available?

Edit: I uninstalled node-sass but it remains in the package-lock as a dependency of this plugin.

Edit 2: I replaced with the @astappiev/rollup-plugin-scss and it seems to run dart sass.

On Sat, Feb 6, 2021 at 1:56 AM Oleh Astappiev notifications@github.com wrote:

Yes, you have to install rollup-plugin-scss and sass (or node-sass).

Like one of:

  • npm install rollup-plugin-scss sass --save-dev
  • npm install rollup-plugin-scss node-sass --save-dev

Until this is merged, you can try

  • npm install @astappiev/rollup-plugin-scss sass --save-dev

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/thgh/rollup-plugin-scss/pull/65#issuecomment-774437771, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAABQLVBV3PFTS64ULO763DS5UG3FANCNFSM4VVXIZIQ .

-- Sent via mobile.

astappiev commented 3 years ago

rollup-plugin-scss will not use sass automatically until this PR is merged and a new version released. There is workaround like setting sass in configuration (like sass: require('sass'),), but it will anyway install node-sass even if not used.

If you use my patched version @astappiev/rollup-plugin-scss, you can verify that node-sass doesn't exist in node_modules directory.

astappiev commented 3 years ago

Done

EtienneBruines commented 3 years ago

It is nice that for v3, the dependency is being removed completely. Just a quick question here:

In v2.6.1, the dependency was already moved towards optionalDependencies -- yet the npm release still includes it under dependencies.

Did something go wrong with the deployment, causing the discrepancy?

From the downloaded package.json:

{
  "_resolved": "https://registry.npmjs.org/rollup-plugin-scss/-/rollup-plugin-scss-2.6.1.tgz",
  "dependencies": {
    "node-sass": "4",
    "rollup-pluginutils": "2"
  },
  "optionalDependencies": {
    "node-sass": "4"
  },
}
thgh commented 3 years ago

Seems to be a "feature": https://github.com/npm/npm/issues/3502

andreimoment commented 3 years ago
  1. Thank you for the great work with this plugin. Now that I am running @astappiev/rollup-plugin-scss version, dart-sass compilation is instantaneous. This is the fastest scss development I've ever experienced and a huge improvement from sassc. And, it watches the whole tree (If you look at the snowpack plugin you'll see they're having issues with this part).
  2. When can I replace this version with the stock version in my installation? Has v3 been released?