Dependency extraction: improve calculation of contentHash #40930
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsnajdr
added
the
[Tool] Dependency Extraction Webpack Plugin
|
Size Change: -1 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
gziolo
reviewed
May 9, 2022
Comment on lines
222
to
224
| const contentHash = createHash( 'sha512' ) | ||
| .update( assetString ) | ||
| .digest( 'hex' ); |
There was a problem hiding this comment.
I would be curious to learn whether we could also refactor this usage to let webpack control it:
Suggested change
| const contentHash = createHash( 'sha512' ) | |
| .update( assetString ) | |
| .digest( 'hex' ); | |
| const contentHash = createHash( compilation.outputOptions.hashFunction ) | |
| .update( assetString ) | |
| .digest( compilation.outputOptions.hashDigest ); |
There was a problem hiding this comment.
Yes, I pushed a commit that does that with the .asset.php file content hash, too 👍
gziolo
reviewed
May 9, 2022
|
I can confirm that it works with Node 18 🎉 I can confirm the same I can also confirm that the content hash generated by webpack is the same as the saved as
Unrelated to this PR: I used the following config: output: {
filename: '[name].[contenthash].js',
path: resolve( process.cwd(), 'build' ),
},By the way, the asset file should probably use the same It's also longer than the one used with the JS file. |
gziolo
approved these changes
May 10, 2022
|
One more observation. the hash in the asset file is a bit longer than the one used for the JS file name. Should we shorten it a bit? We don't need an exact value but something that invalidates the cache so we could be good with having 7 chars like GitHub does commit SHA:
|
Future considerationsI'm testing with the default project scaffolde with
The same applies when importing gutenberg/packages/scripts/config/webpack.config.js Lines 104 to 123 in 326e178 However, I was able to resolve that part by changing the way files are collected for hashing: const hash = createHash( hashFunction );
- for ( const filename of entrypoint.getFiles().sort() ) {
+ for ( const filename of Array.from( entrypointChunk.files ).sort() ) {
const asset = compilation.getAsset( filename );
hash.update( asset.source.buffer() );
} |
Thanks for your observations! By default, webpack outputs hashes that are 20 chars long, which is the default value of the The finding that when importing a CSS file, the hashes no longer match, is also a very good one. I'll work on that. |
It's something you can address in another patch. I'm happy with the improvements applied so far, too 😄 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Follow-up to #34969 that implements suggestions from https://github.com/WordPress/gutenberg/pull/34969/files#r867737688:
createHashexported by webpack rather than Node'scryptomoduleRawSourceprefixesThe resulting hash is identical to what webpack itself would compute for filename like
[name].[contenthash].js, and it's also identical to simply hashing the file contents with, e.g.,