Move webpack manifest out from index chunk(Improve long term caching of index chunk)

[dhruvparmar372]

Issue

index chunk hash does not change if any of the children chunks content changes.

Steps to reproduce

1. yarn run build on master branch.
2. Save the build/client/index-{}.js file for later reference
3. Change content of shared/components/DemoApp/AsyncCounterRoute/CounterRoute.js
4. yarn run build.
5. Save the build/client/index-{}.js file.

Both versions of index-{}.js file have same file names but different hash name for "counter" chunk inside them.

This is currently an issue with webpack webpack/webpack#1856

A possible fix for same is to use https://github.com/roscoe054/webpack-hashed-module-id-plugin
but it tends to generate new chunk names on every build even for unchanged chunks. This hampers long term caching of those chunks

Another side effect of the issue is that since webpack manifest is currently part of the index chunk on every change to any child chunk the index chunk's hash will change.

Fix

The manifest is inlined with every server rendered route in production mode. This PR enables the same in production mode

[dhruvparmar372]

@ctrlplusb any thoughts on this? I ran into the issue while using react-universally in production for an application.

[slipo]

I would strongly recommend this approach is taken.

This is a huge issue if you have a CDN in front of your files and it can be very tricky to debug. Users in one part of the world can get a cached manifest pointing to files that no longer exist (completely breaking the site), while other users in a different region will get the new manifest and everything will work.

I believe the issue is with the md5 hasher:
erm0l0v/webpack-md5-hash#9

The solution is either dhruvparmar372's which I think is ideal because it increases the likelihood of serving a cached index or removing webpack-md5-hash like this (not making a PR because in the end I think we should break out the manifest):

diff --git a/internal/webpack/configFactory.js b/internal/webpack/configFactory.js
index a7060c5..71da446 100644
--- a/internal/webpack/configFactory.js
+++ b/internal/webpack/configFactory.js
@@ -4,7 +4,6 @@ import ExtractTextPlugin from 'extract-text-webpack-plugin';
 import nodeExternals from 'webpack-node-externals';
 import path from 'path';
 import webpack from 'webpack';
-import WebpackMd5Hash from 'webpack-md5-hash';
 
 import { happyPackPlugin, log } from '../utils';
 import { ifElse } from '../../shared/utils/logic';
@@ -226,13 +225,6 @@ export default function webpackConfigFactory(buildOptions) {
       // the significant improvement will be how fast the JavaScript loads in the browser.
       ifProdClient(new webpack.optimize.ModuleConcatenationPlugin()),
 
-      // We use this so that our generated [chunkhash]'s are only different if
-      // the content for our respective chunks have changed.  This optimises
-      // our long term browser caching strategy for our client bundle, avoiding
-      // cases where browsers end up having to download all the client chunks
-      // even though 1 or 2 may have only changed.
-      ifClient(() => new WebpackMd5Hash()),
-
       // These are process.env flags that you can use in your code in order to
       // have advanced control over what is included/excluded in your bundles.
       // For example you may only want certain parts of your code to be

[sinwailam193]

Hi, I believe this should be in the bodyElements array

[dhruvparmar372]

hey, could you give a reasoning for the same? i don't see much difference as long as the manifest is loaded before webpack entry files.

[sinwailam193]

Sorry, there is actually no difference, but I just thought to keep it consistent since other inlineScript is used inside the bodyElements.

[sinwailam193]

I actually tried out this resolution by @dhruvparmar372 , however it will fail for service worker offline, as I get MIME Type error for application/javascript for the manifest.json.

[oyeanuj]

@dhruvparmar372 @sinwailam193 @slipo what solution did you folks end up with here?

[dhruvparmar372]

@oyeanuj i feel this approach should be taken due to the issues pointed above.
@sinwailam193 i've put in the fix for the offline mode error, the inlined manifest needed to have correct nonce value inorder to execute in offline mode.