@wordpress/dependency-extraction-webpack-plugin should use content hash

[anomiex]

Description

@wordpress/dependency-extraction-webpack-plugin outputs the equivalent of [fullhash]/[chunkhash] as the "version" rather than [contenthash]. Note that Webpack itself now recommends using [contenthash].

The use of [fullhash]/[chunkhash] can result in extraneous hash changes. For example, if the JavaScript is identical but some CSS changed, it will still change the hash for the JavaScript files.

In certain configurations, this can even result in a hash change even if the code is unchanged, if the code is built in a different directory. See the reproduction below. We've been running into this frequently with the wpcom TeamCity builds of Jetpack.

You might fix this by changing

gutenberg/packages/dependency-extraction-webpack-plugin/lib/index.js

Line 186 in eb4c8e3

version: runtimeChunk.hash,

to

version: runtimeChunk.contentHash && runtimeChunk.contentHash.javascript || runtimeChunk.hash,

Step-by-step reproduction instructions

1. Download and extract test.zip
2. Run npm install && npx webpack. It also works with pnpm; I haven't tested yarn.
3. Extract test.zip again to a different path.
4. Run npm install && npx webpack there too.
5. Compare the contents of the dist directories. See that, although everything else is identical, the "version" hash in the asset.php file is different.

What's going on here is that @babel/plugin-transform-runtime with absoluteRuntime: true winds up injecting a variable name derived from the absolute path to @babel/runtime. Even though this variable name gets minified away, it still affects the [fullhash]/[chunkhash].

Screenshots, screen recording, code snippet

No response

Environment info

N/A

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

[gziolo]

webpack/webpack.js.org#2096 - thanks for sharing the recommendation. @sirreal, do you remember why you settled on hash? It looks like we should follow what @anomiex shared.

[sirreal]

This is actually part of your contribution from #17298 😆

This is intended to set the script or style version. It's used as the version for WordPress script and style dependencies via WP_Scripts::add:

$ver Optional. String specifying item version number, if it has one, which is added to the URL as a query string for cache busting purposes. If version is set to false, a version number is automatically added equal to current installed WordPress version. If set to null, no version is added.

gutenberg/lib/client-assets.php

Line 246 in d3a5128

$version = isset( $asset['version'] ) ? $asset['version'] : $default_version;

Version is intended to be a hash that represents the entire build and should change if any of the build changes. runtimeChunk.hash may actually be less volatile than intended 🙂

With contenthash, I believe we're typically thinking of a discreet build artifact. We don't want to cache bust a file that hasn't changed at all. That's not the case here, we're not thinking about a discreet artifact but the build as a whole. We're looking for a hash that changes when any part of the build changes.

I don't believe there's such a thing as contenthash for the entire build, although I may be wrong. If we'd like to have more stable versions for all the assets, that's would be a really interesting area to explore. I think we'd need to look at generating a more complete manifest with versions for all assets.

[anomiex]

If you want the version hash to take into account any css and whatever, because the one hash needs to cover all the entrypoint's files, I wouldn't complain about that. You could combine the hashes of every content type in runtimeChunk.contentHash (sorted by type name) instead of only the javascript hash.

The problem we're having is that the current hash takes into account stuff that doesn't make it into the output of any of the entrypoint's files, like variable names that have been minified away. There's no point in busting the cache when nothing in the files to be cached actually changed.

[sirreal]

There's no point in busting the cache when nothing in the files to be cached actually changed.

Absolutely, if there's a better alternative it would be a welcome change.

[gziolo]

This is actually part of your contribution from #17298 😆

That's surprising 😅

With contenthash, I believe we're typically thinking of a discreet build artifact. We don't want to cache bust a file that hasn't changed at all. That's not the case here, we're not thinking about a discreet artifact but the build as a whole. We're looking for a hash that changes when any part of the build changes.

From the WordPress core perspective it's the opposite. We want to maintain versioning for individual files/chunks. If a single chunk never changes then it's perfectly fine to keep the same version that is based on the actual content. I see now how the name the field can be confusing here.

That means that we would benefit from switching to “runtimeChunk.contentHash”.

[sirreal]

From the WordPress core perspective it's the opposite. We want to maintain versioning for individual files/chunks. If a single chunk never changes then it's perfectly fine to keep the same version that is based on the actual content.

Agreed, this is desirable. It would imply a change to the format of the generated asset file. They currently include exactly one version. The asset file would need to include the contentHash for all enqueue-able assets.

[gziolo]

#34969 is ready for the final review. Can someone help to test and validate the changes proposed?