Skip to content
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: Configuration for obfuscate/minify css module's local ident names #9223

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

zamarawka
Copy link

Added variable in advanced configuration to obfuscate css module's class names.
In some cases may reduce bundle size and also reduce html readability.

Preview:
image

Previous behavior saved. You should set env variable to turn this on.

Closes #8984

@facebook-github-bot
Copy link

Hi @zamarawka!

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@zamarawka
Copy link
Author

bump

@zamarawka
Copy link
Author

@iansu @ianschmitz @mrmckeb waiting for your feedback. I'd like to use this feature in the next release!!

@mrmckeb
Copy link
Contributor

mrmckeb commented Jul 11, 2020

Hi @zamarawka, I see you're trying to resolve #8984.

Can you talk us through your thinking here? For example, the hashes created seem quite long. Also, what problem do you feel this PR solves? Thanks!

@zamarawka
Copy link
Author

zamarawka commented Jul 14, 2020

Hi @mrmckeb!
Main idea of this feature is obfuscate code, make resulting class names unreadable. It could increase defense from unwanted data parse or usage of your application (bots, crawlers). It also increase protection of your source code from steal, reverse engineering.
In some cases generated hash is smaller then readable class names, which cra generate by default. Especially in case you give long filenames and class names. So it could reduce bundle size, but, in my opinion, this is not main goal of feature.
It's like GENERATE_SOURCEMAP=false advanced option, which I use for my production builds. This is pretty useful, when you try to make your source code more private.
I use default value for this loader, recommended by official docs for production builds. If you have any idea how resulting class names could be minified with safety, without collision and increasing readability, I'm ready to improve PR by your suggestion.

@zamarawka
Copy link
Author

Bump

@zamarawka
Copy link
Author

@iansu @ianschmitz @mrmckeb bump

@amyrlam amyrlam removed their request for review August 24, 2020 05:41
@ro-savage
Copy link
Contributor

@zamarawka - I prefer the longer and readable class names for two reasons.

  1. The class names when gzipped take minimal size, most likely it is better compressed than this hash because much of the text is repeated, creating a small transferred file size.
  2. When you are debugging your code, viewing our own source, and creating tests the readable of name helps to understand what component something belongs to, and gives it meaning. (E.g. it gives a
    some meaningful name).

I also don't believe this helps obscure your code much, because the important part would be to obscure what CSS affects what elements. This does not help with this, because your class names and your class="xyz" will still match meaning both human and machine can still easily copy any CSS.

I understand this is just an option, but it seems like an unnecessary option, when CRAs goal is to remain simple.

@ro-savage
Copy link
Contributor

I also don't believe this resolves #8984. That would resolve around minifying the classnames, not obscuring the names.

For example turning this:

.module1-foo { color: red; }
.module1-bar { font-size: 1.5em; background: yellow; }

.module2-baz { color: red; }
.module2-qux { font-size: 1.5em; background: yellow; width: 50px; }

into this

.module1-foo,.module2-baz{color:red}.module1-bar,.module2-qux{font-size:1.5em;background:#ff0}.module2-qux{width:50px}

or with advanced minification that renames the classes

.a,.b{color:red}.c,.d{font-size:1.5em;background:#ff0}.d{width:50px}

@zamarawka
Copy link
Author

@ro-savage If you prefer readable class names - don't use this option. As I say, it's like GENERATE_SOURCEMAP option. In development - no reason to use it. Only for stable production builds. By default it turned off, so you don't know about it until you need it.
I also don't, say you need to drop compression of your assets. Compression still main tool to decrease loading time. You mention only gzip, but in 2020 we must use brotli too, which give you a better results. This PR has no relation with compression. It could reduce bundle size in some cases, but it isn't target.

will still match meaning both human and machine

Just try to parse data from some resource like a twitter.
image

There is no way to understand relations of blocks, and after new build of app we got fully new class names (not partial)... Way better then you got something like MainContainer-gdl53fg or PagePanel-kjdfnk - which could be used for identify elements by regexp.
This feature is going out of the box from css-loader, I made a way to use it. CRA still simple.

@stale
Copy link

stale bot commented Dec 25, 2020

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Dec 25, 2020
@stale stale bot removed the stale label Apr 13, 2021
@stale
Copy link

stale bot commented Jan 9, 2022

This pull request has been automatically marked as stale because it has not had any recent activity. It will be closed in 5 days if no further activity occurs.

@stale stale bot added the stale label Jan 9, 2022
@zamarawka zamarawka mentioned this pull request Jan 18, 2022
@aganoza
Copy link

aganoza commented Mar 25, 2022

bump

@stale stale bot removed the stale label Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Production CSS is not minified/obfuscated
5 participants