feat: do not hash localName if the localName is inlined

[subzey]

This PR contains a:

• bugfix
• new feature
• code refactor
• test update
• typo fix
• metadata update

Motivation / Use-Case

Proposal: The localName should not affect the hash if it is present in the generated id inline. This way the generated identifiers are compressed better with both GZIP and Brotli.

---

Currently the hash is computed from the file relative path and the local identifier name. It's good when the [local] is not preset in the template localIdentName:

✔️ [hash], before the change
.header from one.css becomes .ABCDEF
.button from one.css becomes .GHIJKL
.button from two.css becomes .MNOPQR

But it is redundant if the [local] is present:

😔 [local]--[hash], before the change
.header from one.css becomes .header--ABCDEF
.button from one.css becomes .button--GHIJKL
.button from two.css becomes .button--MNOPQR

The .button from one.css and .button from two.css still have to have different hashes.

But the .header from one.css and .button from the same one.css can never collide, they're from the same module! Omitting localName from the hash we get:

✔️ [local]--[hash], after the change
.header from one.css becomes .header--UVWXYZ
.button from one.css becomes .button--UVWXYZ
.button from two.css becomes .button--FGHIJK

Now all identifiers from the same module has the same hash. (If and only if the localName would be substituted into the resulting generated identifier!) And there are less unique random string runs in the output, making the result more GZIP friendly.

I've used test/__snapshots__/modules-option.test.js.snap for my "gzippability" estimations. This file "before" and "after" has the same "raw" byte length. But the "after" is almost 8% smaller after being GZIPped

We use the similar technique on tradingview.com for a year or so -- using custom getLocalIdent() in the webpack settings. But now it's time to reveal one of our optimization secrets 😀

Breaking Changes

None

Additional Info

⚠️ The generated identifiers will change compared to the previous versions - only if the [local] is used in the template. -- Postponed to the next major release.

[codecov]

Codecov Report

Merging #1410 (381d470) into master (3240394) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #1410   +/-   ##
=======================================
  Coverage   96.80%   96.81%           
=======================================
  Files          12       12           
  Lines        1127     1130    +3     
  Branches      408      411    +3     
=======================================
+ Hits         1091     1094    +3     
  Misses         27       27           
  Partials        9        9           

Impacted Files	Coverage Δ
src/utils.js	94.72% <100.00%> (+0.02%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3240394...381d470. Read the comment docs.

[alexander-akait]

Let's move this under option to avoid breaking changes, we can set it by default in the next major release

[subzey]

Sounds reasonable! How should I better do and name it?

Adding a new setting localNamesInHash: false | true | "auto" with the default true. And then in the next major release the default becomes "auto".

Or should it be something more generic like hashStrategy: "include-local-name" | "omit-local-name" | "auto"?

UPD:

I'm going with the options.modules.localIdentHashSource: "use-local-name" | "omit-local-name" | "auto". Is it okay?

[alexander-akait]

I think options.modules.hashStrategy sounds good

[subzey]

On a second thought, it's hard to imagine a use-case when 'omit-local-name' could be used:

• If [local] is used, it works just like 'auto'.
• If [local] is not there, it screws everything up.

Should we keep it for completeness and probably #1181 like features?

Or is it a bad idea too keep senseless options and it should better be removed?

[alexander-akait]

Let's avoid senseless options, we can add them late, if we will need

[subzey]

Done, in 976ff89

[alexander-akait]

@subzey Looks good, let's regenerate lock file due security problem in CI 👍

[subzey]

EPERM on Windows? I hope we could just restart the test and have it fixed

[alexander-akait]

I fixed the problem

[alexander-akait]

After review, I think 'auto' is not good name, 'auto' sounds like it should be default value, yes, maybe this should be the default (as I written above), but let's found better name, I think we can do it more understandable, I suggest: resource-path and resource-path-and-local-name, the developer can immediately understand what is used from the name, instead of reading the documentation

[subzey]

There's a chance a developer would set a resource-path, but the hashing would work as resource-path-and-local-name because of the automagic circuit breaker. That may lead to confusion, especially when copypasting the config partially into StackOverflow or GH issues.

Maybe minimal-subset or infer-from-template or something like that? Something that implies the automatic nature of the setting

I'm not a big fan of bikeshedding and just will use your suggestion if we don't come up with a decision after a short while

[alexander-akait]

Let's use minimal-subset 👍

[subzey]

Renamed the option enum values in 381d470

[alexander-akait]

Thanks

[dovidweisz]

This is really nice.

In what release is this (or will it be) available?

Upgrading css-loader to version 6.7.1 worked for me :)