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

Default SASS implementation #199

Open
drummy2 opened this issue Nov 2, 2018 · 12 comments
Open

Default SASS implementation #199

drummy2 opened this issue Nov 2, 2018 · 12 comments

Comments

@drummy2
Copy link

drummy2 commented Nov 2, 2018

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?

@jbailey4
Copy link
Contributor

jbailey4 commented Feb 5, 2019

@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.

@robclancy
Copy link

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

@jbailey4
Copy link
Contributor

@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.

@javve
Copy link

javve commented May 9, 2019

I made a comment about the slowness here as well: #192 (comment) 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.

@RuslanZavacky
Copy link

@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).

@robclancy
Copy link

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.

@jbailey4
Copy link
Contributor

jbailey4 commented Jun 9, 2019

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 😄

@robclancy
Copy link

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.

@EWhite613
Copy link

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?

@knownasilya
Copy link
Collaborator

knownasilya commented Jun 24, 2022

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

@kdagnan
Copy link

kdagnan commented Jul 19, 2022

@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.

@knownasilya
Copy link
Collaborator

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

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

8 participants