bug #463 Using contenthash instead of chunkhash (weaverryan)

This PR was squashed before being merged into the master branch (closes #463).

Discussion
----------

Using contenthash instead of chunkhash

Since Webpack 4.3.0, a `[contenthash]` has existed, which is the [recommended way](https://webpack.js.org/guides/caching/#output-filenames) of versioning your filenames. Previously, we used a special plugin that gave us a `chunkhash`, which worked around issues with hash changing unnecessarily. However, `chunkhash` actually created a bug - in #461, some chunk id's change unnecessarily. That's bad for caching, but the *real* problem was that `[chunkhash]` did not change, and so the filenames did not change. The filenames DO change with `[contenthash]`.

Commits
-------

3797cba Using contenthash instead of chunkhash

Author: weaverryan

index.js

@@ -993,7 +993,7 @@ class Encore {
      * file is generated.
      *
      *     Encore.configureFilenames({
-     *         js: '[name].[chunkhash].js',
+     *         js: '[name].[contenthash].js',
      *         css: '[name].[contenthash].css',
      *         images: 'images/[name].[hash:8].[ext]',
      *         fonts: 'fonts/[name].[hash:8].[ext]'
@@ -1003,8 +1003,7 @@ class Encore {
      * will be used for any file types not passed.
      *
      * If you are using Encore.enableVersioning()
-     * make sure that your "js" filenames contain
-     * "[chunkhash]" and your "css" filenames contain
+     * make sure that your "js" and "css" filenames contain
      * "[contenthash]".
      *
      * @param {object} filenames

lib/WebpackConfig.js

@@ -614,6 +614,10 @@ class WebpackConfig {
             }
         }
 
+        if (typeof configuredFilenames.js !== 'undefined' && configuredFilenames.js.includes('[chunkhash')) {
+            logger.deprecation('Using the [chunkhash] placeholder in any filenames is deprecated: use [contenthash] instead.');
+        }
+
         this.configuredFilenames = configuredFilenames;
     }
 

lib/config-generator.js

@@ -184,7 +184,7 @@ class ConfigGenerator {
 
     buildOutputConfig() {
         // Default filename can be overridden using Encore.configureFilenames({ js: '...' })
-        let filename = this.webpackConfig.useVersioning ? '[name].[chunkhash:8].js' : '[name].js';
+        let filename = this.webpackConfig.useVersioning ? '[name].[contenthash:8].js' : '[name].js';
         if (this.webpackConfig.configuredFilenames.js) {
             filename = this.webpackConfig.configuredFilenames.js;
         }

lib/plugins/versioning.js

@@ -21,7 +21,7 @@ const PluginPriorities = require('./plugin-priorities');
 module.exports = function(plugins, webpackConfig) {
 
     /*
-     * With versioning, the "chunkhash" used in the filenames and
+     * With versioning, the "contenthash" used in the filenames and
      * the module ids (i.e. the internal names of modules that
      * are required) become important. Specifically:
      *
@@ -32,19 +32,14 @@ module.exports = function(plugins, webpackConfig) {
      *
      * 2) Similarly, if the final contents of a file don't change,
      *    then we also don't want that file to have a new filename.
-     *    The WebpackChunkHash() handles this, by making sure that
-     *    the chunkhash is based off of the file contents.
-     *
-     * Even in the webpack community, the ideal setup seems to be
-     * a bit of a mystery:
-     *  * https://github.com/webpack/webpack/issues/1315
-     *  * https://github.com/webpack/webpack.js.org/issues/652#issuecomment-273324529
-     *  * https://webpack.js.org/guides/caching/#deterministic-hashes
+     *    The "contenthash" handles this.
      */
     if (webpackConfig.isProduction()) {
         // shorter, and obfuscated module ids (versus named modules)
         // makes the final assets *slightly* larger, but prevents contents
         // from sometimes changing when nothing really changed
+        // Note: Should not be needed in Webpack 5:
+        // https://github.com/webpack/webpack/pull/8276
         plugins.push({
             plugin: new webpack.HashedModuleIdsPlugin(),
             priority: PluginPriorities.HashedModuleIdsPlugin
@@ -55,7 +50,7 @@ module.exports = function(plugins, webpackConfig) {
     }
 
     if (webpackConfig.useVersioning) {
-        // enables the [chunkhash] ability
+        // enables the [chunkhash] ability, which is deprecated
         plugins.push({
             plugin: new WebpackChunkHash(),
             priority: PluginPriorities.WebpackChunkHash

test/WebpackConfig.js

@@ -983,14 +983,14 @@ describe('WebpackConfig object', () => {
         it('Calling method sets it', () => {
             const config = createConfig();
             config.configureFilenames({
-                js: '[name].[chunkhash].js',
+                js: '[name].[contenthash].js',
                 css: '[name].[contenthash].css',
                 images: 'images/[name].[hash:8].[ext]',
                 fonts: 'fonts/[name].[hash:8].[ext]'
             });
 
             expect(config.configuredFilenames).to.deep.equals({
-                js: '[name].[chunkhash].js',
+                js: '[name].[contenthash].js',
                 css: '[name].[contenthash].css',
                 images: 'images/[name].[hash:8].[ext]',
                 fonts: 'fonts/[name].[hash:8].[ext]'
@@ -1014,6 +1014,23 @@ describe('WebpackConfig object', () => {
                 });
             }).to.throw('"foo" is not a valid key');
         });
+
+        it('Using chunkhash is deprecated', () => {
+            logger.reset();
+            logger.quiet();
+
+            after(() => {
+                logger.quiet(false);
+            });
+
+            const config = createConfig();
+
+            config.configureFilenames({
+                js: 'file.[chunkhash:16].js'
+            });
+
+            expect(logger.getMessages().deprecation).to.not.be.empty;
+        });
     });
 
     describe('configureUrlLoader', () => {

test/config-generator.js

@@ -199,7 +199,7 @@ describe('The config-generator function', () => {
             config.useVersioning = true;
 
             const actualConfig = configGenerator(config);
-            expect(actualConfig.output.filename).to.equal('[name].[chunkhash:8].js');
+            expect(actualConfig.output.filename).to.equal('[name].[contenthash:8].js');
 
             const miniCssPlugin = findPlugin(MiniCssExtractPlugin, actualConfig.plugins);
 

test/functional.js

@@ -390,11 +390,11 @@ describe('Functional tests using webpack', function() {
                 testSetup.runWebpack(config, (webpackAssert) => {
                     expect(config.outputPath).to.be.a.directory()
                         .with.files([
-                            'main.f1e0a935.js',
+                            'main.89eb104b.js',
                             'styles.8ec31654.css',
                             'manifest.json',
                             'entrypoints.json',
-                            'runtime.d41d8cd9.js',
+                            'runtime.3d179b24.js',
                         ]);
 
                     webpackAssert.assertOutputFileContains(
@@ -421,7 +421,7 @@ describe('Functional tests using webpack', function() {
                 config.enableVersioning(true);
                 config.configureFilenames({
                     js: '[name].js?[chunkhash:16]',
-                    css: '[name].css?[contenthash:16]'
+                    css: '[name].css?[chunkhash:16]'
                 });
 
                 testSetup.runWebpack(config, (webpackAssert) => {
@@ -437,7 +437,7 @@ describe('Functional tests using webpack', function() {
                     );
                     webpackAssert.assertManifestPath(
                         'styles.css',
-                        '/styles.css?8ec316547cc77b39'
+                        '/styles.css?91597a40238e0e66'
                     );
 
                     done();
@@ -491,14 +491,14 @@ describe('Functional tests using webpack', function() {
             testSetup.runWebpack(config, (webpackAssert) => {
                 expect(config.outputPath).to.be.a.directory()
                     .with.files([
-                        '0.8256b1ad.js', // chunks are also versioned
+                        '0.590a68c7.js', // chunks are also versioned
                         '0.8ec31654.css',
-                        'main.ba427376.js',
+                        'main.4a5effdb.js',
                         'h1.8ec31654.css',
                         'bg.0ec2735b.css',
                         'manifest.json',
                         'entrypoints.json',
-                        'runtime.d41d8cd9.js',
+                        'runtime.b84a9b43.js',
                     ]);
 
                 expect(path.join(config.outputPath, 'images')).to.be.a.directory()
@@ -1625,8 +1625,8 @@ module.exports = {
                     expect(config.outputPath).to.be.a.directory()
                         .with.files([
                             'entrypoints.json',
-                            'runtime.d41d8cd9.js',
-                            'main.1172d977.js',
+                            'runtime.21aa1db9.js',
+                            'main.22bad391.js',
                             'manifest.json',
                             'symfony_logo.ea1ca6f7.png',
                             'symfony_logo_alt.f27119c2.png',
@@ -1639,7 +1639,7 @@ module.exports = {
 
                     webpackAssert.assertManifestPath(
                         'build/main.js',
-                        '/build/main.1172d977.js'
+                        '/build/main.22bad391.js'
                     );
 
                     webpackAssert.assertManifestPath(