Skip to content
This repository has been archived by the owner on Jan 16, 2021. It is now read-only.

Unify to have 1 way of handling scss #119

Closed
neo opened this issue Jun 1, 2018 · 5 comments
Closed

Unify to have 1 way of handling scss #119

neo opened this issue Jun 1, 2018 · 5 comments
Labels
cra@v2 This issue will be resolve by [email protected] discussion this is/requires discussion

Comments

@neo
Copy link
Member

neo commented Jun 1, 2018

I notice there are two ways we are handling scss now:

  • Injecting sass-loader in to webpack with rewired
  • watch scss files to generate css files and import the css file

Each of them has pros and cons but I think we might want to agree to using one and stick to it. 🤔

@neo neo added the discussion this is/requires discussion label Jun 1, 2018
@iranreyes iranreyes reopened this Jun 1, 2018
@iranreyes
Copy link
Member

@neo Could you explain your pros and cons here?

@neo
Copy link
Member Author

neo commented Jun 1, 2018

Sure but my list wouldn't be the complete list:

sass-loader:

  • not recommended by create-react-app as it's not generated to work in the future, but we're using rewired anyway... so it might be ok?
  • but it would save the actual css files.

watch sass on the side:

  • recommended by create-react-app
  • could use the global.css in storybook (I'm sure we'll find a way to do it with sass-loader), but now the story book is looking for global.css

These are what I could think of from the top of my head, I'm sure there should be more; please do add more in the comments

@wongbsn
Copy link
Contributor

wongbsn commented Jun 1, 2018

sass-loader is already in for react-scripts 2: facebook/create-react-app#4195

Might be better to go with that?

@neo
Copy link
Member Author

neo commented Jun 1, 2018

great! can't wait for the v2

@neo
Copy link
Member Author

neo commented Jun 7, 2018

I guess it's settled, we just have to wait for version 2 to come out.

@neo neo closed this as completed Jun 7, 2018
@neo neo added the cra@v2 This issue will be resolve by [email protected] label Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cra@v2 This issue will be resolve by [email protected] discussion this is/requires discussion
Projects
None yet
Development

No branches or pull requests

3 participants