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.

Sign up for GitHub

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

Jump to bottom

feat: do not hash localName if the localName is inlined #1410

Merged
merged 5 commits into from
Feb 2, 2022
Merged

feat: do not hash localName if the localName is inlined #1410

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8e93b75
feat: do not hash localName if the localName is inlined:
subzey Jan 26, 2022
976ff89
feat: do not hash localName if the localName is inlined:
subzey Jan 27, 2022
40cb298
chore: update nanoid version in the lockfile
subzey Jan 28, 2022
4c07771
Merge branch 'master' into feat/local-names-in-hashes
alexander-akait Jan 29, 2022
381d470
feat: do not hash localName if the localName is inlined: rename optio…
subzey Feb 2, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 30 additions & 0 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -872,6 +872,36 @@ module.exports = {
};
```

##### `hashStrategy`

Type: `'use-local-name' | 'auto'`
Default: `'use-local-name'`

Should local name be used when computing the hash.

- `'use-local-name'` Each identifier in a module gets its own hash digest.
- `'auto'` Identifiers from the same module shares the same hash digest if it's possible. Use this value to optimize the output for better GZIP or Brotli compression.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use minimal-subset 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Renamed the option enum values in 381d470


**webpack.config.js**

```js
module.exports = {
module: {
rules: [
{
test: /\.css$/i,
loader: "css-loader",
options: {
modules: {
hashStrategy: "auto",
},
},
},
],
},
};
```

##### `localIdentRegExp`

Type: `String|RegExp`
Expand Down
5 changes: 5 additions & 0 deletions src/options.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,11 @@
"link": "https://github.com/webpack-contrib/css-loader#localidenthashdigestlength",
"type": "number"
},
"hashStrategy": {
"description": "Allows to specify should localName be used when computing the hash.",
"link": "https://github.com/webpack-contrib/css-loader#hashstrategy",
"enum": ["use-local-name", "auto"]
},
"localIdentRegExp": {
"description": "Allows to specify custom RegExp for local ident name.",
"link": "https://github.com/webpack-contrib/css-loader#localidentregexp",
Expand Down
10 changes: 8 additions & 2 deletions src/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,14 +330,17 @@ function defaultGetLocalIdent(
localName,
options
) {
const { context, hashSalt } = options;
const { context, hashSalt, hashStrategy } = options;
const { resourcePath } = loaderContext;
const relativeResourcePath = normalizePath(
path.relative(context, resourcePath)
);

// eslint-disable-next-line no-param-reassign
options.content = `${relativeResourcePath}\x00${localName}`;
options.content =
hashStrategy === "auto" && /\[local\]/.test(localIdentName)
? relativeResourcePath
: `${relativeResourcePath}\x00${localName}`;

let { hashFunction, hashDigest, hashDigestLength } = options;
const matches = localIdentName.match(
Expand Down Expand Up @@ -756,6 +759,7 @@ function getModulesPlugins(options, loaderContext) {
localIdentHashDigest,
localIdentHashDigestLength,
localIdentRegExp,
hashStrategy,
} = options.modules;

let plugins = [];
Expand All @@ -780,6 +784,7 @@ function getModulesPlugins(options, loaderContext) {
hashFunction: localIdentHashFunction,
hashDigest: localIdentHashDigest,
hashDigestLength: localIdentHashDigestLength,
hashStrategy,
regExp: localIdentRegExp,
}
);
Expand All @@ -798,6 +803,7 @@ function getModulesPlugins(options, loaderContext) {
hashFunction: localIdentHashFunction,
hashDigest: localIdentHashDigest,
hashDigestLength: localIdentHashDigestLength,
hashStrategy,
regExp: localIdentRegExp,
}
);
Expand Down