CSS Modules localIdentName cleanup

[lnhrdt]

Cleaning up CSS Modules localIdentName in dev and prod configurations in an attempt to provide ease of use for developers in development and encourage best practices in production.

Context and rationale in the commit messages!

[gaearon]

Looks like the integration tests need to be updated.

[gaearon]

@ro-savage Thoughts?

[lnhrdt]

Looks like the integration tests need to be updated.

Well that's embarrassing.

expected '.style-module-cssModulesInclusion{background:darkblue;color:lightblue;}' to match /.+__style-module___cssModulesInclusion+\{background:.+;color:.+}/

I'll use this as a great opportunity to get to know your e2e tests. 😉

[gaearon]

You can look at where they were added (I assume in CSS Modules PR).

[ro-savage]

TL:DR
I see no need to change it.

And I don't understand why people want a hash over path/filename/classname, except that they are used to it and/or its weird for them to see a long classname. You go from having something useful to humans to having something that is only useful for machines.

---

There is some mis-information in the original PR thread (which I contributed to). Hashes are deterministic as they are hashed from module name and local identifier by default. I also tried to change my hash to test this, and couldn't do it with changing filenames.

Why does it need to be deterministic and targetable? kind of kills the whole module thing.

Hashes are deterministic. I don't understand how it affects being modular at all. Your classes name are matched with your components, and never affect anything outside outside of where its imported.

That's another example. Using a hash is be fine by me, and the mentioned use cases can still be addressed with static classes or e.g. data attributes.

We already have classes with can use for this. Why add another layer of complexity (without a benefit).

I agree that we should recommend people decouple styling classes from identifiers for testing or the tools like the ones @sompylasar mentioned. The fact that we used styling classes for so many years as hooks for these tools wasn't because they were actually related concerns, it was just all we had.

Hashed name can still be targeted exactly the same. But beyond that, why would we artificially make it harder for people to do their job or write tests? Their are already so many barriers to writing tests. Why add another one. If people want to use a data-attribute we aren't stopping them.

---

The only reason that made sense (to me) was smaller file sizes. I ran the difference on one of my projects.

We use className as css modules 133 times.
The gzip difference between [path]__[name]___[local] and [hash:base64:5] is 109.33 KB (-241 B) in the JS 9.5 KB (-501 B) in the CSS.

Parsing theoretically takes longer, but when I tried to measure it, the random changes in parsing time were much greater than any affect changing the css names had. So I was unable to measure any real-world difference.

What has been great about using non-hashed names, is using the developer elements/html tab, it has been extremely easy to identify components. Reasoning about and talking about targets as something that makes sense in english vs a random string is much easier also.

Having only [hash:base64:5] appears to me to remove value from the developer experience, in return for a <1% decrease in bundle size. Having different values for classnames between dev & prod also hinders the developer experience.

The compromise would be [name]_[local]__[hash:base64:5] as a developer can still identify the the component but the classname is shorter. However this results in an increase in file size because the hash can't be gzipped while [path] can be. (Again on the same project using the above name+hash causes an increase of 443 B in JS and +424 B). So I don't know why you would do it.

@geelen may have thoughts and has more expertise than me.

File	[path][name]_[local]	[hash:base64:5]	[name]_[local]__[hash:base64:5]
JS	109.56 KB (0 B)	109.33 KB (-241 B)	109.99 KB (+443 B)
CSS	9.99 KB (0 B)	9.5 KB (-501 B)	10.41 KB (+424 B)

[ro-savage]

This would allow clashes in names. Its unlikely, but could happen, so add it as a possibility?

Having difference between the final output and dev, can also cause friction as it makes it much harder to look at the production site and find the issue when running in dev mode because all your class names are unrelated.

[stereobooster]

Agree I would leave it as is

[ro-savage]

This is still deterministic, it is just smaller and no longer useful for a human to read/look at.

[stereobooster]

it is just smaller and no longer useful for a human to read/look at

Yes this is exactly the point. Developers should not use those values. And it is nice that they are small.

[AleksandrZhukov]

Developers should not use those values.

Sometimes you have to overwrite some styles but you have no access to this this component (eg component library), so the only way that you have - is to use this localIdentName, eg [class*="foo__bar__baz"]

Will be better to have ability to configure this

[brentmclark]

To add to @ro-savage's point, containerized deploy systems with immutable builds make the issue of obfuscated classnames quite a bit worse in my experience. Everything that doesn't run on the local machine gets prod classnames while not actually being prod. This leads to increased troubleshooting times in many cases, and a steeper learning curve for developers who haven't worked in this setup yet.

For me and any team I've worked on, it would take much more than a fraction of a percent reduction in bundle size to offset these woes.

[sompylasar]

Maintaining two sets of names (component names for development and "public API" names via data attributes for marketing tools) requires twice more effort with unclear benefits. Given we seek consensus, I'd vote for ComponentName__elementName.

[klimashkin]

We tried a lot of different approaches. But ended up with solution which gave us uniq class names in dev and mangled ones in production.

Uniqueness and readability of generated class names we achieved by code structure - all components have uniq file names, like Button.js with Button.css, Link.js with Link.css, etc. So we can use simple [name]_[local] as localIdentName and it is super readable in dev! For example, Button_primary, Link_active, so it close to BEM, but of course without M, by design of css-modules.

Then, for production we additionally use https://github.com/JPeer264/node-rename-css-selectors
as an extra step to mangle long class names into one-two letter ones. Library is really simple and very useful. It saved 30% of our css size!

[giuseppeg]

dev: [name]-[local]-[hash:base32:5] -> ComponentName-root-hash ala SUIT CSS
prod: [hash:base32] -> hash

[jedrichards]

I vote in favour of dev/prod parity in class names. Furthermore readable class names in both environments are useful too. This was a pattern we settled upon independently after some trial and error and running some apps in production, troubleshooting bugs etc.

Also, our component folder and file structure look like,

components/ComponentName/
  index.js
  index.css

So not having [path] in there ends up being quite prescriptive in terms of folder structures and file naming in order to avoid name collisions.

[ro-savage]

None of these ideas are wrong, or worse, in fact most of them are really cool and make a lot of sense.

But CRA is about sensible defaults that minimize learning curves and the need to retain extra configuration knowledge.

With both [path][name]_[local] & [name]_[local]__[hash:base64:5] the only thing someone needs to know to get started with css modules is:

• CSS Modules automatically scopes your CSS, and removed any possibility of clashes
• Name your css file module.css
• Import the css file as styles, and use it as styles.classname.

Thats it, then it just works, theres no clashes, no weirdness, no unintelligible names, no difference between dev and production, no following rules or naming conventions.

Most of the other suggestions introduce complexity, cognitive load and gotchas.

That is fine for your own personal or companies project, where you know the rules and the trade offs but not for something that sells itself as Just create a project, and you’re good to go.

[ro-savage]

For those who want the saving and benefits of shorter names/smaller file size, use @klimashkin excellent method of running your generated CSS and JS through @JPeer264's rename-css-selectors.

[giuseppeg]

fwiw [name]-[local] matches the current naming convention in App.css (SUIT CSS). That's why I suggested it in the first place. In prod you can use source maps.

[patrikholcak]

Having used css modules for quite some time, I can say that using hashes in prod - while it might be good for file size - can result in a really poor debugging experience. I tried using hashes to obfuscate the code a bit and immediately regreted doing that, when sentry reports stopped being readable.

[gaearon]

Another discussion happening at the same time in #3972

[geelen]

@geelen may have thoughts and has more expertise than me.

Plenty of thoughts, not sure about expertise any more!

The only thing I want to add is that collisions are earth-shatteringly bad. Like, fundamentally-undermine-the-premise-of-CSS-modules bad. To that end, the idea of a class .outer in a file called Button.css generating a classname of Button-outer is definitely not sufficient—it's far too easy for clashes to occur (e.g. src/components/LoginScreen/Button.css and src/components/PaymentForm/Button.css).

That said, I'm not convinced that a classname src_components_LoginScreen_Button-outer gives you much human-readability benefits over just Button-outer (you can usually tell from context which "Button" you're looking at), so, to me, the best solution is this:

[name]-[local]:[hash]

(assuming hash uses path or some other method of ensuring uniqueness internally)

This is relatively short, deterministic, vaguely human-targetable ([class*="Button-outer"]), and gives enough context to find your way back to your source code to debug the problem. This is the behaviour we have in Styled Components (using the babel plugin), and it works well.

Or, you know, just use emoji 💫😉💫

[klimashkin]

@geelen Css-modules users usually don't have files with the same name.
You will have
components/Button/Button.js
components/Button/Button.css
components/LoginScreen/LoginScreen.js
components/LoginScreen/LoginScreen.css

And in LoginScreen.css you will have classes for Button.css that override or compose each other using theming, like in https://github.com/javivelasco/react-css-themr

[geelen]

@geelen Css-modules users usually don't have files with the same name.

I don't think that's true. See @jedrichards' comment above:

Also, our component folder and file structure look like,

components/ComponentName/index.js
components/ComponentName/index.css

But even if you're right and most people adopt or could learn that convention, the problem is the severity of the failure—building a component with a CSS processor that "guarantees" no clashes and suddenly you have CSS rules you didn't write and can't track down. Yes, using a single "Button" and applying theming or something is better, for a bunch of reasons, but baking-in a requirement that no two CSS files in your project have the same filename seems like a disproportionate reponse.

[klimashkin]

@geelen @jedrichards
In the beginning of adopting css-modules we also started with

components/ComponentName/index.js
components/ComponentName/index.css

and localIdentName: '[folder]_[local]' which gave us the same result as I described with filenames, but very soon we realized that having bunch of opened files in IDE with the same name is... confusing, even if IDE shows you ComponentName/index.js as tab title. So

components/ComponentName/ComponentName.js
components/ComponentName/ComponentName.css

is a "choice of experience" ; )

After all, css-modules is just a convention, so anyway you have to extend this convention in particular project to achieve human-readable and uniq class names at the same time. I think we found it making file structure a convention too as I described and gone even further with node-rename-css-selectors in production

[gaearon]

Thanks to everybody for the discussion.

I can get behind [name]__[local]:[hash], inspired by #3965 (comment). I agree it strikes a nice balance between all the requirements: debugging, class name length, and programmable targeting without being too obtuse.

Of course hash must be calculated from the full path. (I believe it is in webpack.)

Bonus points if we can "fall back" to the closest folder name if name is index. I think this should be doable by specifying a custom getLocalIdent function as documented here.

[andriijas]

@klimashkin css modules is a way better convention than wierd naming schemes like BEM though.

[gaearon]

Please let's keep this PR on topic.

[ro-savage]

@lnhrdt - as per @gaearon suggestion, you might try something along these lines:

{
  loader: require.resolve('css-loader'),
  options: {
    importLoaders: 1,
    modules: true,
    getLocalIdent: (context, localIdentName, localName, options) => {
      // Use the filename or folder name, based on some uses the index.js / index.module.css project style
      const fileNameOrFolder = context.resourcePath.endsWith('index.module.css') ? '[folder]' : '[name]'
      // Create a hash based on a the file location and class name. Will be unique across a project, and close to globally unique.
      const hash = loaderUtils.getHashDigest(context.resourcePath + localName, 'md5', 'base64', 5)
      // Use loaderUtils to find the file or folder name
      const className = loaderUtils.interpolateName(context, fileNameOrFolder + '_' + localName + '__' + hash , options)
      // remove the .module that appears in every classname when based on the file.
      return className.replace('.module_', '_')
    },
    minimize: true,
    sourceMap: shouldUseSourceMap,
  },
},

[ro-savage]

As per suggestions from Dan, you can try something along these lines for both dev and prod.

{
  loader: require.resolve('css-loader'),
  options: {
    importLoaders: 1,
    modules: true,
    getLocalIdent: (context, localIdentName, localName, options) => {
      // Use the filename or folder name, based on some uses the index.js / index.module.css project style
      const fileNameOrFolder = context.resourcePath.endsWith('index.module.css') ? '[folder]' : '[name]'
      // Create a hash based on a the file location and class name. Will be unique across a project, and close to globally unique.
      const hash = loaderUtils.getHashDigest(context.resourcePath + localName, 'md5', 'base64', 5)
      // Use loaderUtils to find the file or folder name
      const className = loaderUtils.interpolateName(context, fileNameOrFolder + '_' + localName + '__' + hash , options)
      // remove the .module that appears in every classname when based on the file.
      return className.replace('.module_', '_')
    },
    minimize: true,
    sourceMap: shouldUseSourceMap,
  },
},

[gaearon]

We should also "hide" this function into react-dev-utils/getCSSModuleLocalIdent so that it doesn't get duplicated in both places.

[andykenward]

@ro-savage Thank you so much. Was on my todo list to use getLocalIdent to solve the src/component/button/index.module.css or src/component/button/button.module.css class naming convention headache.

[lnhrdt]

What a great conversation this has been, I've learned a lot.

I plan to tidy this up tonight after work based on @gaearon's suggestions and @ro-savage's examples, but if someone beats me to it to keep this moving I wouldn't be offended.

[gaearon]

@lnhrdt

Did you get a chance to look at this yet?

[ro-savage]

@lnhrdt - Would be great opportunity for you to contribute to CRA. However, if you aren't going to have time. Just let me know, and I'll put in the PR for this.

[lnhrdt]

@ro-savage thanks for the bump. I'll push something up this weekend. 👍

[ro-savage]

@lnhrdt - Thanks for doing the original PR. If you haven't had time to update it by the end of the weekend, then I'll submit a seperate PR so we can get this all merged for the upcoming 2.0 release.

Please just reach out if there is something you are stuck on, have a question or need some assistance with it.

[ro-savage]

As @lnhrdt hasn't had time update the PR. I've now done a PR with these changes in #4192

[lnhrdt]

@ro-savage thank you for helping wrap this up. #4192 looks great.

[codeaid]

I know this issue has been closed but I felt like I had to vent my frustration and add to those voices who were hoping for production build class names to be just hashes.

Having read through all (or at least most) CSS module related discussions here it's sad to see that pretty much everyone who wanted to have hashes were shot down by certain people just because they "see no reason to change". That's the attitude!

Hashes in production builds was the only feature I was looking forward to in CRA2 but after seeing that we will have full paths renders it pointless for me. I never had a problem with using BEM notation in my JSX so switching from that to className={styles.something} doesn't really add any benefits, just a different syntax (even worse if you have multiple class names).

The whole point of using CSS modules for me personally is to "hide" my component naming conventions, behaviour and structure from the prying eyes of "Chrome inspectors" but if they can see stuff like Button_someStateClass_abcd it renders the whole concept pointless.

I know there's not much point in talking any more but all I can say is I couldn't be more disappointed with this decision. Don't even get me started on "I can't read class names in production" argument. It just doesn't make any sense. Somehow Google, Facebook and many other companies and users can deal with it just fine but apparently not us and frankly your code should be tested by the time it gets there.

I wish things like these were actually configurable without ejecting instead of them being imposed on us but alas.