finalize webpack@4 implementation (#134)

* finalize webpack@4 implementation

* test: reduce number of iterations

There should no longer be any race condition possible, so it's pretty safe
even if it could prevent the same issue not to be spotted

* fix: webpack@3 plugin hook name

Author: mastilver

.travis.yml

@@ -3,12 +3,11 @@ node_js:
   - '9'
   - '8'
   - '6'
-  - '4'
 
 env:
   - WEBPACK_VERSION=2 EXTRACT_PLUGIN_VERSION=2
   - WEBPACK_VERSION=3 EXTRACT_PLUGIN_VERSION=3.0.2
-  - WEBPACK_VERSION=4 EXTRACT_PLUGIN_VERSION=3.0.2
+  - WEBPACK_VERSION=4 EXTRACT_PLUGIN_VERSION=next
 
 install:
   - npm install

lib/plugin.js

@@ -2,7 +2,7 @@ var path = require('path');
 var fse = require('fs-extra');
 var _ = require('lodash');
 
-var manifestMap = {};
+const emitCountMap = new Map();
 
 function ManifestPlugin(opts) {
   this.opts = _.assign({
@@ -35,6 +35,10 @@ ManifestPlugin.prototype.apply = function(compiler) {
   var seed = this.opts.seed || {};
   var moduleAssets = {};
 
+  var outputFolder = compiler.options.output.path;
+  var outputFile = path.resolve(compiler.options.output.path, this.opts.fileName);
+  var outputName = path.relative(outputFolder, outputFile);
+
   var moduleAsset = function (module, file) {
     moduleAssets[file] = path.join(
       path.dirname(file),
@@ -43,6 +47,9 @@ ManifestPlugin.prototype.apply = function(compiler) {
   };
 
   var emit = function(compilation, compileCallback) {
+    const emitCount = emitCountMap.get(outputName) - 1
+    emitCountMap.set(outputName, emitCount);
+
     var publicPath = compilation.options.output.publicPath;
     var stats = compilation.getStats().toJson();
 
@@ -106,7 +113,7 @@ ManifestPlugin.prototype.apply = function(compiler) {
       // Don't add hot updates to manifest
       var isUpdateChunk = file.path.indexOf('hot-update') >= 0;
       // Don't add manifest from another instance
-      var isManifest = manifestMap[file.name];
+      var isManifest = emitCountMap.get(file.name) !== undefined;
 
       return !isUpdateChunk && !isManifest;
     });
@@ -158,74 +165,59 @@ ManifestPlugin.prototype.apply = function(compiler) {
       }, seed);
     }
 
-    var output = this.opts.serialize(manifest);
-
-    var outputFolder = compilation.options.output.path;
-    var outputFile = path.resolve(compilation.options.output.path, this.opts.fileName);
-    var outputName = path.relative(outputFolder, outputFile);
+    const isLastEmit = emitCount === 0
+    if (isLastEmit) {
+      var output = this.opts.serialize(manifest);
 
-    compilation.assets[outputName] = {
-      source: function() {
-        return output;
-      },
-      size: function() {
-        return output.length;
-      }
-    };
-
-    if (this.opts.writeToFileEmit) {
-      fse.outputFileSync(outputFile, output);
-    }
-
-    if (!manifestMap[outputName]) {
-      manifestMap[outputName] = {
-        running: false,
-        queue: []
+      compilation.assets[outputName] = {
+        source: function() {
+          return output;
+        },
+        size: function() {
+          return output.length;
+        }
       };
-    }
 
-    function unqueueNext() {
-      if (manifestMap[outputName].queue.length > 0) {
-        manifestMap[outputName].running = true;
-        manifestMap[outputName].queue.shift()();
-      } else {
-        manifestMap[outputName].running = false;
+      if (this.opts.writeToFileEmit) {
+        fse.outputFileSync(outputFile, output);
       }
     }
 
+    if (compiler.hooks) {
+      compiler.hooks.webpackManifestPluginAfterEmit.call(manifest);
+    } else {
+      compilation.applyPluginsAsync('webpack-manifest-plugin-after-emit', manifest, compileCallback);
+    }
+  }.bind(this);
 
-    manifestMap[outputName].queue.push(function () {
-      if (compiler.hooks) {
-        compiler.hooks.afterEmit.tap('ManifestPlugin', function(compilation) {
-          // TODO: when we deprecate webpack < 3, we can remove the queue logic
-          unqueueNext()
-        });
-      } else {
-        compiler.plugin('after-emit', function(compilation, cb) {
-          unqueueNext();
-          cb();
-        });
-
-        compilation.applyPluginsAsync('webpack-manifest-plugin-after-emit', manifest, compileCallback);
-      }
-    })
+  function beforeRun (compiler, callback) {
+    let emitCount = emitCountMap.get(outputName) || 0;
+    emitCountMap.set(outputName, emitCount + 1);
 
-    if(!manifestMap[outputName].running) {
-      manifestMap[outputName].running = true;
-      manifestMap[outputName].queue.shift()();
+    if (callback) {
+      callback();
     }
-  }.bind(this);
+  }
 
   if (compiler.hooks) {
+    const SyncWaterfallHook = require('tapable').SyncWaterfallHook;
+    compiler.hooks.webpackManifestPluginAfterEmit = new SyncWaterfallHook(['manifest']);
+
     compiler.hooks.compilation.tap('ManifestPlugin', function (compilation) {
       compilation.hooks.moduleAsset.tap('ManifestPlugin', moduleAsset);
     });
     compiler.hooks.emit.tap('ManifestPlugin', emit);
+
+    compiler.hooks.run.tap('ManifestPlugin', beforeRun);
+    compiler.hooks.watchRun.tap('ManifestPlugin', beforeRun);
   } else {
     compiler.plugin('compilation', function (compilation) {
       compilation.plugin('module-asset', moduleAsset);
     });
     compiler.plugin('emit', emit);
+
+    compiler.plugin('before-run', beforeRun);
+    compiler.plugin('watch-run', beforeRun);
   }
 };
 

package.json

@@ -9,7 +9,7 @@
   "author": "Dane Thurber <dane.thurber@gmail.com>",
   "license": "MIT",
   "engines": {
-    "node": ">=4"
+    "node": ">=6.11.5"
   },
   "peerDependencies": {
     "webpack": "2 || 3 || 4"
@@ -18,7 +18,7 @@
     "codecov": "^2.2.0",
     "css-loader": "^0.9.1",
     "extract-text-webpack-plugin": "^3.0.2",
-    "file-loader": "^0.9.0",
+    "file-loader": "^1.1.11",
     "jasmine": "^2.2.1",
     "memory-fs": "^0.2.0",
     "nyc": "^10.3.2",
@@ -40,7 +40,8 @@
   "homepage": "https://github.com/danethurber/webpack-manifest-plugin",
   "dependencies": {
     "fs-extra": "^0.30.0",
-    "lodash": ">=3.5 <5"
+    "lodash": ">=3.5 <5",
+    "tapable": "^1.0.0"
   },
   "nyc": {
     "reporter": [

spec/helpers/copy-plugin-mock.js

@@ -2,7 +2,7 @@ function FakeCopyWebpackPlugin() {
 };
 
 FakeCopyWebpackPlugin.prototype.apply = function (compiler) {
-  compiler.plugin('emit', function (compilation, callback) {
+  const emit = function (compilation, callback) {
 
     var compiledMock = '// some compilation result\n';
     compilation.assets['third.party.js'] = {
@@ -15,7 +15,13 @@ FakeCopyWebpackPlugin.prototype.apply = function (compiler) {
     };
 
     callback();
-  });
+  };
+
+  if (compiler.hooks) {
+    compiler.hooks.emit.tapAsync('FakeCopyWebpackPlugin', emit)
+  } else {
+    compiler.plugin('emit', emit);
+  }
 };
 
 module.exports = FakeCopyWebpackPlugin;

spec/plugin.integration.spec.js

@@ -10,7 +10,7 @@ var ManifestPlugin = require('../index.js');
 
 jasmine.DEFAULT_TIMEOUT_INTERVAL = 5 * 60 * 1000;
 
-var isCI = (yes, no) => process.env.CI === 'true' ? yes : no;
+const isWebpack4 = (yes, no) => webpack.version && webpack.version.slice(0, 1) === '4' ? yes : no;
 
 function webpackConfig(webpackOpts, opts) {
   return _.merge({
@@ -25,7 +25,7 @@ function webpackCompile(config, compilerOps, cb) {
 
   _.assign(compiler, compilerOps);
 
-  compiler.watch({
+  return compiler.watch({
     aggregateTimeout: 300,
     poll: true
   }, function(err, stats){
@@ -104,13 +104,18 @@ describe('ManifestPlugin using real fs', function() {
         this.manifest = null;
       }
       TestPlugin.prototype.apply = function (compiler) {
-        var self = this;
-        compiler.plugin('compilation', function (compilation) {
-          compilation.plugin('webpack-manifest-plugin-after-emit', function (manifest, callback) {
-            self.manifest = manifest;
-            callback();
+        if (compiler.hooks) {
+          compiler.hooks.webpackManifestPluginAfterEmit.tap('ManifestPlugin', (manifest) => {
+            this.manifest = manifest;
+          })
+        } else {
+          compiler.plugin('compilation', (compilation) => {
+            compilation.plugin('webpack-manifest-plugin-after-emit', (manifest, callback) => {
+              this.manifest = manifest;
+              callback();
+            });
           });
-        });
+        }
       };
 
       var testPlugin = new TestPlugin();
@@ -145,7 +150,7 @@ describe('ManifestPlugin using real fs', function() {
     });
 
     it('outputs a manifest of one file', function(done) {
-      webpackCompile({
+      const compiler = webpackCompile({
         context: __dirname,
         output: {
           filename: '[name].[hash].js',
@@ -169,6 +174,7 @@ describe('ManifestPlugin using real fs', function() {
 
         if (hashes.length === 2) {
           expect(hashes[0]).not.toEqual(hashes[1]);
+          compiler.close()
           return done();
         }
 
@@ -178,7 +184,7 @@ describe('ManifestPlugin using real fs', function() {
   });
 
   describe('multiple compilation', function() {
-    var nbCompiler = isCI(4000, 1000);
+    var nbCompiler = 10;
     var originalTimeout;
     beforeEach(function() {
       rimraf.sync(path.join(__dirname, 'output/multiple-compilation'));
@@ -199,15 +205,7 @@ describe('ManifestPlugin using real fs', function() {
         plugins: [
           new ManifestPlugin({
             seed
-          }),
-          function () {
-            var compiler = this;
-
-            compiler.plugin('after-emit', function(compilation, cb) {
-              JSON.parse(fse.readFileSync(path.join(__dirname, 'output/multiple-compilation/manifest.json')));
-              cb();
-            });
-          }
+          })
         ]
       })), {}, function() {
         var manifest = JSON.parse(fse.readFileSync(path.join(__dirname, 'output/multiple-compilation/manifest.json')))

spec/plugin.spec.js

@@ -10,6 +10,8 @@ var plugin = require('../index.js');
 var OUTPUT_DIR = path.join(__dirname, './webpack-out');
 var manifestPath = path.join(OUTPUT_DIR, 'manifest.json');
 
+const isWebpack4 = (yes, no) => webpack.version && webpack.version.slice(0, 1) === '4' ? yes : no;
+
 function webpackConfig (webpackOpts, opts) {
   return _.merge({
     output: {
@@ -315,11 +317,21 @@ describe('ManifestPlugin', function() {
       webpackCompile({
         context: __dirname,
         entry: './fixtures/file.txt',
-        module: {
+        module: isWebpack4({
+          rules: [{
+            test: /\.(txt)/,
+            use: [{
+              loader: 'file-loader',
+              options: {
+                name: '[name].[ext]'
+              }
+            }]
+          }]
+        }, {
           loaders: [
             { test: /\.(txt)/, loader: 'file-loader?name=file.[ext]' },
           ]
-        }
+        })
       }, {}, function(manifest, stats) {
         expect(manifest).toBeDefined();
         expect(manifest).toEqual({
@@ -335,11 +347,21 @@ describe('ManifestPlugin', function() {
       webpackCompile({
         context: __dirname,
         entry: './fixtures/file.txt',
-        module: {
+        module: isWebpack4({
+          rules: [{
+            test: /\.(txt)/,
+            use: [{
+              loader: 'file-loader',
+              options: {
+                name: 'outputfile.[ext]'
+              }
+            }]
+          }]
+        }, {
           loaders: [
             { test: /\.(txt)/, loader: 'file-loader?name=outputfile.[ext]' },
           ]
-        }
+        })
       }, {}, function(manifest, stats) {
         expect(manifest).toBeDefined();
         expect(manifest).toEqual({
@@ -400,15 +422,23 @@ describe('ManifestPlugin', function() {
         output: {
           filename: '[name].js'
         },
-        module: {
+        module: isWebpack4({
+          rules: [{
+            test: /\.css$/,
+            use: ExtractTextPlugin.extract({
+              fallback: 'style-loader',
+              use: 'css-loader'
+            })
+          }]
+        }, {
           loaders: [{
             test: /\.css$/,
             loader: ExtractTextPlugin.extract({
               fallback: 'style-loader',
               use: 'css-loader'
             })
           }]
-        },
+        }),
         plugins: [
           new plugin(),
           new ExtractTextPlugin({