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

Optional Sass Support #78

Closed
sgriffey opened this issue Jul 22, 2016 · 113 comments
Closed

Optional Sass Support #78

sgriffey opened this issue Jul 22, 2016 · 113 comments

Comments

@sgriffey
Copy link

sgriffey commented Jul 22, 2016

Note from Maintainers

We now have official Sass integration documentation.


Would be nice to include CSS compilers such as: Sass, Less, Stylus, or even CSS modules, etc.

@mxstbr
Copy link
Contributor

mxstbr commented Jul 22, 2016

I'm not sure we want to make anything optional, in the sense of having people choose between multiple options. We went down that route for generator-keystone and it's really really hard to maintain.

What you have in create-react-app at the moment is a solid, unopinionated base to just get started™. If people want to add more advanced features they can always eject! (or choose a boilerplate 😉)

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

I see where you’re coming from but I’d like to learn more about your use case for Less / SASS in React. With components and a BEM-like convention, many their features are actually not that useful.

The only one I can think of that I’m truly missing in this setup is the ability to share constants across files. This is useful e.g. for color themes. But there is an escape hatch even for this: you can use JavaScript styles (or just inline styles) and then import those values.

What other features are you missing?

@appjitsu
Copy link

appjitsu commented Jul 22, 2016

  • No Testing
  • No LESS or Sass support
  • No Hot reloading of components

So how is this project useful if you still have to add these things to ease your development?

@sgriffey
Copy link
Author

Perhaps just adding recipes or documentation for various customizations, like Sass or test runners, using the eject is simple enough and does not require making the project opinionated.

This would help beginners (like me) be able to step their way to something more complicated.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

So how is this project useful if you still have to add these things to ease your development?

We’re sorry if it’s not useful to you yet. We think that many people learning React would appreciate even the current feature set.

Please let’s keep this issue focused on Less/SASS, not on other issues. You can discuss testing in #80. Hot reloading of components won’t be supported until I figure out how to make it more stable (I’m the author of React Hot Loader). I fully intend to work on this, but there are more pressing issues.

Thanks!

@sotojuan
Copy link

@appjitsu This project isn't made for you then—it's just the basics to get started with a basic web app for demos and learning the basics of React. This isn't ember-cli for React.

There's plenty of awesome projects in that vein like @mxstbr's boilerplate. The simplicity of this project is the best part about it.

@appjitsu
Copy link

Sorry. Don't take my comments as being rude. I think this is a fantastic project.

Although, I do view each of those features to be critical for ease of development.

On Jul 22, 2016, at 12:50 PM, Dan Abramov notifications@github.com wrote:

So how is this project useful if you still have to add these things to ease your development?

We’re sorry if it’s not useful to you yet. We think that many people learning React would appreciate even the current feature set.

Please let’s keep this issue focused on Less/SASS, not on other issues. You can discuss testing in #80. Hot reloading of components won’t be supported until I figure out how to make it more stable (I’m the author of React Hot Loader). I fully intend to work on this, but there are more pressing issues.

Thanks!


You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or mute the thread.

@appjitsu
Copy link

Well OK then! Thanks! 😄

On Jul 22, 2016, at 12:53 PM, Juan Soto notifications@github.com wrote:

@appjitsu This project isn't made for you then—it's just the basics to get started with a basic web app for demos and learning the basics of React. This isn't ember-cli for React.

There's plenty of awesome projects in that vein like @mxstbr's boilerplate. The simplicity of this project is the best part about it.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@sotojuan
Copy link

@appjitsu Didn't mean to sound like I thought you were rude, sorry 😄

Those features are definitely awesome for development and production, but this is for learning and demos (for now at least). Give it some time.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

^^ good discussion but let’s focus on Less/SASS in the future comments 😉

@thecodegoddess
Copy link

SASS would be awesome to have available. I absolutely would have used it today for a project had SASS been available.

@sotojuan
Copy link

My issue is that if you allow Sass, you'll get "where's PostCSS/Sylus!?!?!?!". It's a can of worms you have to open carefully.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

@thecodegoddess What features of SASS do you use most often that you miss in regular CSS?

@thecodegoddess
Copy link

@gaearon It's more that is what our current architecture is using. I know that you can get away with PostCSS for much as well. Mostly, we have variables and mixins that we are currently using.

@appjitsu
Copy link

It would be great to have option from the cli when you first create the project. Similar to yeoman generators.

On Jul 22, 2016, at 1:11 PM, Juan Soto notifications@github.com wrote:

My issue is that if you allow Sass, you'll get "where's PostCSS/Sylus!?!?!?!". It's a can of worms you have to open carefully.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

@sotojuan
Copy link

Sure, adding a flag (--sass) is easy but again, what will Facebook do when Stylus/Less/PostCSS people complain? Supporting all CSS preprocessors isn't fun and having the user config their own kinda defeats the purpose of a no config project (and technically the flag is configuration!).

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

@thecodegoddess

This makes sense, thanks. For now we want to focus on making this useful for new projects because there are all sorts of conventions in old codebases that would be tricky to support. But I agree we need some solution for variables (postcss or otherwise).

Supporting all CSS preprocessors isn't fun and having the user config their own kinda defeats the purpose of a no config project (and technically the flag is configuration!).

Yep.

Not adding flags or configuration options is a hard constraint of this project in the coming months. We might revisit this later.

@roebuk
Copy link

roebuk commented Jul 22, 2016

I know people in the SASS / LESS camps might not be too supportive of this, but here goes.

This project currently uses Babel to ensure that all ES2015 and ES2016 features are supported. So, were ensuring that we the latest JS standards even though the browser support may not be there. So why not take the same approach with CSS and add officially add support for css-next?

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

I thought about css-next but there’s a lot of stuff there. The direction seems valid though.

The most conservative thing we could do is add support just for variables. But obvious we’d need imports too.

Whatever we add, it needs to be rock-stable. I’m worried about issue lists like this: https://github.com/postcss/postcss-import/issues.

@fedesilvaponte
Copy link

I can see in the documentation that it says "currently not supported". Is there a plan to add that or the idea is just to eject and customise it after? I'll be glad to help on finding a solution if the decision is not yet clear.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

We want to wait and see, and most of all we want to understand what problems people are solving with Less/SASS in new React apps today. Perhaps there are other, easier solutions.

@fedesilvaponte
Copy link

I understand your concerns, but in my case sass it's a default. It's not about solving a particular problem, It's just part of my workflow. Just to be clear, maybe the purpose of this tool is not to address every case, but it looks so clean and cool that it saddens me I have to eject and pollute the whole app just to get sass support. I'll be following the development of this tool either way because I think it's great. Thanks.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

Got it, thanks for your perspective. We understand people might be used different tools, but we can’t implement support for them just because of that, as we wouldn’t have good rationales for why some tools are supported and some are not. Then we have 10 different supported tools, and we aren’t sure if they all work, and we need to do a lot of extra work if some underlying tech (like bundler) changes.

I hope this gives you some perspective on why we’d rather avoid adding anything that can be solved by existing means.

@appjitsu
Copy link

http://developer.telerik.com/featured/why-bootstrap-4-means-sass-has-won

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

There is a project called React Bootstrap, does it use either? cc @taion

We shouldn't just do something because Bootstrap does it. Components change how you think about modularising UI quite a lot. So constraints in which Bootstap is operating are not the same as constraints you meet developing React apps.

You should be able to use Bootstrap in React projects, sure. This doesn't mean that something that "won" for Bootstrap is necessary for a React project.

PS I would like further messages in this thread to focus solely on features of Less/SASS that you feel components + CSS don't solve well. Let's not just argue "what is better" because it doesn't help us to make any decisions. Thanks.

@appjitsu
Copy link

Obviously. And it would be pointless to list the benefits of one or another here. That is well documented all over the internets. My point was that if you were to choose one preprocessor, SASS is the clear "winner", because of adoption. If facebook doesn't want to choose, then let us choose.

@taion
Copy link

taion commented Jul 22, 2016

React-Bootstrap just provides React components. We don't ship any of the styling. In cases where I use React-Bootstrap, I pull in Bootstrap via npm, and use less-loader in my webpack config. In general, users of React-Bootstrap will need to provide the Bootstrap CSS somehow, but we don't care how.

But if you want to talk more broadly about Less, Sass, or cssnext, my feedback would be:

  1. In practice, Sass makes more sense than Less; Sass seems to have effectively "won"
  2. Sass is much more user-friendly than cssnext
    1. The cssnext variables syntax is verbose to the point of being painful
    2. Your app doesn't have to be very big at all before mixins are very useful (e.g. style variants for buttons), and they're not in cssnext
    3. If you use CSS modules, then you will hit Add ignoreOrder option to avoid the OrderUndefinedError webpack-contrib/extract-text-webpack-plugin#166, and anyway the way you do composition and components with CSS modules is actually radically different from how you normally do CSS
  3. You can pull together a bunch of PostCSS plugins to make something Sass-like, and @jonathantneal's PreCSS actually gives you that, but then effectively you're shipping something like Sass, except less popular and less familiar to users

Given all that, my recommendation would be to bundle Sass support. I don't think the PostCSS-based ecosystem is sufficiently mature to offer a good option in the context of a project aimed at developers who want an easy way to get started.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

Ah, got it, thanks for clarifying.

@taion
Copy link

taion commented Jul 22, 2016

And I'd be totally fine with pointing users of React-Bootstrap at bootstrap-sass instead of the Less version if they're trying to use React-Bootstrap with create-react-app.

@gaearon
Copy link
Contributor

gaearon commented Jul 25, 2016

Thank you very much for the context, this was super helpful. We’ll keep this in mind, and will be referencing this issue when making decisions in the future.

@taion
Copy link

taion commented Jul 25, 2016

Pardon the logorrhea on my part – on PostCSS, to summarize, we've found the stability just fine. The issue instead is that with the cssnext stuff, it's very easy to write CSS that is badly broken per spec but will right now get processed into something that works.

This is fine for advanced users that are willing to go the extra mile to make sure they're not writing spec-incompliant CSS that still happens to work in cssnext, but to me seems extremely dangerous for general use – like a repeat of the Babel 5 decorator thing, but worse.

This was referenced Jul 26, 2016
@gaearon gaearon mentioned this issue Jul 26, 2016
@timarney
Copy link

First off I would love to have Sass support out of the box. Adding a loader doesn't opt anyone into using Sass - if a Sass file exist it gets parsed.

Looking at this post from @tylermcginnis https://medium.freecodecamp.com/create-react-app-and-the-future-of-creating-react-applications-3c336f29bf1c#.qky3g94y1 very small sample but the CSS comments are both questions about using Sass.

https://medium.com/@jcochran/how-do-you-integrate-sass-without-breaking-things-692dc7c2e248#.5my51vm77

https://medium.com/@FNGR/that-sounds-really-really-cool-but-without-sass-its-not-for-me-7481a85e4283#.voixt9e0u

That said as for Less, Stylus others (likely Sass as well ... ) can they be supported via PostCss?
https://github.com/gilt/postcss-less
https://github.com/MoOx/postcss-cssnext
https://github.com/seaneking/poststylus

I have an issue open here tweaking the plugin setup for PostCSS that would handle these cases

#171

@gaearon
Copy link
Contributor

gaearon commented Jul 26, 2016

We haven’t made any final decisions here. It’s just that we think this is currently out of scope, and we want to come back to this when we have time to look at CSS in React more holistically.

In the meantime you can always use a console SASS tool that outputs .css files right inline, and put *.css in .gitignore.

@taion
Copy link

taion commented Jul 26, 2016

I think this might be a good place to consider adding some customizability.

Ultimately, the choice of CSS framework is mostly orthogonal to the tooling complexity in setting up React.

I think giving a very limited set of choices for mostly orthogonal things like the choice of CSS framework would be a value-add here.

@doodirock
Copy link

I have to say I don't really like the idea that SASS isn't being considered "just because people use it." I fully understand that you are trying to find out "why" people want it included before you decide what to do. However, I would ague that because so many people use it, is really just as valid of a reason.

If the goal of this project is to help beginners, then I would suggest most beginners out there probably have spent a lot of time in the world of SASS over the last few years on the web. These newer concepts for CSS are great, but for most people SASS is what they know. So by limiting these people to either new concepts or plain CSS you limit the goal of the project.

Just my thoughts.

@gaearon
Copy link
Contributor

gaearon commented Jul 29, 2016

Just to make it clear, you can use SASS with this project if you want to.

If you’re used to SASS, you’re also probably used to launching it in the console and letting its watcher compile the SASS files. So this is exactly what you can do:

  1. Put *.css into .gitignore
  2. Create *.sass files instead of *.css
  3. Launch the SASS watcher and let it create the CSS file(s)
  4. Import that *.css file(s)

Yes, this is a bit of extra work, but it’s perfectly doable.

@just-boris
Copy link
Contributor

just-boris commented Jul 29, 2016

@gaearon there is some important issue with your solution: you can't run the whole your development environment with a single npm start, you would have to run sass-watcher as a second process.
That probably will be blocker against doing this.

@mxstbr
Copy link
Contributor

mxstbr commented Jul 29, 2016

That probably will be blocker against doing this.

How is opening a second terminal window a blocker? 😃 It's CMD + T and then npm run sass, that doesn't seem like a blocker!

@gaearon
Copy link
Contributor

gaearon commented Jul 29, 2016

You can also change your start command to start the SASS watcher before react-scripts start.

@just-boris
Copy link
Contributor

just-boris commented Jul 29, 2016

@mxstbr let's say it like I have to open twice more terminal windows than before

@gaearon if I do this:

node-sass --watch --recursive src & react-scripts start

Then I will not be able to stop node-sass because it will be in background.

@gaearon
Copy link
Contributor

gaearon commented Jul 29, 2016

There are workarounds to that too.
(But it might be easier to write your own Node script that spawns both.)

@just-boris
Copy link
Contributor

@gaearon thank you, that probably solves my issue

@giuseppeg
Copy link

@just-boris you can define pre and post scripts in npm that will run before/after a given script e.g. start.
FWIW you can bring up processes that are running in background with fg [ jobid ] and eventually kill them with ctrl+c

@poksme
Copy link

poksme commented Aug 18, 2016

The node-sass watcher in the background with its pid in a variable for cleanup on ctrl+c is a really clean solution.

I also recommend using node-sass binary from your node_modules folder to avoid global dependency.

Another thing to keep in mind is that the --watch option prevents node-sass to compile unchanged files on startup hence the need of a compile-sass rule for initial compilation.

My npm scripts rules ended up looking like this:

  "scripts": {
    "dev-server": "node ./scripts/start.js && kill $WATCH_SASS_PID",
    "background-sass-watcher": "./node_modules/.bin/node-sass --watch --recursive src -o src & WATCH_SASS_PID=$!",
    "compile-sass": "./node_modules/.bin/node-sass --recursive src -o src",
    "start": "npm run compile-sass && npm run background-sass-watcher && npm run dev-server",
    "build": "npm run compile-sass && node ./scripts/build.js"
  },

@mxstbr mxstbr mentioned this issue Aug 21, 2016
@yordis
Copy link

yordis commented Aug 22, 2016

My 2 cents in the discussion.

The only thing I will support having by default is cssnext just because at the end of the day we are writing CSS from the specs.

@brianberlin
Copy link

My workaround uses node-sass and concurrently. Install both as a dev dependency and modify your start script to ./node_modules/.bin/concurrently -r './node_modules/.bin/node-sass src -w -o src -r' 'react-scripts start' I'm surprised how well this works. If it fails to compile, the error stays in the console until the error is cleared and I found that by adding -r to concurrently it doesn't mangle the original colors.

@iRoachie
Copy link
Contributor

iRoachie commented Sep 8, 2016

Is there any reason flags can't be supported. For instance create react app --sass. There's no harm in having the option and it wouldn't make the project confusing for new people. Those that want sass, just add the flag. Most dev cli's have flag options for additional functionality.

@gaearon
Copy link
Contributor

gaearon commented Sep 8, 2016

This is my personal opinion so please don’t take this as an official position of React team or something like that. Here goes.

Adding any flag explodes the number of possible configuration we need to support. The day we add less-loader and sass-loader, many of these issues will become our issues because they affect our users in the configuration we recommended:

We take issues in the underlying libraries very seriously. We follow up with them and make sure they get fixed, fork or replace the dependencies, if necessary, so that our users have the best experience. But we are not ready to officially support the spectrum of CSS transpilation alternatives.

Unlike, for example, Babel plugins, we don’t use SASS or Less at Facebook, so we have no idea which issues are affecting the users, and would not be able to allocate enough time and effort to fix them.

Unlike with webpack, we wouldn’t get to constrain the features you use to a tightly controlled subset so that we could ensure they work well together.

Additionally SASS (the most commonly requested tool in this thread) is a wrapper over a native binary written in C/C++ which adds more rare but complex issues that we wouldn’t be able to help anyone with. If you never had any issues with these tools, it’s great. It is however not cool if somebody attempts to run a React example built with an official React toolkit, and they get a segmentation fault.

At this point in time we are going to keep treating CSS transpilation as a problem outside of CRA concern. As demonstrated above you can use SASS/Less or something else in a CRA pre-ejected app if you treat .css as a compilation target that you import, and have a SASS/Less watcher running in background, just like this worked before Webpack. But we are not ready to recommend either of those solutions officially, even as we understand that many in the community use them.

Also, as I said previously, we intend to look into this space (styling in React) later this year. If our effort is unfruitful, we might give in and enable SASS/Less by default, but this will require Create React App to further mature and prove its value anyway.

Let’s revisit this topic in the beginning of 2017.

@facebook facebook locked and limited conversation to collaborators Sep 8, 2016
@gaearon
Copy link
Contributor

gaearon commented Feb 12, 2017

We now have official Sass integration documentation.
Feel free to contribute if something is missing!

For now I think this is enough, and covers the use cases most people have.

@gaearon
Copy link
Contributor

gaearon commented Jan 14, 2018

We're exploring a tighter integration in the spirit of what most people in this thread wanted (just import it and it works). But we'll need some changes on sass-loader side to make this happen. If you're interested, please subscribe to this discussion: webpack-contrib/sass-loader#532.

@gaearon
Copy link
Contributor

gaearon commented Oct 2, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests