Rule/loader config bug fix and enhancements from #256

- Don't move `loader` config into an `options` object - Validate that rule `use` config is an Array - Add a `path` property to user config when sucessfully loaded - Default rules can now be disabled by configuring them as false - Default rule loader and options are now omitted if you configure `loader` or `use` - Add a separate factory for loaders, which only handles loader+options - Add support for a webpack.config() function which receives the generated config to be edited and returned.
insin · Feb 6, 2017 · 81e5b9f · 81e5b9f
1 parent 339903e
commit 81e5b9f
Show file tree

Hide file tree

Showing 7 changed files with 277 additions and 15 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,8 +1,19 @@
 # Unreleased (in `next` branch)
 
+**Added:**
+
+- You can now provide a [`webpack.config()` function](https://github.com/insin/nwb/blob/master/docs/Configuration.md#config-function) which will be given the generated Webpack config to do whatever it wants with, then return it.
+- You can now provide [`use` config](https://github.com/insin/nwb/blob/master/docs/Configuration.md#customising-loaders) with a list of loaders in `webpack.rules` to replace a rule's default loader with chained loaders.
+- You can now [disable a default Webpack config rule](https://github.com/insin/nwb/blob/master/docs/Configuration.md#disabling-default-rules) by setting them to `false`.
+
+**Changed:**
+
+- Default options are no longer used if you provide a custom `loader` for a rule in `webpack.rules`.
+
 **Fixed:**
 
 - An output directory specified with a trailing slash is now cleaned properly when creating a build.
+-A `loader` configured for a Webpack rule is no longer moved into an `options` object when an options object isn't configured.
 - Fixed cleaning nested dirs, including a demo app's `demo/dist` dir.
 - Fixed cleaning output directories specified with a trailing slash.
 

diff --git a/docs/Configuration.md b/docs/Configuration.md
@@ -73,9 +73,12 @@ The configuration object can include the following properties:
     - [Default Rules](#default-rules)
     - [Configuring PostCSS](#configuring-postcss)
     - [Configuring CSS Preprocessor Plugins](#configuring-css-preprocessor-plugins)
+    - [Customising loaders](#customising-loaders)
+    - [Disabling default rules](#disabling-default-rules)
   - [`webpack.publicPath`](#publicpath-string) - path to static resources
   - [`webpack.uglify`](#uglify-object--false) - configure use of Webpack's `UglifyJsPlugin`
   - [`webpack.extra`](#extra-object) - an escape hatch for extra Webpack config, which will be merged into the generated config
+  - [`webpack.config`](#config-function) - an escape hatch for manually editing the generated Webpack config
 - [Karma Configuration](#karma-configuration)
   - [`karma`](#karma-object)
   - [`karma.browsers`](#browsers-arraystring--plugin) - browsers tests are run in
@@ -566,6 +569,44 @@ Using [nwb-sass](https://github.com/insin/nwb-sass) as example, you can use the
 
 There will also be a `vendor-sass-pipeline` for Sass stylesheets with the same setup as `sass-pipeline` but using a `vendor-` prefix.
 
+###### Customising loaders
+
+Use `loader` config to replace the loader used in a default rules. Any options provided for the default loader will be ignored.
+
+Provide a list of loaders via `use` config to replace a default loader with a chain of loaders, specified as loader names or `loader`/`options` objects.
+
+```js
+module.exports = {
+  webpack: {
+    rules: {
+      svg: {
+        use: [
+          {
+            loader: 'svg-inline-loader',
+            options: {classPrefix: true}
+          },
+          'image-webpack-loader'
+        ]
+      }
+    }
+  }
+}
+```
+
+###### Disabling default rules
+
+To disable inclusion of a default rule, set its id to `false`:
+
+```js
+module.exports = {
+  webpack: {
+    rules: {
+      svg: false
+    }
+  }
+}
+```
+
 ##### `publicPath`: `String`
 
 > This is just Webpack's [`output.publicPath` config](https://webpack.js.org/configuration/output/#output-publicpath) pulled up a level to make it more convenient to configure.
@@ -652,7 +693,7 @@ Note that you *must* use Webpack's own config structure in this object - e.g. to
 ```js
 var path = require('path')
 
-function(nwb) {
+module.exports = function(nwb) {
   return {
     type: 'react-app',
     webpack: {
@@ -676,6 +717,25 @@ function(nwb) {
 }
 ```
 
+##### `config`: `Function`
+
+Finally, if you need *complete* control, you can configure a `webpack.config()` function which will be given the generated config.
+
+Note that you *must* return the config object from this function.
+
+```js
+module.exports = {
+  webpack: {
+    config(config) {
+      // Change config as you wish
+
+      // You MUST return the edited config object
+      return config
+    }
+  }
+}
+```
+
 ### Karma Configuration
 
 nwb's default [Karma](http://karma-runner.github.io/) configuration uses the [Mocha](https://mochajs.org/) framework and reporter plugins for it, but you can configure your own preferences.

diff --git a/src/createWebpackConfig.js b/src/createWebpackConfig.js
@@ -12,6 +12,7 @@ import merge from 'webpack-merge'
 
 import createBabelConfig from './createBabelConfig'
 import debug from './debug'
+import {UserError} from './errors'
 import {deepToString, typeOf} from './utils'
 import StatusPlugin from './WebpackStatusPlugin'
 
@@ -21,29 +22,75 @@ import StatusPlugin from './WebpackStatusPlugin'
 let replaceArrayMerge = merge({customizeArray(a, b, key) { return b }})
 
 /**
- * Merge webpack rule config ({test, loader|use, options, include, exclude}) objects.
+ * Merge webpack rule config ({test, loader+options|use, include, exclude, ...}) objects.
  */
 export function mergeRuleConfig(defaultConfig = {}, buildConfig = {}, userConfig = {}) {
-  let rule = replaceArrayMerge(defaultConfig, buildConfig, userConfig)
+  let rule
+  // Omit the default loader and options if the user is configuring their own
+  if (defaultConfig.loader && (userConfig.loader || userConfig.use)) {
+    let {
+      loader: defaultLoader, options: defaultOptions, // eslint-disable-line no-unused-vars
+      ...defaultRuleConfig
+    } = defaultConfig
+    rule = merge(defaultRuleConfig, userConfig)
+  }
+  else {
+    rule = replaceArrayMerge(defaultConfig, buildConfig, userConfig)
+  }
   if (rule.options && Object.keys(rule.options).length === 0) {
     delete rule.options
   }
   return rule
 }
 
+/**
+ * Merge webpack loader config ({loader, options}) objects.
+ */
+export function mergeLoaderConfig(defaultConfig = {}, buildConfig = {}, userConfig = {}) {
+  let loader
+  // If the loader s being changed, only use the provided config
+  if (userConfig.loader) {
+    loader = {...userConfig}
+  }
+  else {
+    loader = replaceArrayMerge(defaultConfig, buildConfig, userConfig)
+  }
+  if (loader.options && Object.keys(loader.options).length === 0) {
+    delete loader.options
+  }
+  return loader
+}
+
 /**
  * Create a function which configures a rule identified by a unique id, with
  * the option to override defaults with build-specific and user config.
  */
 export let ruleConfigFactory = (buildConfig, userConfig = {}) =>
   (id, defaultConfig) => {
     if (id) {
+      // Allow the user to turn off rules by configuring them with false
+      if (userConfig[id] === false) {
+        return null
+      }
       let rule = mergeRuleConfig(defaultConfig, buildConfig[id], userConfig[id])
       return rule
     }
     return defaultConfig
   }
 
+/**
+ * Create a function which configures a loader identified by a unique id, with
+ * the option to override defaults with build-specific and user config.
+ */
+export let loaderConfigFactory = (buildConfig, userConfig = {}) =>
+  (id, defaultConfig) => {
+    if (id) {
+      let loader = mergeLoaderConfig(defaultConfig, buildConfig[id], userConfig[id])
+      return loader
+    }
+    return defaultConfig
+  }
+
 /**
  * Create a function which applies a prefix to a given name when a prefix is
  * given, unless the prefix ends with a name, in which case the prefix itself is
@@ -118,6 +165,7 @@ export function createStyleLoaders(loader, server, userWebpackConfig, {
  */
 export function createRules(server, buildConfig = {}, userWebpackConfig = {}, pluginConfig = {}) {
   let rule = ruleConfigFactory(buildConfig, userWebpackConfig.rules)
+  let loader = loaderConfigFactory(buildConfig, userWebpackConfig.rules)
 
   // Default options for url-loader
   let urlLoaderOptions = {
@@ -141,12 +189,12 @@ export function createRules(server, buildConfig = {}, userWebpackConfig = {}, pl
     }),
     rule('css-pipeline', {
       test: /\.css$/,
-      use: createStyleLoaders(rule, server, userWebpackConfig),
+      use: createStyleLoaders(loader, server, userWebpackConfig),
       exclude: /node_modules/,
     }),
     rule('vendor-css-pipeline', {
       test: /\.css$/,
-      use: createStyleLoaders(rule, server, userWebpackConfig, {
+      use: createStyleLoaders(loader, server, userWebpackConfig, {
         prefix: 'vendor',
       }),
       include: /node_modules/,
@@ -184,15 +232,15 @@ export function createRules(server, buildConfig = {}, userWebpackConfig = {}, pl
     // Extra rules from build config, still configurable via user config when
     // the rules specify an id.
     ...createExtraRules(buildConfig.extra, userWebpackConfig.rules),
-  ]
+  ].filter(rule => rule != null)
 
   if (pluginConfig.cssPreprocessors) {
     Object.keys(pluginConfig.cssPreprocessors).forEach(id => {
       let {test, loader: preprocessorLoader} = pluginConfig.cssPreprocessors[id]
       rules.push(
         rule(`${id}-pipeline`, {
           test,
-          use: createStyleLoaders(rule, server, userWebpackConfig, {
+          use: createStyleLoaders(loader, server, userWebpackConfig, {
             prefix: id,
             preprocessor: {id, config: {loader: preprocessorLoader}},
           }),
@@ -202,7 +250,7 @@ export function createRules(server, buildConfig = {}, userWebpackConfig = {}, pl
       rules.push(
         rule(`vendor-${id}-pipeline`, {
           test,
-          use: createStyleLoaders(rule, server, userWebpackConfig, {
+          use: createStyleLoaders(loader, server, userWebpackConfig, {
             prefix: `vendor-${id}`,
             preprocessor: {id, config: {loader: preprocessorLoader}},
           }),
@@ -567,5 +615,14 @@ export default function createWebpackConfig(buildConfig, nwbPluginConfig = {}, u
     webpackConfig = merge(webpackConfig, userWebpackConfig.extra)
   }
 
+  // Finally, give them a chance to do whatever they want with the generated
+  // config.
+  if (typeOf(userWebpackConfig.config) === 'function') {
+    webpackConfig = userWebpackConfig.config(webpackConfig)
+    if (!webpackConfig) {
+      throw new UserError(`webpack.config() in ${userConfig.path} didn't return anything - it must return the Webpack config object.`)
+    }
+  }
+
   return webpackConfig
 }
diff --git a/src/getUserConfig.js b/src/getUserConfig.js
@@ -115,8 +115,8 @@ function checkForRedundantCompatAliases(projectType, aliases, configPath, report
 export function prepareWebpackRuleConfig(rules) {
   Object.keys(rules).forEach(ruleId => {
     let rule = rules[ruleId]
-    if (rule.options) return
-    let {exclude, include, test, ...options} = rule // eslint-disable-line no-unused-vars
+    if (rule.use || rule.options) return
+    let {exclude, include, test, loader, ...options} = rule // eslint-disable-line no-unused-vars
     if (Object.keys(options).length > 0) {
       rule.options = options
       Object.keys(options).forEach(prop => delete rule[prop])
@@ -353,15 +353,23 @@ export function processUserConfig({
     }
     else {
       Object.keys(userConfig.webpack.rules).forEach(ruleId => {
-        if (userConfig.webpack.rules[ruleId].query) {
+        let rule = userConfig.webpack.rules[ruleId]
+        if (rule.query) {
           if (!warnedAboutWebpackRuleQuery) {
             report.deprecated('query Object in webpack.rules config',
               `Deprecated as of nwb v0.15 - an ${chalk.green('options')} Object should now be used to specify rule options, to match Webpack 2 config.`
             )
             warnedAboutWebpackRuleQuery = true
           }
-          userConfig.webpack.rules[ruleId].options = userConfig.webpack.rules[ruleId].query
-          delete userConfig.webpack.rules[ruleId].query
+          rule.options = rule.query
+          delete rule.query
+        }
+        if (rule.use && typeOf(rule.use) !== 'array') {
+          report.error(
+            `webpack.rules.${ruleId}.use`,
+            `type: ${typeOf(rule.use)}`,
+            'Must be an Array.'
+           )
         }
       })
       prepareWebpackRuleConfig(userConfig.webpack.rules)
@@ -396,6 +404,14 @@ export function processUserConfig({
     }
   }
 
+  if ('config' in userConfig.webpack && typeOf(userConfig.webpack.config) !== 'function') {
+    report.error(
+      `webpack.config`,
+      `type: ${typeOf(userConfig.webpack.config)}`,
+      'Must be a Function.'
+     )
+  }
+
   if (report.hasErrors()) {
     throw new ConfigValidationError(report)
   }
@@ -444,5 +460,11 @@ export default function getUserConfig(args = {}, options = {}) {
     }
   }
 
-  return processUserConfig({args, check, required, userConfig, userConfigPath})
+  userConfig = processUserConfig({args, check, required, userConfig, userConfigPath})
+
+  if (configFileExists) {
+    userConfig.path = userConfigPath
+  }
+
+  return userConfig
 }
diff --git a/src/webpackBuild.js b/src/webpackBuild.js
@@ -23,7 +23,13 @@ export default function webpackBuild(name, args, buildConfig, cb) {
     buildConfig = buildConfig(args)
   }
 
-  let webpackConfig = createWebpackConfig(buildConfig, pluginConfig, userConfig)
+  let webpackConfig
+  try {
+    webpackConfig = createWebpackConfig(buildConfig, pluginConfig, userConfig)
+  }
+  catch (e) {
+    return cb(e)
+  }
 
   debug('webpack config: %s', deepToString(webpackConfig))