Skip to content

CSS Modules – random vs deterministic class names in production #3972

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

Closed
michaelhogg opened this issue Feb 4, 2018 · 5 comments
Closed

Comments

@michaelhogg
Copy link

Firstly, many thanks to @ro-savage for implementing CSS Modules in #2285 (merged and ready for release in [email protected] – see #3815) 🎉

I'm opening this issue to continue the discussion regarding random-vs-deterministic localIdentName class names in production builds.

⚠️ There were some important comments from @heavi5ide and @simonrelet which apparently were forgotten about, possibly due to a force-push which made it impossible to reach the original commit containing the comments (a15df83).


For ease of reading, I'm copying the relevant comments from here:

Click 👈

[@ro-savage] When using css modules, class names follow a deterministic convention rather than the standard random hash with the covention [directory]__[filename]___[classname].

Given src/components/Button/Button.module.css with a class .primary {}, the generated classname will be src-components-Button__Button-module___primary.

This is done to allow targeting off elements via classname, causes minimal overhead with gzip-enabled, and allows a developer to find component location in the devtools.


and here:

Click 👈

[@ro-savage] I've updated the pull request to use deterministic classnames rather than random hash, as suggested by another user (sorry can't find the comment!).

Instead of someFileName__someClassName___hash it becomes directory__someFileName___someClassName.

This allows predictability as to what the class name will be, so that it can be targeted.

This can can not clash because the file would have to be in the same dir + same file name + same class name. Which if it is, it is the same class anyway. It also shouldn't contribute very minimally to filesize because gzip should handle the repetition of long classnames.

This has been how react-scripts-cssmodules has been naming things for a couple months.


and here:

Click 👈

[@klzns] Should the production build expose the app architecture?

[@amannn] In regards to @klzns comment, I'd also appreciate not exposing the file path

[@andriijas] Why does it need to be deterministic and targetable? kind of kills the whole module thing. Why not recommend people to add additional static class if it needs to be targetable? Im upp for hashes.

[@sompylasar] Web marketing people often freak out if they cannot attach the out-of-the-box tools like HeapAnalytics or MouseFlow to specific elements of a website/webapp by id/classname (or if the classnames they attach to are changing with each release).


and here (you'll need to click the "Show outdated" link to see the comments):

Click 👈

[@recidive] One thing that's very useful, specially when you are using element based event tracking in your site/app, is naming classes based on component path instead of a hash so classnames don't change on every build. This can be accomplished with localIdentName: '[path][name]__[local]'. Making classnames less volatile is useful for automations that rely on element classnames.

[@ro-savage] Is there any security implications of showing the repos paths?

[@ro-savage] After rewriting this to hash the path, I decided to see if I could to a PR to css-loader to add it as an option.

Once I looked at the source, I can see they are already generating the hash based on the path.
options.content = options.hashPrefix + request + "+" + localName;
which translates to relative/path/to/file+classname

I had never tested it before, but if you build it multiple times the classname should not change. Unless you change the file location or the classname itself.

@recidive & @heavi5ide are you getting different classnames with each build?

[@heavi5ide] Yes I just tested this out and it seems you are correct and this shouldn't really be a problem at all. @recidive are you sure this is still an issue? It looks like the hash generation code that @ro-savage linked to in css-loader was changed "to something more reproducable" almost two years ago: webpack-contrib/css-loader@55ad466

[@simonrelet] I'm probably missing an important thing here and I understand the need for the naming during the development, but why isn't it just [hash:base64:*] in production?


I think there are two key points which merit further discussion:


1️⃣ The concerns of @klzns and @amannn regarding exposing the source code's file paths in production builds.

I share their concerns – I would feel uncomfortable seeing class names such as src-components-Button__Button-module___primary in production.  I understand the benefits of deterministic/predictable class names which don't change with each build (so that elements can be targeted by class name), but this can be achieved without exposing the source code's file paths.


2️⃣ The fact that css-loader's [hash] token for localIdentName is actually deterministic (not random), as @heavi5ide and @simonrelet noticed.

Click for details of getLocalIdent() and interpolateName() 👈

CreateReactApp / ReactScripts currently uses css-loader v0.28.7.  Looking at the source code, the localIdentName template is passed to getLocalIdent(), which effectively generates the class name in this way:

//                Filepath of the CSS file, relative to the project root dir         Source CSS class name
options.content = path.relative(options.context, loaderContext.resourcePath) + "+" + localName;

loaderUtils.interpolateName(loaderContext, localIdentName, options);

interpolateName() is provided by Webpack's loader-utils library, and the description of the [hash] token is:

the hash of options.content (Buffer) (by default it's the hex digest of the md5 hash)

[<hashType>:hash:<digestType>:<length>] optionally one can configure

  • other hashTypes, i. e. sha1, md5, sha256, sha512
  • other digestTypes, i. e. hex, base26, base32, base36, base49, base52, base58, base62, base64
  • and length the length in chars

The default template for localIdentName is "[hash:base64]", which generates the MD5 hash (as raw binary data) and then base64-encodes it.


So [hash] is not random.  It's the digest of: <Source CSS relative filepath> "+" <Source CSS class name>


So my recommendation is to change localIdentName in packages/react-scripts/config/webpack.config.prod.js from:

localIdentName: '[path]__[name]___[local]'

to something like:

localIdentName: '[hash:base64]'

The hash will be deterministic, predictable, unique and targetable, without exposing the source code's file paths.

@andriijas
Copy link
Contributor

I'm 100% with you. The current localident is to verbose. And more importantly if you have third parties relying on your dom you should add other target points like data-target="Foo" and not rely on how you style your components.

@sompylasar
Copy link

I'd like to emphasize that CSS styles being tied to HTML classes is a coincidence. Originally HTML classes were meant to describe the semantic meaning of an element in the markup regardless of the styling. CSS then targets these markup elements via one or more types of selectors (one being selector by HTML class) to add styles to them.

@gaearon
Copy link
Contributor

gaearon commented Feb 4, 2018

At Facebook we're using something similar to CSS Modules but with a different syntax.

Our class names look like this:

  • something like -PRIVATE-comment__root in development
  • something like _3u16 in production (they're not hashes, but are stored in a database)

I feel like this achieves a nice balance. In development it's obvious you shouldn't rely on them, and in production they're even shorter than hashes. I guess keeping a database doesn't really work in CRA context though.

For debugging, we use a special query parameters that serves "development-like" CSS class names. Again, this is something we can't easily do with webpack and static builds, but I hope this is helpful as a reference point.

@klimashkin
Copy link

Keep the same localIdentName, but just add https://github.com/JPeer264/node-rename-css-selectors to get gmail-like short classes for production build as I described here #3965 (comment)

@gaearon
Copy link
Contributor

gaearon commented Feb 4, 2018

I'm closing the discussion in favor of the earlier discussion in #3965. In particular, I propose this solution: #3965 (review).

@gaearon gaearon closed this as completed Feb 4, 2018
@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

5 participants