-
Notifications
You must be signed in to change notification settings - Fork 5
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
feat: make css-modules class names human-readable #143
Conversation
src/common/webpack/config.ts
Outdated
...(isCssModuleRules | ||
? { | ||
modules: { | ||
localIdentName: '[name]__[local]--[hash:base64:5]', | ||
exportLocalsConvention: 'asIs', | ||
}, | ||
} | ||
: {}), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use these options and no other changes will be required.
...(isCssModuleRules | |
? { | |
modules: { | |
localIdentName: '[name]__[local]--[hash:base64:5]', | |
exportLocalsConvention: 'asIs', | |
}, | |
} | |
: {}), | |
modules: { | |
auto: true, // uses modules only with files *.module.s?css | |
localIdentName: isEnvDevelopment | |
? '[name]__[local]--[hash:base64:5]' | |
: '[hash:base64]', | |
exportLocalsConvention: 'camelCase', // gives both camel case and as is | |
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you think readable class names are useful for non-dev environments as well?
I see no harm in exposing those in production builds.
This approach is hardly different from using BEM-naming.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We minify all other code except class names, which seems strange to me. And here are source-maps to see original classnames.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, we don't minify BEM classnames. I really doubt we minify css custom property names.
If we were to compare those side-by-side, the difference to total file size seems negligeable:
.MnET1WGLIieyGuzfo5cw {}
.SomeComponent_wrapper-f1da4 {}
Hashes generally have more entropy than variable (or file) names, which leads me to suggest total amount of bytes transferred over network (after gzip/brotli) will be even less for readable names. Didn't measure it, ofc :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a55a94c
to
dcc0281
Compare
Hey? :] |
Minor improvement to css modules support.
This pull request makes generated css-modules class names human readable (instead of something like
kImIF7sZoAjSPn74Hpe8
).Without this improvement css-modules are nearly unusable and are completely undebuggable.
===============
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en.