ember-cli-sass icon indicating copy to clipboard operation
ember-cli-sass copied to clipboard

Default SASS implementation

Open drummy2 opened this issue 6 years ago • 12 comments

Originally we had all addons had a dev dependency of ember-cli-sass. Is it now the case I have to go through all our addons and add in the

sassOptions: {implementation: require("node-sass")}

AND

then install node-sass on every addons dev dependencies?

drummy2 avatar Nov 02 '18 10:11 drummy2

@drummy2 PR #192 should have defaulted installations of ember-cli-sass to use dart-sass by default, removing the need for node-sass. This was released in v8.0.1.

jbailey4 avatar Feb 05 '19 16:02 jbailey4

We have to stick to 8.0.1 because dart-sass is painfully slow and the node-sass never worked in newer versions.

robclancy avatar Feb 14 '19 17:02 robclancy

@robclancy Interesting. I haven't noticed any slow down when I switched a large ember app from node-sass -> dart-sass. At least we have the implementation escape-hatch in this addon. Hopefully future versions of dart-sass will improve whatever slowness you're seeing.

jbailey4 avatar Feb 17 '19 16:02 jbailey4

I made a comment about the slowness here as well: https://github.com/aexmachina/ember-cli-sass/pull/192#issuecomment-490434285 TLDR: dart-sass was 6x slower

When I told a @himynameisjonas about this they also switched back to node-sass and got a speed increase by 4.5x.

javve avatar May 09 '19 12:05 javve

@javve we've also switched to node-sass back. Our builds inside addon times are:

4.5-6 seconds vs 400-500ms for node-sass.

Then using the same addon inside the app with its own scss files is:

8-9 seconds vs 400-600ms for node-sass.

I highly recommend to make node-sass by default and dart version an opt-in.

Also, dart version allow to use variables that are not imported in the file were you are using them (or maybe dart version is smart enough to resolve dependencies? But not sure).

RuslanZavacky avatar May 31 '19 14:05 RuslanZavacky

Yeah honestly I'm probably just going to fork this in future based on when it was just node-sass instead of trying to support everything.

robclancy avatar May 31 '19 17:05 robclancy

Making dart-sass the default started here and was completed here, for reference.

The motivation was based on the fact that dart-sass is the primary, default SASS implementation declared by the official sass-lang team. This means all new SASS features will land in dart-sass first, which I assume would mean performance improvements too. IMO a package that wraps SASS, like this one, should default and "bet on" the default implementation that is supported by the official team.

As always YMMV - that's why there is an escape-hatch since other implementations do exist. For the projects I've worked on dart-sass was a huge improvement over node-sass due to no longer needing OS bindings, which caused a lot of pain in CI and fresh project installations (dart-sass via NPM is compiled to pure JS).

Yeah honestly I'm probably just going to fork this in future based on when it was just node-sass instead of trying to support everything.

I think installing node-sass and adding a one line config, is a much better tradeoff than forking 😄

jbailey4 avatar Jun 09 '19 12:06 jbailey4

The fact it never worked when I tried to configure to node last time is why I will be forking it. Node is now a second class citizen here even though dart has significant performance regressions. And addons that try to do everything always do each thing a little worse than one that just does one thing well. If node-sass has a bug I guarantee it will take much longer to fix here than if this was only node-sass still. On smaller projects maybe I would use this version since it would be less sass to wait on, or more likely I would ditch sass all together since css is pretty good now. But dart-sass here is so slow... just no.

robclancy avatar Jun 12 '19 16:06 robclancy

Do we know if there's a way to point ember-cli-sass to a non javascript implementation?

Say you have the dart-sass version running in the DartVm?

EWhite613 avatar Jun 23 '22 21:06 EWhite613

The default is currently the dart sass version. Which is the only one that works on ARM processors as well.

knownasilya avatar Jun 24 '22 00:06 knownasilya

@EWhite613 @knownasilya I was able to successfully get "embedded sass host" working with ember-cli-sass. It's basically a faster implementation of dart-sass that uses a DartVM instead of pure Javascript. It's updated together with the dart sass project by the same team.

Super simple, just need to add:

const embeddedSass = require('sass-embedded');
sassOptions: {
     implementation: embeddedSass,
     outputStyle: 'expanded' //Required or 'compressed'
}

I was able to cut my production build time from 9 mins to 3 mins locally with this change vs dart-sass. (This is because we have 250 white labels to compile stylesheets for).

Confirmed no issues on Apple Silicon Mac & x86 Jenkins CI Linux Server.

kdagnan avatar Jul 19 '22 13:07 kdagnan

Oh, that might be worth a breaking change to make it the default.

knownasilya avatar Jul 19 '22 13:07 knownasilya