-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
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
fix(core): always use hash for CSS module class names #10423
Conversation
@Josh-Cena do you see any reason to keep the current behavior? |
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
One reason could be to easily see where a class name is coming from and thus change it. I don't think it's strong enough though and React dev tools is better. Another reason could be performance? Hash computation can be much slower. Anyway you would also need to amend https://docusaurus.io/docs/styling-layout. |
⚡️ Lighthouse report for the deploy preview of this PR
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
Size Change: +355 B (0%) Total Size: 11.6 MB
ℹ️ View Unchanged
|
Agree 👍 The problem is:
Eventually, we could keep the longer name and add the hash in the middle of the class name in dev? localIdentName: isProd
? `[local]_[contenthash:base64:4]`
- : `[local]_[path][name]`,
+ : `[local]_[contenthash:base64:4]_[path][name]`, I'm afraid adding the hash at the end would lead to devs writing selectors starting with
According to my recent perf investigations, this is not at all a dev bottleneck currently, compared to other things like MDX and postcss
👍 will do once we agree on the change to be made |
I'm +1 to make dev and prod consistent. |
Motivation
For CSS modules, using clean class names in dev and hash in prod is confusing.
Users write CSS locally with selectors that only break after they deploy to production.
I believe it's better to have hashed class names in both environments to avoid this issue.
I consider this as a bugfix and an implementation detail.
Test Plan
CI + Argos
Test links
https://deploy-preview-10423--docusaurus-2.netlify.app/