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

Removing ember-cli-sass dependency #172

Closed
musaffa opened this issue Apr 12, 2018 · 12 comments · Fixed by #659
Closed

Removing ember-cli-sass dependency #172

musaffa opened this issue Apr 12, 2018 · 12 comments · Fixed by #659

Comments

@musaffa
Copy link

musaffa commented Apr 12, 2018

I've migrated from sass to postcss. I still use sass syntax (variables, import) in postcss, but I don't need ember-cli-sass to do it. This is the only addon in my app which has a sass dependency. I would love to see this dependency removed. Those who want to override the sass variables of ember-freestyle can install ember-cli-sass on their own. Thanks.

@musaffa musaffa changed the title Making ember-cli-sass an optional dependency Removing ember-cli-sass dependency Apr 12, 2018
@PoslinskiNet
Copy link

Seems like related: #152

@bertdeblock
Copy link
Collaborator

Is this still an issue now that node-sass isn't used anymore?

@patrickberkeley
Copy link

@bertdeblock yeah this issue is still relevant IMO. We're in the same boat as @musaffa: Our app doesn't use sass, but we would like to use ember-freestyle which requires sass as a dependency.

@bertdeblock
Copy link
Collaborator

Okay, but would you mind clarifying what the issue is with ember-freestyle using ember-cli-sass? Sorry, I'm not really following I'm afraid.

@patrickberkeley
Copy link

patrickberkeley commented Oct 27, 2021

We don't want ember-cli-sass (and its dep tree) as a dependency of our apps. But ember-freestyle requires it, so if we want freestyle we must install ember-cli-sass, even though we have no use for it.

ember-cli-sass is just a heavy thing to install for something as small as a style guide.

@bertdeblock
Copy link
Collaborator

I don't think that's true? We use ember-freestyle in apps that don't have ember-cli-sass as a dependency.

@patrickberkeley
Copy link

When we install ember-freestyle, ember-cli-sass is a dependency that's installed. Here's our yarn.lock addition after installing freestyle:

#152 also describes and solves this same issue.

@bertdeblock
Copy link
Collaborator

I thought that you meant that you needed to install ember-cli-sass as a direct dependency of your app as well in order to use ember-freestyle. The reason for #152 is slightly different though afaict.
Anyhow, I don't have any strong feelings on the fact that this addon uses ember-cli-sass, but I also don't see it as a big issue if the author wants to use this.
How would you feel about addons using e.g. ember-cli-postcss?

@bertdeblock
Copy link
Collaborator

I'll see if I can pick up #152 again somewhere in the near future.

@patrickberkeley
Copy link

I thought that you meant that you needed to install ember-cli-sass as a direct dependency of your app as well in order to use ember-freestyle.

Ah, understood.

How would you feel about addons using e.g. ember-cli-postcss?

I would feel good about it, but that is just because our apps happen to use ember-cli-postcss. IMO it would be best if freestyle didn't require a CSS processor. A style guide is to highlight an app's components, and shouldn't add CSS processing the app doesn't need. It's just extra weight. We know every app is going to use – or at least can handle – raw CSS because it's a browser baseline. So IMO a style guide addon would be less intrusive / more valuable if it just used raw CSS.

Maybe a better/simpler alternative would be to allow ember-freestyle to use whatever it wanted for CSS processing, but make it optional to include default styles i.e., if I don't want to use freestyle's built-in styles and get it's CSS processing packages, then I have to write the styles myself.

@bertdeblock
Copy link
Collaborator

@patrickberkeley FYI, ember-cli-sass and sass are ditched in v0.13.2.

@patrickberkeley
Copy link

Thanks @bertdeblock ! We've been following your work here, and have a ticket to upgrade and remove sass.

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

Successfully merging a pull request may close this issue.

4 participants