From 903abc49e1a3651c0b02dcfcdd0a19232da3df5b Mon Sep 17 00:00:00 2001 From: Lyrkan Date: Fri, 22 Sep 2017 23:52:15 +0200 Subject: [PATCH 1/2] Add Encore.sortPlugins() method --- index.js | 31 ++++++++++ lib/WebpackConfig.js | 9 +++ lib/config-generator.js | 36 +++++++++-- test/WebpackConfig.js | 23 +++++++ test/config-generator.js | 126 +++++++++++++++++++++++++++++++++++++++ test/index.js | 9 +++ 6 files changed, 230 insertions(+), 4 deletions(-) diff --git a/index.js b/index.js index d27b2f1a..25296c1f 100644 --- a/index.js +++ b/index.js @@ -278,6 +278,37 @@ const publicApi = { return this; }, + /** + * Sorts plugins so they are added to Webpack in the given order. + * + * This method takes an array of the constructor functions used to + * initialize the plugins you want to sort. + * + * You only need to provide the plugins that need to be sorted: + * the other ones will keep their default position or respect + * the order in which the Encore.addPlugin() method has been called. + * + * For example: + * + * const webpack = require('webpack'); + * const Plugin1 = require('plugin1'); + * const Plugin2 = require('plugin2'); + * + * Encore.sortPlugins([ + * Plugin2, + * webpack.optimize.UglifyJsPlugin, + * Plugin1 + * ]) + * + * @param {Array} sortOrder + * @return {exports} + */ + sortPlugins(sortOrder) { + webpackConfig.sortPlugins(sortOrder); + + return this; + }, + /** * Adds a custom loader config * diff --git a/lib/WebpackConfig.js b/lib/WebpackConfig.js index 3e89a013..4f408127 100644 --- a/lib/WebpackConfig.js +++ b/lib/WebpackConfig.js @@ -45,6 +45,7 @@ class WebpackConfig { this.sharedCommonsEntryName = null; this.providedVariables = {}; this.configuredFilenames = {}; + this.sortPluginsOrder = []; // Features/Loaders flags this.useVersioning = false; @@ -256,6 +257,14 @@ class WebpackConfig { this.plugins.push(plugin); } + sortPlugins(sortOrder = []) { + if (!Array.isArray(sortOrder)) { + throw new Error('Argument 1 to sortPlugins() must be an Array of constructor functions (e.g.: [ExtractTextPlugin, CleanWebpackPlugin])'); + } + + this.sortPluginsOrder = sortOrder; + } + addLoader(loader) { this.loaders.push(loader); } diff --git a/lib/config-generator.js b/lib/config-generator.js index b4d9a135..5e134248 100644 --- a/lib/config-generator.js +++ b/lib/config-generator.js @@ -214,9 +214,6 @@ class ConfigGenerator { // register the pure-style entries that should be deleted deleteUnusedEntriesPluginUtil(plugins, this.webpackConfig); - // Dump the manifest.json file - manifestPluginUtil(plugins, this.webpackConfig); - loaderOptionsPluginUtil(plugins, this.webpackConfig); versioningPluginUtil(plugins, this.webpackConfig); @@ -236,11 +233,42 @@ class ConfigGenerator { assetOutputDisplay(plugins, this.webpackConfig, friendlyErrorPlugin); + // Custom plugins this.webpackConfig.plugins.forEach(function(plugin) { plugins.push(plugin); }); - return plugins; + // Dump the manifest.json file last so it includes all + // emitted assets. + manifestPluginUtil(plugins, this.webpackConfig); + + // Sort plugins based on the order given to Encore.sortPlugins() + const sortOrder = [...this.webpackConfig.sortPluginsOrder]; + const sortedPlugins = []; + + while (plugins.length) { + const currentPlugin = plugins.shift(); + const sortedIndex = sortOrder.findIndex((type) => currentPlugin instanceof type); + + if (sortedIndex >= 0) { + // If the current plugin type has been found in the sortOrder array + // we move every instances of the types that are supposed to be placed + // before it. + for (let i = 0; i < sortedIndex; i++) { + const currentType = sortOrder.shift(); + + // Add plugins of the current type to the sorted array + sortedPlugins.push(...plugins.filter((plugin) => plugin instanceof currentType)); + + // Remove plugins of the current type from the unsorted array + plugins = plugins.filter((plugin) => !(plugin instanceof currentType)); + } + } + + sortedPlugins.push(currentPlugin); + } + + return sortedPlugins; } buildStatsConfig() { diff --git a/test/WebpackConfig.js b/test/WebpackConfig.js index 3f49fd68..872dd3d8 100644 --- a/test/WebpackConfig.js +++ b/test/WebpackConfig.js @@ -617,6 +617,29 @@ describe('WebpackConfig object', () => { }); }); + describe('sortPlugins', () => { + it('Calling method sets it', () => { + const config = createConfig(); + + expect(config.sortPluginsOrder.length).to.equal(0); + + config.sortPlugins([ + new webpack.IgnorePlugin(/^\.\/locale$/, /moment$/), + new webpack.optimize.UglifyJsPlugin() + ]); + + expect(config.sortPluginsOrder.length).to.equal(2); + }); + + it('Calling with non-array throws an error', () => { + const config = createConfig(); + + expect(() => { + config.sortPlugins('foo'); + }).to.throw('must be an Array'); + }); + }); + describe('addLoader', () => { it('Adds a new loader', () => { const config = createConfig(); diff --git a/test/config-generator.js b/test/config-generator.js index 04923f6c..d8f21e11 100644 --- a/test/config-generator.js +++ b/test/config-generator.js @@ -61,6 +61,35 @@ function findRule(regex, rules) { throw new Error(`No rule found for regex ${regex}`); } +/** + * Test if all the plugins respect the expected order. + * + * @param {Array} plugins + * @param {Array} expectedOrder + * @returns {void} + */ +function assertPluginsOrder(plugins, expectedOrder) { + let expectedIndex = 0; + let previousPlugin = null; + + for (let plugin of plugins) { + if (previousPlugin && (plugin instanceof previousPlugin.constructor)) { + continue; + } + + const actualIndex = expectedOrder.findIndex((type) => plugin instanceof type); + if (actualIndex !== -1) { + if (actualIndex !== expectedIndex) { + throw new Error(`Expected plugin ${plugin.constructor.name} to be at index ${expectedIndex}, got ${actualIndex}`); + } else { + expectedIndex++; + } + } + + previousPlugin = plugin; + } +} + describe('The config-generator function', () => { describe('Test basic output properties', () => { @@ -557,4 +586,101 @@ describe('The config-generator function', () => { }); }); }); + + describe('Test plugins sorting', () => { + function CustomPlugin1() {} + function CustomPlugin2() {} + function CustomPlugin3() {} + + it('Without calling sortPlugins', () => { + const config = createConfig(); + config.outputPath = '/tmp/public/build'; + config.setPublicPath('/build/'); + config.cleanupOutputBeforeBuild(); + config.addPlugin(new CustomPlugin1()); + config.addPlugin(new CustomPlugin2()); + + const actualConfig = configGenerator(config); + assertPluginsOrder( + actualConfig.plugins, + [ + ExtractTextPlugin, + CleanWebpackPlugin, + CustomPlugin1, + CustomPlugin2, + ManifestPlugin + ] + ); + }); + + it('When calling sortPlugins without custom plugins', () => { + const config = createConfig(); + config.outputPath = '/tmp/public/build'; + config.setPublicPath('/build/'); + config.cleanupOutputBeforeBuild(); + + config.sortPlugins([ + CleanWebpackPlugin, + ManifestPlugin, + ExtractTextPlugin + ]); + + const actualConfig = configGenerator(config); + assertPluginsOrder( + actualConfig.plugins, + [ + CleanWebpackPlugin, + ManifestPlugin, + ExtractTextPlugin + ] + ); + }); + + it('When calling sortPlugins with custom plugins', () => { + const config = createConfig(); + config.outputPath = '/tmp/public/build'; + config.setPublicPath('/build/'); + config.cleanupOutputBeforeBuild(); + + config.addPlugin(new CustomPlugin1()); + config.addPlugin(new CustomPlugin2()); + config.addPlugin(new CustomPlugin3()); + + // It should work with multiple instances of the same plugin + config.addPlugin(new CustomPlugin3()); + config.addPlugin(new CustomPlugin2()); + config.addPlugin(new CustomPlugin1()); + config.addPlugin(new CustomPlugin3()); + + config.sortPlugins([ + CustomPlugin1, + CleanWebpackPlugin, + CustomPlugin2, + CustomPlugin3, + ManifestPlugin, + ExtractTextPlugin + ]); + + const actualConfig = configGenerator(config); + assertPluginsOrder( + actualConfig.plugins, + [ + CustomPlugin1, + CleanWebpackPlugin, + CustomPlugin2, + CustomPlugin3, + ManifestPlugin, + ExtractTextPlugin + ] + ); + + // It shouldn't remove any plugin + expect(actualConfig.plugins.filter((plugin) => plugin instanceof CleanWebpackPlugin).length).to.equal(1); + expect(actualConfig.plugins.filter((plugin) => plugin instanceof ManifestPlugin).length).to.equal(1); + expect(actualConfig.plugins.filter((plugin) => plugin instanceof ExtractTextPlugin).length).to.equal(1); + expect(actualConfig.plugins.filter((plugin) => plugin instanceof CustomPlugin1).length).to.equal(2); + expect(actualConfig.plugins.filter((plugin) => plugin instanceof CustomPlugin2).length).to.equal(2); + expect(actualConfig.plugins.filter((plugin) => plugin instanceof CustomPlugin3).length).to.equal(3); + }); + }); }); diff --git a/test/index.js b/test/index.js index 5c8ce33c..058c3513 100644 --- a/test/index.js +++ b/test/index.js @@ -71,6 +71,15 @@ describe('Public API', () => { }); + describe('sortPlugins', () => { + + it('must return the API object', () => { + const returnedValue = api.sortPlugins([]); + expect(returnedValue).to.equal(api); + }); + + }); + describe('addLoader', () => { it('must return the API object', () => { From 9acb0aba47db606ad4dc5408658571c2c347c41e Mon Sep 17 00:00:00 2001 From: Lyrkan Date: Sat, 23 Sep 2017 12:52:27 +0200 Subject: [PATCH 2/2] Remove unnecessary check when sorting plugins --- lib/config-generator.js | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/lib/config-generator.js b/lib/config-generator.js index 5e134248..a4468196 100644 --- a/lib/config-generator.js +++ b/lib/config-generator.js @@ -250,19 +250,17 @@ class ConfigGenerator { const currentPlugin = plugins.shift(); const sortedIndex = sortOrder.findIndex((type) => currentPlugin instanceof type); - if (sortedIndex >= 0) { - // If the current plugin type has been found in the sortOrder array - // we move every instances of the types that are supposed to be placed - // before it. - for (let i = 0; i < sortedIndex; i++) { - const currentType = sortOrder.shift(); - - // Add plugins of the current type to the sorted array - sortedPlugins.push(...plugins.filter((plugin) => plugin instanceof currentType)); - - // Remove plugins of the current type from the unsorted array - plugins = plugins.filter((plugin) => !(plugin instanceof currentType)); - } + // If the current plugin type has been found in the sortOrder array + // we move every instances of the types that are supposed to be placed + // before it. + for (let i = 0; i < sortedIndex; i++) { + const currentType = sortOrder.shift(); + + // Add plugins of the current type to the sorted array + sortedPlugins.push(...plugins.filter((plugin) => plugin instanceof currentType)); + + // Remove plugins of the current type from the unsorted array + plugins = plugins.filter((plugin) => !(plugin instanceof currentType)); } sortedPlugins.push(currentPlugin);