Prune empty JS bundles when plugin is used in conjunction with code splitting #339

rowanoulton · 2019-01-16T00:53:52Z

This PR contains a:

Motivation / Use-Case

This is a proposed fix for #147. It's a stab in the dark and, while it seems to work, it may be the wrong approach entirely. Please advise!

For reference: the code samples below are from this reproduction shared by @dcousineau.

Outline of the issue

When MiniCssExtractPlugin is used in conjunction with code splitting and the split chunk contains nothing other than CSS, webpack will still emit a JS file. Consider the following configuration:

optimization: {
	splitChunks: {
		cacheGroups: {
		styles: {
			name: "styles",
			test: /\.css$/,
			chunks: "all",
			enforce: true
		}
	}
}

This will move all CSS across the entire compilation into a single file, styles.bundle.css. However, the compilation will also create a JavaScript bundle for this chunk under the same name: styles.bundle.js.

Aside: I'd be curious to know why there's a JS module for every CSS module. My guess is the plugin needs a JS module to "hang" its CSS modules off of?

Often these bundles are more or less empty, save for some boilerplate:

(window["webpackJsonp"] = window["webpackJsonp"] || []).push([["styles"],{

/***/ "./src/index.css":
/*!***********************!*\
  !*** ./src/index.css ***!
  \***********************/
/*! no static exports found */
/***/ (function(module, exports, __webpack_require__) {

// extracted by mini-css-extract-plugin

/***/ }),

/***/ "./src/sync.css":
/*!**********************!*\
  !*** ./src/sync.css ***!
  \**********************/
/*! no static exports found */
/***/ (function(module, exports, __webpack_require__) {

// extracted by mini-css-extract-plugin

/***/ })

}]);

This makes sense. The bodies of these modules are CSS, and the plugin has extracted them leaving only a comment behind.

Problematic bundles

These empty JavaScript bundles are problematic in that they must be loaded — and loaded in the correct order — for the application to boot properly. This is due to the way webpack boots, the logic of which is contained in webpack's boilerplate code. Here're the relevant parts:

/******/ 	var jsonpArray = window["webpackJsonp"] = window["webpackJsonp"] || [];
/******/ 	var oldJsonpFunction = jsonpArray.push.bind(jsonpArray);
/******/ 	jsonpArray.push = webpackJsonpCallback;
/******/ 	jsonpArray = jsonpArray.slice();
/******/ 	for(var i = 0; i < jsonpArray.length; i++) webpackJsonpCallback(jsonpArray[i]);
/******/ 	var parentJsonpFunction = oldJsonpFunction;
/******/
/******/
/******/ 	// add entry module to deferred list
/******/ 	deferredModules.push(["./src/index.js","styles"]);
/******/ 	// run deferred modules when ready
/******/ 	return checkDeferredModules();

This code is responsible for installing modules: executing the function each module is contained in and returning its exports. Specifically, the install handler (webpackJsonpCallback) is executed for every module that was loaded before the main script.

Let's break it down. Here's a simplified version ofstyles.bundle.js again:

window["webpackJsonp"] = window["webpackJsonp"] || []).push([["styles"], {
	(function(module, exports, __webpack_require__) {
		// extracted by mini-css-extract-plugin
	}),

	(function(module, exports, __webpack_require__) {
		// extracted by mini-css-extract-plugin
	})
}];

You can see that, as each file loads, it injects its modules into window['webpackJsonp'].

When the main script finally runs, it iterates over these same modules and installs them via webpackJsonpCallback:

/******/ 	var jsonpArray = window["webpackJsonp"] = window["webpackJsonp"] || [];
/******/ 	var oldJsonpFunction = jsonpArray.push.bind(jsonpArray);
/******/ 	jsonpArray.push = webpackJsonpCallback;
/******/ 	jsonpArray = jsonpArray.slice();
/******/ 	for(var i = 0; i < jsonpArray.length; i++) webpackJsonpCallback(jsonpArray[i]);

After all the modules are installed, webpack checks it has all the modules it needs (ie. "deferred modules"):

/******/ 	// add entry module to deferred list
/******/ 	deferredModules.push(["./src/index.js","styles"]);
/******/ 	// run deferred modules when ready
/******/ 	return checkDeferredModules();

Note the mention of a module called "styles": this is that empty script we saw earlier, produced by the combination of MiniCssExtractPlugin and webpack's code splitting functionality.

Here's what webpack does when it checks deferred modules:

/******/ 	function checkDeferredModules() {
/******/ 		var result;
/******/ 		for(var i = 0; i < deferredModules.length; i++) {
/******/ 			var deferredModule = deferredModules[i];
/******/ 			var fulfilled = true;
/******/ 			for(var j = 1; j < deferredModule.length; j++) {
/******/ 				var depId = deferredModule[j];
/******/ 				if(installedChunks[depId] !== 0) fulfilled = false;
/******/ 			}
/******/ 			if(fulfilled) {
/******/ 				deferredModules.splice(i--, 1);
/******/ 				result = __webpack_require__(__webpack_require__.s = deferredModule[0]);
/******/ 			}
/******/ 		}
/******/ 		return result;
/******/ 	}

It checks that every module in the array after ./src/index.js has been installed, or "fulfilled", before it does anything else.

This is critical: If the module "styles" has not been installed already, the boot will stall. This is because the if(fulfilled) branch doesn't execute, and it contains the first invocation of __webpack_require__, which is responsible for executing the main entry point and therefore the whole application.

Proposed fix

Here's what we want to do:

Remove these empty JS files from the boot flow
Keep our intended split chunks intact (the "styles" CSS bundle in our example)

And, if possible:

Don't emit the empty JS files at all

Fixing the boot flow

The problem is what's written to webpack's boilerplate code:

/******/ 	// add entry module to deferred list
/******/ 	deferredModules.push(["./src/index.js","styles"]);

We need to catch and prune empty files before they're included here.

Those exact lines are written out by the JsonpMainTemplate plugin, invoked by the creation of chunk assets during compilation. This is convenient as MiniCssExtractPlugin already taps that event:

compilation.mainTemplate.hooks.renderManifest.tap(
	pluginName,
	(result, { chunk }) => {
	  const renderedModules = Array.from(chunk.modulesIterable).filter(
	    (module) => module.type === MODULE_TYPE
	  );
	  if (renderedModules.length > 0) {
	    result.push({
	      render: () =>
	        this.renderContentAsset(
	          compilation,
	          chunk,
	          renderedModules,
	          compilation.runtimeTemplate.requestShortener
	        ),
	      filenameTemplate: this.options.filename,
	      pathOptions: {
	        chunk,
	        contentHashType: MODULE_TYPE,
	      },
	      identifier: `${pluginName}.${chunk.id}`,
	      hash: chunk.contentHash[MODULE_TYPE],
	    });
	  }
	}
);

This block of code is responsible for extracting the CSS from each module. Once that extraction has happened, it seems as if we're free to do whatever we like with the leftover module. This presents an opportune moment to search for split chunks that are purely CSS and perform some tidyup:

const splitChunks = compilation.chunks.filter(chunk => {
  return (
    chunk.chunkReason && chunk.chunkReason.includes("split chunk")
  );
});

splitChunks.forEach((splitChunk) => {
  const modulesWeCouldRemove = [];
  const nonCssModules = Array.from(splitChunk.modulesIterable).filter((mod) => {
    return mod.type !== MODULE_TYPE;
  });

  nonCssModules.forEach((nonCssMod) => {
    if (
      nonCssMod._source &&
      nonCssMod._source._value === `// extracted by ${pluginName}`
    ) {
      modulesWeCouldRemove.push(nonCssMod);
    }
  });

  // If there's nothing but CSS modules left in this split chunk, remove the whole thing
  // This will mean that the main template manifest (ie. webpack's boilerplate code) won't
  // contain any references to these empty modules
  if (modulesWeCouldRemove.length === nonCssModules.length) {
    modulesWeCouldRemove.forEach((nonCssMod) => {
      // Trace all the "reasons" for this module (ie. other modules that depend on it)
      // then add this module into their respective chunks. This effectively reverses
      // the work SplitChunksPlugin did to break out the logic into a separate chunk.
      // Without this step there will be script errors owing to missing dependencies
      // and adding them back to their origin chunks is harmless as they're empty.
      nonCssMod.reasons.forEach((reason) => {
        reason.module.chunksIterable.forEach((previouslyConnectedChunk) => {
          splitChunk.moveModule(nonCssMod, previouslyConnectedChunk);
        });
      });
    });

    splitChunk.groupsIterable.forEach((group) => {
      group.removeChunk(splitChunk);
    });
  }
});

For any purely CSS-based split chunks we reverse the work SplitChunksPlugin did by moving the empty JS modules back to their origin chunks then remove the split chunk from the module graph altogether. By this method we prevent them from being referenced by the boilerplate boot code.

Keeping the stylesheets in play

With the first problem solved, we encounter another: by removing empty split chunks from the dependency graph, the stylesheets are emitted by webpack but are ignored by plugins like HtmlWebpackPlugin. In practice this can mean HTML rendered without the appropriate <link> tags.

To combat this, we have to retain a dependency on our CSS split chunks. If we track which groups a given chunk was removed from, we can restore the connections after the boilerplate has been generated. First this requires a bit of extra book-keeping:

class MiniCssExtractPlugin {
  constructor(options) {
    this.disconnectedGroups = {};
    ...

splitChunk.groupsIterable.forEach((group) => {
  group.removeChunk(splitChunk);

  // Book-keeping
  this.disconnectedGroups[splitChunk.id] =
    this.disconnectedGroups[splitChunk.id] || [];
  this.disconnectedGroups[splitChunk.id].push(group);
});

The first event after the boilerplate code has been generated is compilation.hooks.chunkAsset, so we do our reconnection work there:

compilation.hooks.chunkAsset.tap(pluginName, (chunk, file) => {
  // If this was a split chunk we disconnected previously
  if (this.disconnectedGroups[chunk.id]) {
    if (path.extname(file) === '.css') {
      this.disconnectedGroups[chunk.id].forEach((group) => {
        // Add the stylesheet as a file on every chunk that requires it
        // This ensures plugins like html-webpack-plugin will find and include
        // split CSS chunks
        group.chunks.forEach((parentChunk) => {
          parentChunk.files.push(file);
        });
      });
    }
  }
});

I'm especially unsure about this code but the intention is to reinsert the CSS files into every chunk that depends on those styles, and this appears to be enough to appease HtmlWebpackPlugin.

Discarding the empty JS files

From here it's just a small jump to discard the empty JS bundles altogether. We simply branch from the previous file extension check and, if it's JavaScript, delete it:

if (path.extname(file) === '.css') {
  this.disconnectedGroups[chunk.id].forEach((group) => {
    // Add the stylesheet as a file on every chunk that requires it
    // This ensures plugins like html-webpack-plugin will find and include
    // split CSS chunks
    group.chunks.forEach((parentChunk) => {
      parentChunk.files.push(file);
    });
  });
} else if (path.extname(file) === '.js') {
  // Discard empty JS file
  chunk.files.pop();
  delete compilation.assets[file];
}

This is undoing the work that would have just happened in the webpack compiler.

Outcome

Applying these changes to the aforementioned reproduction gives us the result we want:

One CSS bundles (styles.css)
No empty JS bundle (styles.js)
An application that boots

I've confirmed the same is true of a webpack-dev-server build, and tested with both html-webpack-plugin@latest and html-webpack-plugin@next.

Breaking Changes

Unclear. This change will mean empty JS bundles are no longer output for every CSS split chunk. That may break workarounds people have been using.

Additional information

I'm not convinced this is the best way to fix this problem, and could use some input from someone better versed in this plugin and with webpack in general.

jsf-clabot · 2019-01-16T00:53:53Z

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

Fixes webpack-contrib#147

alexander-akait

This problem should be solved on webpack side.

Anyway couple new eyes will be great.
/cc @sokra @ooflorent

Also need fix CI problems and add tests

rowanoulton · 2019-01-16T19:57:50Z

@evilebottnawi thanks for the quick reply. Do you have some thoughts on how this should be fixed in webpack?

RE: CI failures. Are they worth fixing if this isn't the right approach? The failure is in regard to npm audit which I'm unsure is related to my change:

┌───────────────┬──────────────────────────────────────────────────────────────┐
│ High          │ Missing Origin Validation                                    │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package       │ webpack-dev-server                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ webpack-dev-server [dev]                                     │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path          │ webpack-dev-server                                           │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info     │ https://nodesecurity.io/advisories/725                       │
└───────────────┴──────────────────────────────────────────────────────────────┘

alexander-akait · 2019-01-17T09:16:02Z

@rowanoulton webpack doesn't should emit empty chunks, it is should be fixed in webpack@5 in split chunks

alexander-akait · 2019-01-17T09:16:17Z

Update lock file and commit

Jab2870 · 2019-04-10T08:06:18Z

I am still getting this issue. Has it been fixed? Is there a workaround?

alexander-akait

We need tests for this

rowanoulton · 2019-06-01T19:03:46Z

@evilebottnawi I took your earlier comment to mean this isn't the right approach. The main issue to patch being this one. Is that right? if so, I'll close this PR.

rowanoulton · 2019-06-06T18:56:53Z

For those following along, it looks like this is the fix.

rowanoulton · 2019-08-05T21:59:20Z

Relevant commit here

rowanoulton changed the title ~~Potential fix for empty JS bundles when mini-css-extract-plugin used in conjunction with code splitting~~ Fix empty JS bundles when mini-css-extract-plugin used in conjunction with code splitting Jan 16, 2019

rowanoulton changed the title ~~Fix empty JS bundles when mini-css-extract-plugin used in conjunction with code splitting~~ Prune empty JS bundles when mini-css-extract-plugin used in conjunction with code splitting Jan 16, 2019

rowanoulton changed the title ~~Prune empty JS bundles when mini-css-extract-plugin used in conjunction with code splitting~~ Prune empty JS bundles when plugin is used in conjunction with code splitting Jan 16, 2019

fix: prune empty js bundles when code splitting

2962f42

Fixes webpack-contrib#147

rowanoulton force-pushed the fix-empty-js-stalled-boots branch from 49ef3a9 to 2962f42 Compare January 16, 2019 01:15

alexander-akait reviewed Jan 16, 2019

View reviewed changes

alexander-akait requested changes Apr 10, 2019

View reviewed changes

rowanoulton closed this Aug 5, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prune empty JS bundles when plugin is used in conjunction with code splitting #339

Prune empty JS bundles when plugin is used in conjunction with code splitting #339

rowanoulton commented Jan 16, 2019 •

edited

Loading

jsf-clabot commented Jan 16, 2019 •

edited

Loading

alexander-akait left a comment •

edited

Loading

rowanoulton commented Jan 16, 2019

alexander-akait commented Jan 17, 2019

alexander-akait commented Jan 17, 2019

Jab2870 commented Apr 10, 2019

alexander-akait left a comment

rowanoulton commented Jun 1, 2019

rowanoulton commented Jun 6, 2019

rowanoulton commented Aug 5, 2019

Prune empty JS bundles when plugin is used in conjunction with code splitting #339

Prune empty JS bundles when plugin is used in conjunction with code splitting #339

Conversation

rowanoulton commented Jan 16, 2019 • edited Loading

Motivation / Use-Case

Outline of the issue

Problematic bundles

Proposed fix

Fixing the boot flow

Keeping the stylesheets in play

Discarding the empty JS files

Outcome

Breaking Changes

Additional information

jsf-clabot commented Jan 16, 2019 • edited Loading

alexander-akait left a comment • edited Loading

Choose a reason for hiding this comment

rowanoulton commented Jan 16, 2019

alexander-akait commented Jan 17, 2019

alexander-akait commented Jan 17, 2019

Jab2870 commented Apr 10, 2019

alexander-akait left a comment

Choose a reason for hiding this comment

rowanoulton commented Jun 1, 2019

rowanoulton commented Jun 6, 2019

rowanoulton commented Aug 5, 2019

rowanoulton commented Jan 16, 2019 •

edited

Loading

jsf-clabot commented Jan 16, 2019 •

edited

Loading

alexander-akait left a comment •

edited

Loading