search

ro-savage / react-scripts-cssmodules

Enable CSS Modules for Create-React-App using the official CRA api
https://www.npmjs.com/package/react-scripts-cssmodules
47 stars 5 forks source link

Some questions #3

Closed stereobooster closed 6 years ago

stereobooster commented 6 years ago

First of all thank you for creating it.

  1. I wonder why there are two css files not one? Like
476 B     build/static/css/main-cssmodules.96cb6ae4.css
237 B     build/static/css/main.8d62f548.css
  1. Why in production build class names are so long? Like
.src-components-__Wrapper-module___Wrapper
  1. Do you have opinion on https://github.com/kriasoft/isomorphic-style-loader? It seems it is able to do critical CSS for SSR.
ro-savage commented 6 years ago

Thanks for the questions.

1) The first is any styles created using CSS Modules, the second is for any normal styles. In theory, if you are only using css modules, or only normal styles. You should just get one css file. (But I haven't checked, but probably should). The reason we had to create two, was because we needed to have different webpack ExtractTextPlugin configs.

2) To allow classnames to be targeted, we are using the file dir__filename___classname. This is unique, will also be the same, allows you to easily decipher where a component came from in your directory, and with gzip should be the same as using a short hash.

3) No opinions, if it what you're looking for and you are willing to eject use isomorphic-style-loader. This repo is meant to align with the future cssmodules in Create-React-App which is something that just works but isnt complicated/bleeding edge.

stereobooster commented 6 years ago

To allow classnames to be targeted

What do you mean? Targeted for testing?

This is unique, will also be the same, allows you to easily decipher where a component came from in your directory

There are sourcemaps for it. Or not?

and with gzip should be the same as using a short hash.

Well yes, but browser still needs to parse all characters, it can not do this like gzip

ro-savage commented 6 years ago

What do you mean? Targeted for testing?

Yes targeted for testing, for overwriting with non-css module styles, and for non-react JS.

There are sourcemaps for it. Or not?

Yep. It isn't necessary, it was just a free (since we needed the whole directory to ensure it classname was unique

Well yes, but browser still needs to parse all characters, it can not do this like gzip

This is true. I didn't really worry / think about that because its pretty minimal (compared to say all stuff babel puts in). But it is still overhead that isn't needed.

One option I considered in the past was using the directory structure to generate a hash. So it would look the same as the standard naming convention but be deterministic based on directory structure. I will look into doing this again for the next release (when CRA next gets updated).

stereobooster commented 6 years ago

Yes targeted for testing, for overwriting with non-css module styles, and for non-react JS

I imagine how Dan Abramov jumping out here in saying "you should not depend on internal implementation, otherwise it can break in the future".

If you need classes for testing, for overwriting with non-css module styles, and for non-react JS. You should not use for this classes generated by CSS Modules.

It would be nice to have "automatic atomic styles", like styletron has. It is theoretically possible with CSS Modules and should help with compression a lot.

ro-savage commented 6 years ago

Closing issue, as we wont be doing any changes in this repo. Feel free to bring up any ideas/suggestion directly in CRA, as v2 will contain CSS modules by default.