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 - Part 2 #2498

Closed
BrodaNoel opened this issue Jun 8, 2017 · 29 comments
Closed

Optional Sass support - Part 2 #2498

BrodaNoel opened this issue Jun 8, 2017 · 29 comments
Milestone

Comments

@BrodaNoel
Copy link
Contributor

As I just read in #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
Copy link
Contributor

cr101 commented Jun 8, 2017

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

@BrodaNoel
Copy link
Contributor Author

BrodaNoel commented Jun 8, 2017 via email

@cr101
Copy link
Contributor

cr101 commented Jun 8, 2017

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
Copy link
Contributor Author

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
Copy link

yeluolei commented Jun 9, 2017

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

@ruler47
Copy link

ruler47 commented Jun 19, 2017

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

@timarney
Copy link

@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
Copy link
Contributor

Timer commented Jun 19, 2017

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

@gaearon
Copy link
Contributor

gaearon commented Jun 26, 2017

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
Copy link
Contributor

viankakrisna commented Jun 26, 2017

configComposer from #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
Copy link
Contributor

gaearon commented Jun 26, 2017

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
Copy link
Contributor

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
Copy link

KaRkY commented Jun 30, 2017

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
Copy link

xajhqffl commented Jul 1, 2017

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
Copy link

danieldram commented Oct 2, 2017

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
Copy link

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
Copy link
Contributor

gaearon commented Jan 14, 2018

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

@gaearon
Copy link
Contributor

gaearon commented Jan 16, 2018

If webpack-contrib/sass-loader#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
Copy link

miraage commented Jan 17, 2018

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:

  • npm link react-scripts from my fork
  • Set dependency as "react-scripts": "file:/path/to/my/fork/packages/react-scripts"

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
Copy link

aurelienshz commented Jan 21, 2018

I tried to quickly put something together: aurelienshz@8d679a4, based on the work done by @miraage. I feel like this could be worth exploring in case webpack-contrib/sass-loader#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 webpack-contrib/sass-loader#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! 🙂

@justinlevi
Copy link

justinlevi commented Feb 2, 2018

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
Copy link

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
Copy link

@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
Copy link

ghost commented Feb 13, 2018

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
Copy link
Contributor Author

BrodaNoel commented Feb 13, 2018 via email

@bjankord
Copy link

@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. 👍

@Fabianopb
Copy link
Contributor

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

@Timer
Copy link
Contributor

Timer commented Mar 26, 2018

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

There's an open PR now!

@Timer Timer closed this as completed Mar 26, 2018
@samuelcolvin
Copy link

#4195

@lock lock bot locked and limited conversation to collaborators Jan 20, 2019
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