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

Nicer css module generated names #90

Closed
modernserf opened this issue Jul 22, 2016 · 40 comments
Closed

Nicer css module generated names #90

modernserf opened this issue Jul 22, 2016 · 40 comments

Comments

@modernserf
Copy link

You may not even realize this, but your current config supports css modules, it just doesn't use them by default -- i.e. you can use :local in CSS files and it will generate class names, it just doesn't make all class selectors local by default.

I'm totally fine with global being the default scope for now (much less confusing for beginners) and adding the :local pseudo-class where I need it, but it would be nice if more human-readable classes were generated. Use something like:

style!css?localIdentName=[path][name]---[local]---[hash:base64:5]!postcss

as the loader config in webpack.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

Can you please explain this on an example? We don’t “officially” support advanced features but they indeed may work by accident 😄 . I don’t know enough about CSS modules to understand what you’re saying, but we’d rather not make them easier to use because we want to be sure that everybody has the same setup. Otherwise can’t just release updates that switch to another CSS solution, for example.

@mxstbr
Copy link
Contributor

mxstbr commented Jul 22, 2016

True, very good catch @modernserf! I'm +1 for this change.

@gaearon css-loader supports the :local wrapper for class names to create localized classes. (i.e. css modules by effort)

// This is now usable like if it was CSS modules! 
:local(.button) {

} 

We didn't specify how it should generate those classes, so they're quite ugly and unreadable – what @modernserf is suggesting is making them more readable.

@corbanbrook
Copy link

Simply using [local]---[hash:base64:5] is nice solution. This will output classnames like button---2VzVQ. Including the path and filename is a tad excessive imo and just makes it difficult to grok when scanning over the outputted markup.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

I don’t think we want to support this. I would actually like to turn this feature off.

The reason is we might want to migrate away from webpack in the future. We’d need to make our CSS files are as vanilla CSS as possible (aside from minor stuff like relative dependency paths).

I would accept a PR that forbids extra features if possible.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

(Btw I totally understand the benefits of local scoping. We just don’t think it’s a good default for people learning React, and it’s way too custom syntax that is incompatible with many other tools. We’d rather move into direction of encouraging CSS in JS for better encapsulation.)

@mxstbr
Copy link
Contributor

mxstbr commented Jul 22, 2016

I think it's not possible to turn off local scope by default (ref webpack-contrib/css-loader#193), but we can simply set the generated name to whatever the name is anyway. Give me 5 minutes to try that.

@modernserf
Copy link
Author

I regret bringing this to your attention.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

@modernserf I understand your frustration but it’s still better than if we change the bundler in a few months and all your styles would break. Having to back out of infrastructure changes (e.g. super fast new bundler) because people relied on webpack in ways like this would be super sad.

@mxstbr
Copy link
Contributor

mxstbr commented Jul 22, 2016

I kind of agree with @modernserf though, this is a nice escape hatch for people to explicitly use CSS modules without needing to eject. It's not like we document or encourage this anywhere…

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

People will use it whether we document it or not, and then “create-react-app broke my app” will become the next javascript f*****e.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

A semver-compatible patch update can literally break people’s builds because of it.

@mxstbr
Copy link
Contributor

mxstbr commented Jul 22, 2016

I still haven't found a way to disable it properly, it'll break peoples builds either way.

@gaearon
Copy link
Contributor

gaearon commented Jul 22, 2016

We can add a custom loader that throws on non-standard syntax.

@mxstbr
Copy link
Contributor

mxstbr commented Jul 22, 2016

Submitted #97 not in the hope of it getting merged but to have some code to talk about

Why would we not add that option to css-loader? That sounds easier than writing our own… (not 100% what css-loader actually does though)

@slmgc
Copy link

slmgc commented Jul 23, 2016

@modernserf you could try https://github.com/insin/nwb, it's a similar solution which runs out of the box but also has many additional config options if you need them.

@ForbesLindesay
Copy link
Contributor

I would think we'd want to/be able to support css-modules in any future version of this tool even if we move away from webpack in the future.

@gaearon
Copy link
Contributor

gaearon commented Jul 25, 2016

I would think we'd want to/be able to support css-modules in any future version of this tool even if we move away from webpack in the future.

I’m not opposed to CSS modules in the future. I only think we need to let this tool live for a while with simple defaults (vanilla CSS is as unopinionated as we can be). When we are ready to focus on figuring the best way to do styling in React apps (both externally and internally), we can revisit this decision. However I wouldn’t want the users’ apps to use inconsistent notations in .css files because they would be hard to codemod (if we ever provide that codemod of course).

@mxstbr
Copy link
Contributor

mxstbr commented Jul 25, 2016

I would think we'd want to/be able to support css-modules in any future version of this tool even if we move away from webpack in the future.

Yeah, according a twitter poll I ran yesterday (130 RTs, 1600 respondents!) CSS Modules is almost as popular as plain CSS (inc. Sass/Less/…) and way more popular than inline styles and CSS-in-JS combined. (this is biased because it's my twitter followers, but 1600 respondents…)

That doesn't mean we should be implementing them, but it does mean that any new bundler that doesn't support them is going to have a hard time getting any traction at the moment. That might mean it's fine to leave them in, especially in combination with our "You never need to update create-react-app, whatever you version you're on works" Mantra.

EDIT:

When we are ready to focus on figuring the best way to do styling in React apps (both externally and internally), we can come revisit this decision.

Fair enough.

@ForbesLindesay
Copy link
Contributor

When we are ready to focus on figuring the best way to do styling in React apps (both externally and internally), we can come revisit this decision.

Maybe... I think the problem css modules solve is going to come up pretty quickly for users, but I do agree that css-modules don't seem hugely mature right now.

FWIW I much prefer the idea of something like .module.css rather than special :local syntax within the file. It also would be possible to codemod out in the future, albeit quite a lot of work.

@gaearon
Copy link
Contributor

gaearon commented Jul 25, 2016

FWIW I much prefer the idea of something like .module.css rather than special :local syntax within the file.

Agree that this seems better if we want to allow it.

@geelen
Copy link

geelen commented Jul 27, 2016

Just chiming in here. I definitely agree the :local syntax should not be supported as it is. That was an early decision in CSS Modules land where I wanted to make the ?modules option to css-loader change as little as possible (it just enables one extra postcss transform that wraps everything that's not :global in :local). But that behaviour is pretty unexpected, and I'd be surprised if many people are making use of :local at all these days (CSS Modules feels like an all-or-nothing thing for most people). So I'd be in favour of removing :local altogether, but in the interests of expedience I think the linting rule is better.

The developer API for this stuff has never been great, but I've now dug myself out a few weeks to work on it so I'm looking at improving it now. That discussion probably should take place elsewhere though.

❤️❤️❤️ to this project, just quietly.

@oscar-b
Copy link

oscar-b commented Sep 28, 2016

When we are ready to focus on figuring the best way to do styling in React apps (both externally and internally), we can revisit this decision. However I wouldn’t want the users’ apps to use inconsistent notations in .css files because they would be hard to codemod (if we ever provide that codemod of course).

@gaearon What do you mean with "inconsistent notations in .css files"? My .css files using CSS Modules are 100% vanilla CSS, the magic is done on the fly by webpack and not in the .css. Same thing as referencing an image with import, it just returns a string value pointing to the new unique class name. I would say that this way of treating css is much more consistent with the way create-react-app recommends handling images and fonts.

If you were to go away from using CSS Modules, it's not much that needs to change:

import styles from './Button.css';
<button className={styles.Button} />

would change into

import './Button.css';
<button className="Button" />

This is already dependent on custom behaviour of import by webpack, as noted in the docs.

I think CSS Modules are a much better way of handling css in React than the suggested way of manually namespacing your classes, especially for a newbie not realising that an imported css will be global to all components.

@gaearon
Copy link
Contributor

gaearon commented Sep 28, 2016

By “inconsistent notation” I mean extensions like composes and :global that are part of CSS Modules.

@oscar-b
Copy link

oscar-b commented Sep 28, 2016

Oh yes, agree about those. Never used them.

@fourcolors
Copy link

I'd like to use CSS Modules... just saying...

@craigcosmo
Copy link

craigcosmo commented Oct 14, 2016

@gaearon why do you want to migrate away from webpack in the future? What tool would help bundling code?

@ddemolina
Copy link

@Gaeron (Btw I totally understand the benefits of local scoping. We just don’t think it’s a good default >for people learning React, and it’s way too custom syntax that is incompatible with many other tools. >We’d rather move into direction of encouraging CSS in JS for better encapsulation.)

Is https://github.com/rtsao/csjs moving in that direction? Do you think is an alternative to use nowadays in CRA?

@geelen
Copy link

geelen commented Oct 31, 2016

By the way this whole topic lead me to write a pure JS version of CSS Modules (kinda) called Styled Components. I'm now using it with CRA in my own projects and it's ace. 👌

@kurtharriger
Copy link

Personally I don't know enough about css modules to know if they are good or bad thing--but I do know that some third party libraries one might like to use, such as react-toolbox, require it to be enabled.
react-toolbox/react-toolbox#1054

Thankfully there is the option to eject, but it would be nice if one could configure some sort of webpack config transform to allow replacing parts of the webpack config without fully ejecting perhaps these transforms could be plugins to allow one to enable things like sass or less if they want.

@odigity
Copy link

odigity commented Jan 14, 2017

I'm fairly new to React, and just learned about CSS Modules a few days ago, so I can't speak with great confidence, but it seems like CSS Modules is just obviously better than everything that has come before (much like React in the domain of UI frameworks).

I've been working on my first React app over the last week, a process made easier by the existence of create-react-app, but the inability to start using CSS Modules means I must now pause and learn how to operate Webpack and Babel myself so that I will no longer have to rely on CRA. I was hoping to put that off longer, and could have, if not for the lack of CSS Modules support.

I agree that automatically scoping plain CSS would be an unpleasant and confusing surprise to beginners, but if there's any way to avoid that while still making it accessible to those who want to opt-in (perhaps by using a different file extension like .csm), then I think it should be pursued.

@oscar-b
Copy link

oscar-b commented Jan 14, 2017

@odigity look at https://styled-components.com/ instead, thank me later ;)

@bondz
Copy link
Contributor

bondz commented Jan 14, 2017

@geelen @oscar-b just checked out styled-components really cool. 👍

@odigity
Copy link

odigity commented Jan 15, 2017

@oscar-b It's on my short-list of study topics for the next two days.

@DenisIzmaylov
Copy link

DenisIzmaylov commented Mar 27, 2017

Last 3 years we've developed a lot of large projects for commercial and Enterprise segments using React. We've tried everything including LESS, Stylus, CSS modules, styled-components, SASS and we found that JSS have the best DX ever. With using react-jss by Dan.

With CSS you can control your styles by CSS class only while in JSS you can control every property, every value to reduce unused CSS as well as it possible.

But there is only one problem in JSS - is learning curve. It may be hard to switch from CSS notation to JSS in a few days or even couple weeks.

To solve this problem we've made PreJSS - universal CSS to CSS-in-JS adapter which is actually became platform and ecosystem like webpack or babel.

PreJSS have a three basic concepts PreJSS Styles Declarations, CSS Parser, PreJSS adapter.

image

  • PreJSS Styles Declarations - is CSS styles in Literal Template Strings like we love:

    import preJSS from 'prejss'
    
    const styles = preJSS`
      $fore-color: #fff;
      $bg-color: #444;
      .button {
        color: $fore-color;
        background: $bg-color;
        font-weight: ${props => props.isPrimary ? 'bold' : 'normal'};
      }
    `;
  • CSS parser is an optional package with function which takes CSS/SCSS/any code and transforms it to JSS format. By default PreJSS uses PostCSS, but there is a lot of other nice CSS parsers.

  • CSS adapter is main abstraction, set of 3 independent functions - prepare (e.g. remove JS comments from your styles), parse (by CSS parser) and finalize to adopt JSS to any custom JSS library such as Radium, React Native, React Desktop, etc.

That's how PreJSS makes CSS world simpler:

image

@cr101
Copy link
Contributor

cr101 commented Mar 27, 2017

@DenisIzmaylov Since styled-components was your inspiration for PreJSS why didn't you make improvements to styled-components instead of creating yet another CSS plugin to style your components?
Also, what are the benefits of using PreJSS over styled-components?

@ro-savage
Copy link
Contributor

@gaearon @Timer
I know there has been gone through before, I personally use a fork of react-scripts to handle css modules.

However now v1.0.0 is out and using webpack 2.0 (e.g. not likely to change soon). I suggest revisiting CSS Modules.

My current config supports both

  • if the css file has .modules.css (/\.modules.css$/) then it's loaded as a CSS module,
  • if its anything else () it imported as regular css (/[^\.modules]\.css$/).

This allows both, its an extra only people who choose to use get, doesn't interfere with loading CSS from libs, with webpack2 now in place I don't think CSS Module support is going to die any time soon, and of course CSS Modules rocks according to stateofjs with 97% satisfaction.

Thoughts?

@gaearon
Copy link
Contributor

gaearon commented May 19, 2017

Can you file a new issue about this? Let’s say I’m open to supporting it with an explicit filename suffix.

@czebe
Copy link

czebe commented May 19, 2017

@ro-savage thanks for your input, this sounds good, I'll try your config.

@ro-savage
Copy link
Contributor

ro-savage commented May 20, 2017

@gaearon @czebe
CSS Module Support: Issue #2278 and PR #2285

@faceyspacey
Copy link

faceyspacey commented Jun 29, 2017

my 2 cents: to me the biggest thing is having cacheable traditional stylesheets and avoiding the render/runtime overhead of calculating styles in every render (which you need to do on both the client and server if doing SSR--not good). I avoid true css-in-js solutions for this precise reason.

So a solution that could be great for CRA--and one it doesn't need to implement itself--is a loader that removes css-in-js and converts it to stylesheets. That can be done far down the line in webpack land, long after you have ejected. Someone just needs to make this.

That way during development CRA doesn't need to support anything new. And that way developers can have the option to achieve optimum performance if they ever want it.

Static css-in-js--the new hotness lol

the one caveat is that your css-in-js would have to be static like regular css modules, but that's what you require for the aforementioned performance reasons anyway. This comes full circle coupled with code-splitting and multiple stylesheets, as ur also sending far less total bytes related to css then the css-in-js solutions du jour which ultimately incorporate both your styles in ur js chunks AND send render-path css.

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