facebook / create-react-app

Set up a modern web app by running one command.
https://create-react-app.dev
MIT License
102.67k stars 26.84k forks source link

Optional Sass support - Part 2 #2498

Closed BrodaNoel closed 6 years ago

BrodaNoel commented 7 years ago

As I just read in https://github.com/facebookincubator/create-react-app/issues/78, CRA don't want to add support to Sass and Less.

Then, I read Dan saying:

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

So, what's the state of this? I few hours ago I implemented Sass and Less in a personal project out of CRA. I just added a new rule in webpack modules, and a couple of dependencies and Gualá! Following this example, we should do not deal too much with this implementation: https://github.com/webpack-contrib/sass-loader#examples

Something really nice in those loaders is that it's not necessary to convert Sass to CSS explicitly. So, ~as all loaders do~, just including the .scss file is enough.

I read tons of times that you repeat the same question: "Why you guess Sass is necessary in React?". Well, in my case, I don't use Sass to reutilize CSS. I use Sass to write CSS as a tree, and it's useful to define vars that are useful to create a template structure in the site/component.

My best example (tree view, vars, cals, and imports that can be used to define the whole site styling):

@import './base/colors.scss';

.Xxxxx {
  $width: 30px;

  position: fixed;
  right: 10px;
  bottom: 10px;
  width: $width;
  height: $width;
  border: 1px solid $color-primary;
  border-radius: 5px;
  cursor: pointer;

  > i.anticon-smile-o {
    color: $color-primary;
    font-size: calc(#{$width} * 0.60);
    line-height: $width;
  }
}

If you think it's time to add Sass, I can create a PR with those chances (or, at least, I'll try). If not, ok.

Thank you!

cr101 commented 7 years ago

@BrodaNoel CRA added SASS support a while ago if that's what you are asking for.

BrodaNoel commented 7 years ago

Nop. That's support for pre-processors (from Sass to Css), and then You have to include the CSS file, instead of the .scss.

I'm asking if we can add the Sass loader to our Webpack config, so then will not be necessary to do those changes in package.json, or install new dependencies (as you can read in that doc you shared)

El 8 jun 2017, a las 02:45, Cristian Rosescu notifications@github.com escribió:

CRA has been providing SASS support for a while now

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

cr101 commented 7 years ago

I'm asking if we can add the Sass loader to our Webpack config, so then will not be necessary to do those changes in package.json, or install new dependencies (as you can read in that doc you shared)

I'm with you now. I agree that being able to import .scss files would be awesome and very helpful. Most popular CSS frameworks like Bootstrap 4, Foundation Sites, Materialize, etc all use SASS. And being able to only import the SASS components you need instead of the entire CSS framework would greatly reduce the size of the pages by only shipping the code that you need.

BrodaNoel commented 7 years ago

Absolutely! I have the same problem with ant.design, but at least ant has all CSS isolated (for each component), so I have no that size problem. But yes, I would be really great if we have the Sass support.

yeluolei commented 7 years ago

need for Less support too if you want to use ant.design

ruler47 commented 7 years ago

So someone is trying find solution for this issue? Is it worth waiting for the support of scss?

timarney commented 7 years ago

@BrodaNoel @ruler47 This code needs to be updated to support react-scripts 1.0 but is this the sort of thing your looking for https://github.com/timarney/react-app-rewired/blob/master/packages/react-app-rewire-sass/index.js

Might be a workaround to get you what you need for now.

Timer commented 7 years ago

@ruler47 I'd like to note that it is not required to eject to use Sass. Please read the following guide.

gaearon commented 7 years ago

I'm open to supporting Sass but only if people not using it don't have to get sass-loader or node-sass installed as part of react-scripts. This is similar to my concern about supporting Relay.

These use cases call for a plugin system but I don't think we're ready to support a whole addon ecosystem that's one step removed from webpack. So this is where it breaks down a little.

Suggestions?

viankakrisna commented 7 years ago

configComposer from https://github.com/facebookincubator/create-react-app/issues/2449? 😜

*presed comment prematurely.

I'm thinking about https://github.com/webpack-contrib/npm-install-webpack-plugin but then, the config should be resolved too

gaearon commented 7 years ago

Another concern is that I want config to be simple for people who eject. I don't want Sass to be there if they don't use it, but I also don't want Sass users to end up with an abstraction there.

viankakrisna commented 7 years ago

It's still configurable (with the same webpack API) if we place the resolvers in the config folder though. Just that it's in another place with resolving and installing logic. Like you said, it's still one step removed from regular webpack configuration object.

KaRkY commented 7 years ago

When can we expect a solution for this to come out? Because this is a blocker for me and I can not find suitable replacement for CRA that is actually maintained.

xajhqffl commented 7 years ago

why don't you add an option for adding sass/less to the project when create a new project like: create-react-app my-app --style=sass otherwise, the default should still be css and not including any additional sass loaders.

This was used similarly in angular-cli project, link here

danieldram commented 7 years ago

I can't think of any major production project that doesn't use SASS or LESS. It doesn't make sense why this wouldn't be included in a solution designed to help development into production..

samuelcolvin commented 6 years ago

For anyone else struggling to understand why sass isn't support out of the box and trying to add it - you can add support using react-app-rewired and config-override.js:

module.exports = function override (config, env) {
  config.module.rules[1].oneOf.splice(config.module.rules[1].oneOf.length - 1, 0,
    {
        test: /\.scss$/,
        use: ['style-loader', 'css-loader', 'sass-loader']
    },
  )
  // console.dir(config, { depth: 10, colors: true })
  return config
}

With that you can import sass with just import './main.scss'. See sass-loader for more docs.

You'll need sass-loader and node-sass installed, this was tested with CRA installed today (2017/12/11).

gaearon commented 6 years ago

Here's my proposal for how we can include Sass without compromising on user experience for people who don't want it: https://github.com/webpack-contrib/sass-loader/issues/532

gaearon commented 6 years ago

If https://github.com/webpack-contrib/sass-loader/pull/533 gets in we have a relatively neat way of doing this. In that case the next action item here would be to ask for an alpha of sass-loader to be published with that change, and then we’ll be ready to take a PR to CRA that supports it.

miraage commented 6 years ago

I have some stuff in my backpack. I was forced to add node-sass to react-scripts because when I add it to project, CRA is not able to find it for some reason.

I was testing my project via:

Neither did work.

After adding node-sass to react-scripts it worked perfectly fine.

I see an issue that developer might not want to use Sass in the project, while he or she will get a redundant dependency.

I created these commits from master branch. I can update to next if needed.

https://github.com/miraage/create-react-app/commit/751d15ed59a7e7a9ba29bc5b6c9bdd98a5e3444d

https://github.com/miraage/create-react-app/commit/4aebe2349514fee9c75ea85fbea31ea271b73dc9

aurelienshz commented 6 years ago

I tried to quickly put something together: https://github.com/aurelienshz/create-react-app/commit/8d679a429e2c1701a317ff51f8e2443d072ba002, based on the work done by @miraage. I feel like this could be worth exploring in case https://github.com/webpack-contrib/sass-loader/pull/533 doesn't make it, or in any other situation where an opt-in loader is needed.

The idea is pretty much the same as what @gaearon described in https://github.com/webpack-contrib/sass-loader/issues/532, but using a custom wrapper around sass-loader. I'm not really sure how reliable this would be, and it also probably implies a performance hit, but the main idea is there. Suggestions welcome! :slightly_smiling_face:

justinlevi commented 6 years ago

I tried the solution proposed by @samuelcolvin and the sourceMap does not seem to show the actual SCSS file where the styles originate.

I also tried the node-sass-chokidar solution but the sourcemaps don't work correctly either.

scripts": {
    "start": "npm-run-all -p watch-css start-js",
    "start-js": "react-scripts start",
    "build": "npm run build-css && react-scripts build",
    "build-css": "node-sass-chokidar --output-style nested --source-map-embed true --source-map true --source-map-root ./src/styles --include-path ./src/styles src/styles/ -o ./src/styles/",
    "watch-css": "npm run build-css && node-sass-chokidar --output-style nested --source-map-embed true --source-map true --source-map-root ./src/styles --include-path ./src/styles src/styles/ -o ./src/styles/ --watch --recursive",
    "sass": "sass --watch src/styles/scss:src/styles/css",
    "test": "react-scripts test --env=jsdom",
    "test:debug": "react-scripts --inspect-brk test --runInBand --env=jsdom",
    "eject": "react-scripts eject"

Working with SASS without a sourcemap solution is far less efficient from my experience. I'm enthusiastic for an opt-in solution that might include a working sourcemap solution without ejecting.

samuelcolvin commented 6 years ago

SASS without a viable sourcemap solution really makes this workflow untenable.

Unnecessarily hyperbolic, sass without source maps isn't perfectly but it's completely usable.

justinlevi commented 6 years ago

@samuelcolvin Sincere apologies. I went ahead and updated the comment. Super appreciative of this project and all the work that everyone has done here.

ghost commented 6 years ago

Seems like what Next.js has implemented: https://github.com/zeit/next-plugins/tree/master/packages/next-sass#usage

Thanks for the effort to bring this to CRA, the current workflow is really confusing and I find myself often editing the .css just to realize there was a .scss on its side.

BrodaNoel commented 6 years ago

Hide the .css file in your text editor, for the current proyect.

El 13 feb 2018, a las 03:57, franciscop-invast notifications@github.com escribió:

Seems like what Next.js has implemented: https://github.com/zeit/next-plugins/tree/master/packages/next-sass#usage

Thanks for the effort to bring this to CRA, the current workflow is really confusing and I find myself often editing the .css just to realize there was a .scss on its side.

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

bjankord commented 6 years ago

@gaearon If sass/scss support is added to create-react-app, I'd recommend taking a page from rails/webpacker and check for .module.sass and .module.scss to run the sass/scss files through css-loader with CSS modules turned on after it runs through the sass-loader. :+1:

Fabianopb commented 6 years ago

For the record, I've been discussing with the contributors of CRA and I'm preparing a PR for that atm!

Timer commented 6 years ago

Sass support is coming. I'll close this, but it'll be released in 2.0.

There's an open PR now!

samuelcolvin commented 6 years ago

4195