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

Add .sass support #579

Merged
merged 2 commits into from
Aug 17, 2016
Merged

Add .sass support #579

merged 2 commits into from
Aug 17, 2016

Conversation

alex88
Copy link
Contributor

@alex88 alex88 commented Aug 17, 2016

This adds real .sass support, not just the .scss support.
We don't need to add indentedSyntax to the sass loader parameters since the sass loader
doesn't require it anymore (webpack-contrib/sass-loader#196)

This adds real .sass support, not just the .scss support.
We don't need to add `indentedSyntax` to the sass loader parameters since the sass loader
doesn't require it anymore (webpack-contrib/sass-loader#196)
@CLAassistant
Copy link

CLAassistant commented Aug 17, 2016

CLA assistant check
All committers have signed the CLA.

@doug-wade
Copy link
Collaborator

lgtm 👍

Thanks @alex88

@gigabo gigabo added the enhancement New functionality. label Aug 17, 2016
@gigabo
Copy link
Contributor

gigabo commented Aug 17, 2016

Nice, thanks @alex88!

If you're feeling ambitious you could also add a .sass import to the sass test page.

@alex88
Copy link
Contributor Author

alex88 commented Aug 17, 2016

Ok let me do that right now, just a quick question, how you test those pages? create a project using the local linked version of the cli?

@alex88
Copy link
Contributor Author

alex88 commented Aug 17, 2016

Oh nvm, RTFM to me :D

@gigabo
Copy link
Contributor

gigabo commented Aug 17, 2016

Heh, yep some basic instructions in the README.

You'll need to npm run prepublish in packages/react-server-cli once to get your patch built.

^ That's actually covered by the npm run bootstrap step mentioned in the README.

If you run into any trouble just ask here or on slack!

@alex88
Copy link
Contributor Author

alex88 commented Aug 17, 2016

Already tried to join slack before
screen shot 2016-08-17 at 11 20 00 am

@gigabo
Copy link
Contributor

gigabo commented Aug 17, 2016

Whoa, that's not good!

@davidalber - Looks like slackin's rejecting people!

@gigabo
Copy link
Contributor

gigabo commented Aug 17, 2016

^ Ticket for slack issue: #580

@alex88 - I tried to send you a manual invite, but ran into the same issue. :(

image

@alex88
Copy link
Contributor Author

alex88 commented Aug 17, 2016

Damn, so strict :) Maybe contact them directly, or have an IRC channel too :)
Anyway, better to squash the commits?

@gigabo
Copy link
Contributor

gigabo commented Aug 17, 2016

Yeah, I reached out via their help form. :fingers_crossed:

No need to squash. Your commits represent discrete logical changes.

Thanks for improving React Server!

@gigabo gigabo merged commit 5f55eba into redfin:master Aug 17, 2016
@alex88
Copy link
Contributor Author

alex88 commented Aug 17, 2016

@gigabo is there a way to use the master version in a project? I've seen that npm doesn't support using packages directly from a git subfolder, you guys ever found a solution?
Except linking a local folder obviously.

@alex88 alex88 deleted the sass-support branch August 17, 2016 18:52
@gigabo
Copy link
Contributor

gigabo commented Aug 17, 2016

@alex88 We've just been trying to publish frequently so the latest features/fixes are available.

I'll publish 0.4.4 to get your patch out now. 🚀

@alex88
Copy link
Contributor Author

alex88 commented Aug 17, 2016

Awesome! 🎉 I just tried to checkout locally and seems the sub-dependencies can't be loaded..
Thanks a lot for publishing!

@gigabo
Copy link
Contributor

gigabo commented Aug 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New functionality.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants