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

Adds .scss support with sass-loader, node-sass, and a simple webpack config edit. #115

Closed
wants to merge 3 commits into from
Closed

Conversation

FiberJW
Copy link

@FiberJW FiberJW commented Jul 22, 2016

A simple feature I enjoyed when I was helping with and using enclave. Edited the template to ensure that it worked.

@ghost
Copy link

ghost commented Jul 22, 2016

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@ghost ghost added the CLA Signed label Jul 22, 2016
@ghost
Copy link

ghost commented Jul 22, 2016

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@lacker
Copy link
Contributor

lacker commented Jul 23, 2016

There is sort of a paradox here. If this project accepts all extra file formats like scss, it will become bloated, and everyone will want to eject so that they can slim things down by removing all the things they don't use. Yet each individual one doesn't seem that bad. It feels to me like we need some philosophy on what features to install by default beyond "it is a useful feature to some people".

@mxstbr
Copy link
Contributor

mxstbr commented Jul 23, 2016

Agreed @lacker, let's discuss further in #78

@ghost ghost added the CLA Signed label Jul 23, 2016
@gaearon
Copy link
Contributor

gaearon commented Jul 23, 2016

Hi Juwan, thanks for PR!
We will hold off from adding SASS and other CSS preprocessors, at least for the initial time.

We would like to know which features specifically you find useful in React apps, and why.

In my experience, Less/SASS don’t add much to React projects because features like nesting are much less necessary thanks to the component model. Please add your feedback in #78 to let us know which features you are missing with React + regular CSS, and we might revisit this decision later.

Thanks!

@gaearon gaearon closed this Jul 23, 2016
@adiachenko
Copy link

adiachenko commented Nov 13, 2016

@gaearon What if I use css framework and want to customize it's styles via SASS variables (see bootstrap)? You assume that developers will either write their own css which they are in total control of or plug in the complete package like React Bootstrap which is not always the case.

I understand why you're reluctant to include SASS support, but with it quickly becoming an industry standard this issue shouldn't be dismissed easily.

@petehouston
Copy link

petehouston commented Nov 13, 2016

@adiachenko you can just eject and add SASS for your needs. SASS is just another CSS-preprocessor, not the main part in the React ecosystem. In React, you style component with JS, which is a more powerful tool. I'd rather want to add CSS modules to create-react-app as default supported feature, but it is actually unnecessary since it is part of other CSS-module implementation.

SASS has been around for several years but I've not seen it becomes industry standard. When it really becomes, then you won't need to use create-react-app tool.

The purpose of create-react-app is to help developers getting started with React development faster and easier without being distracted by other components like Babel, Webpack... not a tool that provides everything.

I prefer the current state of create-react-app, small and lightweight.
Just my opinion, though.

@thien-do
Copy link
Contributor

Hi @adiachenko , there are ways to use Sass with Create React App without ejecting, please see here: #1008

You can also fork React scripts to add Sass yourself, please see #779

@adiachenko
Copy link

adiachenko commented Nov 13, 2016

@petehouston I see your point. It's just that for now as much as I like create-react-app I am reluctant to use it in way other than a cheetsheet of common practices when I need to eject for such a simple reason. I am not sure adding sass-loader by default would be the best way to address the issue considering the plethora of potential node-gyp issues, but I think it should have gotten more care than simply dismissing the problen under the pretext of being outside of scope of React.

I mean, I don't want to see create-react-app becoming a black box that is used by newcomers to build todo lists. Being officialy supported, this project has plenty of potential for more.

@dvkndn thanks, I've seen that pull request, but it looks a bit ugly and will result in errors in some environments (try running that in powershell). Forking, though, seems like a way to go for now.

@petehouston
Copy link

@adiachenko I know you've fallen in love with create-react-app as much as I do. At current state, I think, just keep ejecting or you can fork your own create-react-app version for a while, until someone comes up with a better idea.

@thien-do
Copy link
Contributor

thien-do commented Nov 13, 2016

Hi @adiachenko , Regarding the SASS approach in that PR, you will need some changes in the command if you don't run it in Unix enviroment, specifically the "&&" syntax (which chains commands). The idea here is to run a watch for your SASS and use node-sass to compile it, without concerning Create React App.

Anyway, I think forking is a good solution for medium to large project. I'm currently using a fork of react-scripts for our company primary products. It is quite stable and a lot better than when we need to build and maintain a build setup ourselves previously. So.. Create React App might not fit for big projects, but a fork will be.

After all, they are just build setup..

@cr101
Copy link
Contributor

cr101 commented Nov 13, 2016

Most web developers who use Bootstrap v4 need to be able to make changes to Bootstrap _variables.scss.
There is a massive Bootstrap community and I find it quite bizarre that CRA still doesn't have support for SASS without having to eject of fork. This is clearly the most sought after feature.
I am still hopeful that SASS support will be added soon.

kalekseev pushed a commit to kalekseev/create-react-app that referenced this pull request Sep 11, 2017
Update tsconfig to support dynamic import
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants