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

Replace node-sassy with node-sass (eliminate Ruby requirement) #76

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

xdissent
Copy link
Contributor

This PR uses node-sass, which is a wrapper for the libsass C implementation. This gets rid of the Ruby requirement, at the cost of .sass-file processing. In that regard it's kind of an opinionated PR, so I understand if you're not interested.

@techpines
Copy link
Owner

I'm cool with this. The sass module was community generated. The only caveat here, is that I don't want to make node-sass a dependency in the package.json.

Binary dependencies sometimes make installing more of a pain, so I would just make a note that you need to do npm install node-sass to use the module. Otherwise installing asset-rack might fail for people that aren't even using it for sass, like me :)

I'll let this sit for a bit to see if anyone that uses the Sass module has an issue with it. Otherwise, I'm fine with merging it if the api is the same.

@xdissent
Copy link
Contributor Author

xdissent commented Jun 1, 2013

Now node-sass is in optionalDependencies, meaning npm will carry on if installation fails for any reason. I put node-sassy back in dependencies since it's tiny and doesn't hurt anything if it's installed and you don't use it. Sass assets will detect whether node-sass is available.

@freshlogic
Copy link

Would love to see this merged

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 this pull request may close these issues.

3 participants