feat: opt-in rules for loader include/exclude (#99)

Author: jonathanglasmeyer

CONTRIBUTING.md

@@ -13,6 +13,18 @@
 ### Testing workflow
 Have a look at `test/README.md` to see how testing works. Run `npm run watch:test` while developing. Have fun! :)
 
+### Tips for working with Joi
+- Use `!!` at the beginning of your custom error string to get rid of the key at the beginning of the error message. Example:
+```js
+const ERROR_MSG = '!!A custom error message without a "key" in front of it'
+
+// Look at node_modules/joi/lib/language.js to know
+// which key to override in the options.language object
+const schema = Joi
+  .string()
+  .options({ language: { string: { base: ERROR_MSG } } })
+```
+
 ## Editor setup
 
 Please install [editorconfig plugin](http://editorconfig.org/#download) for your preferred editor.

README.md

@@ -79,6 +79,8 @@ module.exports = validate(config, { schemaExtension: yourSchemaExtension })
 #### Rules
 Some validations do more than just validating your data shape, they check for best practices and do "more" which you might want to opt out of / in to. This is an overview of the available rules (we just started with this, this list will grow :)):
 - **no-root-files-node-modules-nameclash** (default: true): this checks that files/folders that are found in directories specified via webpacks `resolve.root` option do not nameclash with `node_modules` packages. This prevents nasty path resolving bugs (for a motivating example, have a look at [this redux issue](https://github.com/reactjs/redux/issues/1681)).
+- **loader-enforce-include-or-exclude** (default: false): enforce that [loader](https://webpack.github.io/docs/configuration.html#module-loaders) objects use `include` or/and `exclude`, throw when neither is supplied. Without supplying one of these conditions it is too easy to process too many files, for example your `node_modules` folder.
+- **loader-prefer-include** (default: false): enforce that [loader](https://webpack.github.io/docs/configuration.html#module-loaders) objects use `include` and not `exclude`. `exclude` makes it easy to match too many files, which might inadvertently slow your build down.
 
 You opt in/out of rules by using the `rules` option:
 ```js

package.json

@@ -41,13 +41,14 @@
     "basename": "0.1.2",
     "chalk": "1.1.3",
     "commander": "2.9.0",
+    "common-tags": "0.1.1",
     "cross-env": "^1.0.7",
     "find-node-modules": "^1.0.1",
     "joi": "9.0.0-0",
     "lodash": "4.11.1",
     "npmlog": "2.0.3",
-    "yargs": "4.7.1",
-    "shelljs": "0.7.0"
+    "shelljs": "0.7.0",
+    "yargs": "4.7.1"
   },
   "devDependencies": {
     "autoprefixer": "6.3.6",

src/index.js

@@ -1,6 +1,6 @@
 import Joi from 'joi'
 import chalk from 'chalk'
-import moduleSchema from './properties/module'
+import moduleSchemaFn from './properties/module'
 import entrySchema from './properties/entry'
 import contextSchema from './properties/context'
 import devtoolSchema from './properties/devtool'
@@ -19,6 +19,7 @@ sh.config.silent = true
 
 const makeSchema = (schemaOptions, schemaExtension) => {
   const resolveSchema = resolveSchemaFn(schemaOptions)
+  const moduleSchema = moduleSchemaFn(schemaOptions)
 
   const schema = Joi.object({
     amd: Joi.object(),
@@ -59,6 +60,8 @@ const makeSchema = (schemaOptions, schemaExtension) => {
 const defaultSchemaOptions = {
   rules: {
     'no-root-files-node-modules-nameclash': true,
+    'loader-enforce-include-or-exclude': false,
+    'loader-prefer-include': false,
   },
 }
 

src/properties/module/index.js

@@ -1,36 +1,86 @@
 import Joi from 'joi'
+import { oneLine } from 'common-tags'
 
-export const CONDITION_MESSAGE =
-  'may be a RegExp (tested against absolute path), ' +
-  'a string containing the absolute path, a function(absPath): bool, ' +
-  'or an array of one of these combined with “and”.'
+export const CONDITION_MESSAGE = oneLine`
+  may be a RegExp (tested against absolute path),
+  a string containing the absolute path, a function(absPath): bool,
+  or an array of one of these combined with “and”.`
 
-export const LOADERS_QUERY_MESSAGE =
-  'You can only pass the \`query\` property when you specify your ' +
-  'loader with the singular \`loader\` property'
+export const LOADERS_QUERY_MESSAGE = oneLine`
+  You can only pass the \`query\` property when you specify your
+  loader with the singular \`loader\` property`
+
+const LOADER_INCLUDE_OR_EXCLUDE_MESSAGE = oneLine`!!
+  Please supply \`include\` or/and \`exclude\` to narrow down which
+  files will be processed by this loader.
+  Otherwise it's too easy
+  to process too many files, for example your \`node_modules\` (loader-enforce-include-or-exclude).
+`
+
+const LOADER_INCLUDE_REQUIRED = oneLine`
+  is required. You should use \`include\` to specify which files should be processed
+  by this loader. (loader-prefer-include)
+`
+
+const LOADER_EXCLUDE_FORBIDDEN = oneLine`
+  should not be used. Reason: \`exclude\` makes it easy to match too many files,
+  which might inadvertently slow your build down. Use \`include\` instead. (loader-prefer-include)
+`
 
 const conditionSchema = Joi.array().items([
   Joi.string(),
   Joi.object().type(RegExp),
   Joi.func().arity(1),
 ]).single().options({ language: { array: { includesSingle: CONDITION_MESSAGE } } })
 
-const loaderSchema = Joi.object({
-  test: conditionSchema.required(),
-  exclude: conditionSchema,
-  include: conditionSchema,
-  loader: Joi.string(),
-  query: Joi.object(),
-  loaders: Joi.array().items(Joi.string()),
-})
-  .xor('loaders', 'loader')
-  .nand('loaders', 'query').options({ language: { object: { nand: LOADERS_QUERY_MESSAGE } } })
-
-const loadersSchema = Joi.array().items(loaderSchema)
-
-export default Joi.object({
-  loaders: loadersSchema.required(),
-  preLoaders: loadersSchema,
-  postLoaders: loadersSchema,
-  noParse: Joi.array(Joi.object().type(RegExp)).single(),
-})
+const loaderSchemaFn = ({ rules }) => {
+  let rule = Joi.object({
+    test: conditionSchema.required(),
+    exclude: conditionSchema,
+    include: conditionSchema,
+    loader: Joi.string(),
+    query: Joi.object(),
+    loaders: Joi.array().items(Joi.string()),
+  })
+    .xor('loaders', 'loader')
+    .nand('loaders', 'query')
+    .options({ language: { object: { nand: LOADERS_QUERY_MESSAGE } } })
+
+  if (rules['loader-enforce-include-or-exclude']) {
+    rule = rule
+      .or('exclude', 'include')
+      .label('loader')
+      .options({ language: { object: { missing: LOADER_INCLUDE_OR_EXCLUDE_MESSAGE } } })
+  }
+
+  if (rules['loader-prefer-include']) {
+    rule = rule.concat(
+      Joi.object({
+        include: conditionSchema.required(),
+        exclude: Joi.any().forbidden(),
+      })
+      .options({
+        language: {
+          any: {
+            required: LOADER_INCLUDE_REQUIRED,
+            unknown: LOADER_EXCLUDE_FORBIDDEN,
+          },
+        },
+      })
+    )
+  }
+
+  return rule
+}
+
+
+export default (options) => {
+  const loaderSchema = loaderSchemaFn(options)
+  const loadersSchema = Joi.array().items(loaderSchema)
+  return Joi.object({
+    loaders: loadersSchema.required(),
+    preLoaders: loadersSchema,
+    postLoaders: loadersSchema,
+    noParse: Joi.array(Joi.object().type(RegExp)).single(),
+  })
+}

src/properties/module/index.test.js

@@ -1,4 +1,4 @@
-import moduleSchema, { CONDITION_MESSAGE, LOADERS_QUERY_MESSAGE } from './index'
+import schemaFn, { CONDITION_MESSAGE, LOADERS_QUERY_MESSAGE } from './index'
 import { allValid, allInvalid } from '../../../test/utils'
 
 const validModuleConfigs = [
@@ -37,6 +37,20 @@ const validModuleConfigs = [
       ],
     },
   },
+  {
+    // should allow both `include` and `exclude with the rule 'loader-enforce-include-or-exclude'
+    input: {
+      loaders: [{ test: /foo/, loader: 'foo', include: 'foo', exclude: 'bar' }],
+    },
+    schema: schemaFn({ rules: { 'loader-enforce-include-or-exclude': true } }),
+  },
+  {
+    // should be fine with `include` with rule 'loader-enforce-include-or-exclude'
+    input: {
+      loaders: [{ test: /foo/, loader: 'foo', include: 'foo' }],
+    },
+    schema: schemaFn({ rules: { 'loader-prefer-include': true } }),
+  },
 ]
 
 const invalidModuleConfigs = [
@@ -110,8 +124,41 @@ const invalidModuleConfigs = [
     },
     error: { message: '"query" must be an object' },
   },
+  {
+    // doesn't include `include`
+    input: {
+      loaders: [{ test: /foo/, loader: 'foo' }],
+    },
+    schema: schemaFn({ rules: { 'loader-prefer-include': true } }),
+  },
+  {
+    // includes `exclude` and `include`, should only use `include`
+    input: {
+      loaders: [{ test: /foo/, loader: 'foo', include: 'foo', exclude: 'bar' }],
+    },
+    schema: schemaFn({ rules: { 'loader-prefer-include': true } }),
+  },
+  {
+    // includes `exclude`, should prefer `include`
+    input: {
+      loaders: [{ test: /foo/, loader: 'foo', exclude: 'bar' }],
+    },
+    schema: schemaFn({ rules: { 'loader-prefer-include': true } }),
+  },
+  {
+    // should use either `include` or `exclude
+    input: {
+      loaders: [{ test: /foo/, loader: 'foo' }],
+    },
+    schema: schemaFn({ rules: { 'loader-enforce-include-or-exclude': true } }),
+  },
 ]
 
+const moduleSchema = schemaFn({
+  rules: {
+  },
+})
+
 describe('module', () => {
   allValid(validModuleConfigs, moduleSchema)
   allInvalid(invalidModuleConfigs, moduleSchema)