minor #99 Refactor plugin configuration (davidmpaz)

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

Discussion
----------

Refactor plugin configuration

Extract encore built in plugin configuration logic into plugin utils.

First step toward allowing to be able to configure plugins through options and have more modular integration for plugins inside encore. From now on only public API with proper options is needed,
for most cases.

This changes are internals and tries to improve plugin architecture inside encore. Please take a look on it and feedback are welcome :). So far I see improvements for several issues:

* #2 - Point .2 ...for plugins we should do the same as for loaders, from point of view of module split (utilities).
* #63 - Better (more modular) integration of the plugin as well its configuration.
* #65 - Same as #63
* #79 - Now it should be possible to pass options and/or check for flags to apply plugin or not.
* #87 - Some small steps to make clean plugin more configurable. Public API still need to be changed though.

thanks in advance.

Commits
-------

65adfdc Refactor plugin configuration

Author: weaverryan

lib/config-generator.js

@@ -9,28 +9,28 @@
 
 'use strict';
 
-const webpack = require('webpack');
-const ExtractTextPlugin = require('extract-text-webpack-plugin');
 const extractText = require('./loaders/extract-text');
-const ManifestPlugin = require('./webpack/webpack-manifest-plugin');
-const DeleteUnusedEntriesJSPlugin = require('./webpack/delete-unused-entries-js-plugin');
-const AssetOutputDisplayPlugin = require('./friendly-errors/asset-output-display-plugin');
-const CleanWebpackPlugin = require('clean-webpack-plugin');
-const WebpackChunkHash = require('webpack-chunk-hash');
-const FriendlyErrorsWebpackPlugin = require('friendly-errors-webpack-plugin');
-const missingLoaderTransformer = require('./friendly-errors/transformers/missing-loader');
-const missingLoaderFormatter = require('./friendly-errors/formatters/missing-loader');
-const missingPostCssConfigTransformer = require('./friendly-errors/transformers/missing-postcss-config');
-const missingPostCssConfigFormatter = require('./friendly-errors/formatters/missing-postcss-config');
-const vueUnactivatedLoaderTransformer = require('./friendly-errors/transformers/vue-unactivated-loader-error');
-const vueUnactivatedLoaderFormatter = require('./friendly-errors/formatters/vue-unactivated-loader-error');
 const pathUtil = require('./config/path-util');
+// loaders utils
 const cssLoaderUtil = require('./loaders/css');
 const sassLoaderUtil = require('./loaders/sass');
 const lessLoaderUtil = require('./loaders/less');
 const babelLoaderUtil = require('./loaders/babel');
 const tsLoaderUtil = require('./loaders/typescript');
 const vueLoaderUtil = require('./loaders/vue');
+// plugins utils
+const extractTextPluginUtil = require('./plugins/extract-text');
+const deleteUnusedEntriesPluginUtil = require('./plugins/delete-unused-entries');
+const manifestPluginUtil = require('./plugins/manifest');
+const loaderOptionsPluginUtil = require('./plugins/loader-options');
+const versioningPluginUtil = require('./plugins/versioning');
+const variableProviderPluginUtil = require('./plugins/variable-provider');
+const cleanPluginUtil = require('./plugins/clean');
+const commonChunksPluginUtil = require('./plugins/common-chunks');
+const definePluginUtil = require('./plugins/define');
+const uglifyPluginUtil = require('./plugins/uglify');
+const friendlyErrorPluginUtil = require('./plugins/friendly-errors');
+const assetOutputDisplay = require('./plugins/asset-output-display');
 
 class ConfigGenerator {
     /**
@@ -180,173 +180,32 @@ class ConfigGenerator {
     buildPluginsConfig() {
         let plugins = [];
 
-        /*
-         * All CSS/SCSS content (due to the loaders above) will be
-         * extracted into an [entrypointname].css files. The result
-         * is that NO css will be inlined, *except* CSS that is required
-         * in an async way (e.g. via require.ensure()).
-         *
-         * This may not be ideal in some cases, but it's at least
-         * predictable. It means that you must manually add a
-         * link tag for an entry point's CSS (unless no CSS file
-         * was imported - in which case no CSS file will be dumped).
-         */
-        plugins.push(new ExtractTextPlugin({
-            filename: this.webpackConfig.useVersioning ? '[name].[contenthash].css' : '[name].css',
-            // if true, async CSS (e.g. loaded via require.ensure())
-            // is extracted to the entry point CSS. If false, it's
-            // inlined in the AJAX-loaded .js file.
-            allChunks: false
-        }));
+        extractTextPluginUtil(plugins, this.webpackConfig);
 
         // register the pure-style entries that should be deleted
-        plugins.push(new DeleteUnusedEntriesJSPlugin(
-            // transform into an Array
-            [... this.webpackConfig.styleEntries.keys()]
-        ));
-
-        /*
-         * Dump the manifest.json file
-         */
-        let manifestPrefix = this.webpackConfig.manifestKeyPrefix;
-        if (null === manifestPrefix) {
-            // by convention, we remove the opening slash on the manifest keys
-            manifestPrefix = this.webpackConfig.publicPath.replace(/^\//,'');
-        }
-        plugins.push(new ManifestPlugin({
-            basePath: manifestPrefix,
-            // guarantee the value uses the public path (or CDN public path)
-            publicPath: this.webpackConfig.getRealPublicPath(),
-            // always write a manifest.json file, even with webpack-dev-server
-            writeToFileEmit: true,
-        }));
-
-        /*
-         * This section is a bit mysterious. The "minimize"
-         * true is read and used to minify the CSS.
-         * But as soon as this plugin is included
-         * at all, SASS begins to have errors, until the context
-         * and output options are specified. At this time, I'm
-         * not totally sure what's going on here
-         * https://github.com/jtangelder/sass-loader/issues/285
-         */
-        plugins.push(new webpack.LoaderOptionsPlugin({
-            debug: !this.webpackConfig.isProduction(),
-            options: {
-                context: this.webpackConfig.getContext(),
-                output: { path: this.webpackConfig.outputPath }
-            }
-        }));
-
-        /*
-         * With versioning, the "chunkhash" used in the filenames and
-         * the module ids (i.e. the internal names of modules that
-         * are required) become important. Specifically:
-         *
-         * 1) If the contents of a module don't change, then you don't want its
-         *    internal module id to change. Otherwise, whatever file holds the
-         *    webpack "manifest" will change because the module id will change.
-         *    Solved by HashedModuleIdsPlugin or NamedModulesPlugin
-         *
-         * 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
-         */
-        if (this.webpackConfig.isProduction()) {
-            // shorter, and obfuscated module ids (versus NamedModulesPlugin)
-            // makes the final assets *slightly* larger, but prevents contents
-            // from sometimes changing when nothing really changed
-            plugins.push(new webpack.HashedModuleIdsPlugin());
-        } else {
-            // human-readable module names, helps debug in HMR
-            // enable always when not in production for consistency
-            plugins.push(new webpack.NamedModulesPlugin());
-        }
+        deleteUnusedEntriesPluginUtil(plugins, this.webpackConfig);
 
-        if (this.webpackConfig.useVersioning) {
-            // enables the [chunkhash] ability
-            plugins.push(new WebpackChunkHash());
-        }
+        // Dump the manifest.json file
+        manifestPluginUtil(plugins, this.webpackConfig);
 
-        if (Object.keys(this.webpackConfig.providedVariables).length > 0) {
-            plugins = plugins.concat([
-                new webpack.ProvidePlugin(this.webpackConfig.providedVariables)
-            ]);
-        }
+        loaderOptionsPluginUtil(plugins, this.webpackConfig);
 
-        if (this.webpackConfig.cleanupOutput) {
-            plugins.push(
-                new CleanWebpackPlugin(['**/*'], {
-                    root: this.webpackConfig.outputPath,
-                    verbose: false,
-                })
-            );
-        }
+        versioningPluginUtil(plugins, this.webpackConfig);
 
-        // if we're extracting a vendor chunk, set it up!
-        if (this.webpackConfig.sharedCommonsEntryName) {
-            plugins = plugins.concat([
-                new webpack.optimize.CommonsChunkPlugin({
-                    name: [
-                        this.webpackConfig.sharedCommonsEntryName,
-                        /*
-                         * Always dump a 2nd file - manifest.json that
-                         * will contain the webpack manifest information.
-                         * This changes frequently, and without this line,
-                         * it would be packaged inside the "shared commons entry"
-                         * file - e.g. vendor.js, which would prevent long-term caching.
-                         */
-                        'manifest'
-                    ],
-                    minChunks: Infinity,
-                }),
-            ]);
-        }
+        variableProviderPluginUtil(plugins, this.webpackConfig);
 
-        if (this.webpackConfig.isProduction()) {
-            plugins = plugins.concat([
-                new webpack.DefinePlugin({
-                    'process.env': {
-                        NODE_ENV: '"production"'
-                    }
-                }),
-
-                // todo - options here should be configurable
-                new webpack.optimize.UglifyJsPlugin({
-                    sourceMap: this.webpackConfig.useSourceMaps
-                })
-            ]);
-        }
+        cleanPluginUtil(plugins, this.webpackConfig, ['**/*']);
 
-        const friendlyErrorsPlugin = new FriendlyErrorsWebpackPlugin({
-            clearConsole: false,
-            additionalTransformers: [
-                missingLoaderTransformer,
-                missingPostCssConfigTransformer,
-                vueUnactivatedLoaderTransformer
-            ],
-            additionalFormatters: [
-                missingLoaderFormatter,
-                missingPostCssConfigFormatter,
-                vueUnactivatedLoaderFormatter
-            ],
-            compilationSuccessInfo: {
-                messages: []
-            }
-        });
-        plugins.push(friendlyErrorsPlugin);
+        commonChunksPluginUtil(plugins, this.webpackConfig);
 
-        if (!this.webpackConfig.useDevServer()) {
-            const outputPath = pathUtil.getRelativeOutputPath(this.webpackConfig);
-            plugins.push(new AssetOutputDisplayPlugin(outputPath, friendlyErrorsPlugin));
-        }
+        // todo - options here should be configurable
+        definePluginUtil(plugins, this.webpackConfig);
+        uglifyPluginUtil(plugins, this.webpackConfig);
+
+        let friendlyErrorPlugin = friendlyErrorPluginUtil();
+        plugins.push(friendlyErrorPlugin);
+
+        assetOutputDisplay(plugins, this.webpackConfig, friendlyErrorPlugin);
 
         this.webpackConfig.plugins.forEach(function(plugin) {
             plugins.push(plugin);

lib/plugins/asset-output-display.js

@@ -0,0 +1,30 @@
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+'use strict';
+
+const pathUtil = require('../config/path-util');
+const AssetOutputDisplayPlugin = require('../friendly-errors/asset-output-display-plugin');
+
+/**
+ * Updates plugins array passed adding AssetOutputDisplayPlugin instance
+ *
+ * @param {Array} plugins
+ * @param {WebpackConfig} webpackConfig
+ * @param {FriendlyErrorsWebpackPlugin} friendlyErrorsPlugin
+ * @return {void}
+ */
+module.exports = function(plugins, webpackConfig, friendlyErrorsPlugin) {
+    if (webpackConfig.useDevServer()) {
+        return;
+    }
+
+    const outputPath = pathUtil.getRelativeOutputPath(webpackConfig);
+    plugins.push(new AssetOutputDisplayPlugin(outputPath, friendlyErrorsPlugin));
+};

lib/plugins/clean.js

@@ -0,0 +1,35 @@
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+'use strict';
+
+const CleanWebpackPlugin = require('clean-webpack-plugin');
+
+/**
+ * Updates plugins array passed adding CleanWebpackPlugin instance
+ *
+ * @param {Array} plugins to push to
+ * @param {WebpackConfig} webpackConfig read only variable
+ * @param {Array} paths to clean
+ * @param {Object} cleanUpOptions
+ * @return {void}
+ */
+module.exports = function(plugins, webpackConfig, paths, cleanUpOptions = {}) {
+
+    if (!webpackConfig.cleanupOutput) {
+        return;
+    }
+
+    let config = Object.assign({}, cleanUpOptions, {
+        root: webpackConfig.outputPath,
+        verbose: false,
+    });
+
+    plugins.push(new CleanWebpackPlugin(paths, config));
+};

lib/plugins/common-chunks.js

@@ -0,0 +1,40 @@
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+'use strict';
+
+const webpack = require('webpack');
+
+/**
+ * @param {Array} plugins
+ * @param {WebpackConfig} webpackConfig
+ * @return {void}
+ */
+module.exports = function(plugins, webpackConfig) {
+
+    if (!webpackConfig.sharedCommonsEntryName) {
+        return;
+    }
+
+    // if we're extracting a vendor chunk, set it up!
+    plugins.push(new webpack.optimize.CommonsChunkPlugin({
+        name: [
+            webpackConfig.sharedCommonsEntryName,
+            /*
+             * Always dump a 2nd file - manifest.json that
+             * will contain the webpack manifest information.
+             * This changes frequently, and without this line,
+             * it would be packaged inside the "shared commons entry"
+             * file - e.g. vendor.js, which would prevent long-term caching.
+             */
+            'manifest'
+        ],
+        minChunks: Infinity,
+    }));
+};

lib/plugins/define.js

@@ -0,0 +1,34 @@
+/*
+ * This file is part of the Symfony package.
+ *
+ * (c) Fabien Potencier <fabien@symfony.com>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+'use strict';
+
+const webpack = require('webpack');
+
+/**
+ * @param {Array} plugins
+ * @param {WebpackConfig} webpackConfig
+ * @param {Object} defineOptions
+ * @return {void}
+ */
+module.exports = function(plugins, webpackConfig, defineOptions = {}) {
+
+    if (!webpackConfig.isProduction()) {
+        return;
+    }
+
+    let defineConfig = Object.assign({}, defineOptions, {
+        'process.env': {
+            NODE_ENV: '"production"'
+        }
+    });
+    let define = new webpack.DefinePlugin(defineConfig);
+
+    plugins.push(define);
+};