eslint: Refactor eslint-loader and eslintrc config generation (#1182)

* Adds option validation to `@neutrinojs/eslint`, so that it now throws when passed options that are not supported by eslint-loader. * The eslintrc conversion now correctly handles all supported options under `baseConfig` rather than silently ignoring some of them. * The eslintrc conversion no longer looks for settings in locations that are invalid for eslint-loader (such as top-level `settings`, `extends`, `root` or `overrides` options). * The eslintrc conversion now no longer errors if passed options that were previously missing from the `omit()` list, such as `ignore` and `reportUnusedDisableDirectives`. * The custom lint `merge()` function has been replaced with ESLint's own merging function, which correctly handles several cases that were broken before. * All non-loader related configuration has now been moved under the `baseConfig` key, since the options under it more closely relate to the options that users will have previously seen in `.eslintrc` files, thereby reducing user-confusion and making it possible to copy and paste the contents of the `baseConfig` section to a static `.eslintrc.*`. * Support has been added for projects that want to use their own non-generated `.eslintrc.*` file. They now only need to set `useEslintrc` to `true`, which will make the linting presets only set the loader-related options, preventing conflicts with the options set in their static `.eslintrc.*` file. * The `@neutrinojs/loader-merge` middleware has been removed, since its functionality was not sufficient for merging lint configuration correctly, and was not used for any other purpose. Fixes #1181. Fixes #382.
neutrinojs · Oct 21, 2018 · a13feee · a13feee
1 parent ff30cd6
commit a13feee
Show file tree

Hide file tree

Showing 47 changed files with 1,599 additions and 682 deletions.
diff --git a/.neutrinorc.js b/.neutrinorc.js
@@ -8,61 +8,57 @@ module.exports = {
       // Excludes are managed via `.eslintignore`.
       eslint: {
         baseConfig: {
-          extends: [
-            'prettier'
-          ]
-        },
-        envs: [
-          'browser',
-          'jest',
-          'mocha',
-          'node'
-        ],
-        plugins: [
-          'prettier'
-        ],
-        rules: {
-          // Disallow trailing commas on arrays, objects, functions, et al
-          'comma-dangle': ['error', 'never'],
-          // Allow using console since most of the code in this repo isn't run in a browser.
-          'no-console': 'off',
-          // Allowing shadowing variable that share the same context as the outer scope
-          'no-shadow': 'off'
-        },
-        overrides: [
-          {
-            files: ['packages/create-project/commands/init/templates/**'],
-            rules: {
-              // The dependencies in create-project's templates are installed by
-              // by create-project and so are expected to be missing from package.json.
-              'import/no-extraneous-dependencies': 'off'
-            }
+          extends: ['prettier'],
+          env: {
+            browser: true,
+            jest: true,
+            mocha: true,
+            node: true
+          },
+          plugins: ['prettier'],
+          rules: {
+            // Disallow trailing commas on arrays, objects, functions, et al
+            'comma-dangle': ['error', 'never'],
+            // Allow using console since most of the code in this repo isn't run in a browser.
+            'no-console': 'off',
+            // Allowing shadowing variable that share the same context as the outer scope
+            'no-shadow': 'off'
           },
-          {
-            files: ['packages/create-project/commands/init/templates/preact/**'],
-            settings: {
-              react: {
-                pragma: 'h'
+          overrides: [
+            {
+              files: ['packages/create-project/commands/init/templates/**'],
+              rules: {
+                // The dependencies in create-project's templates are installed by
+                // by create-project and so are expected to be missing from package.json.
+                'import/no-extraneous-dependencies': 'off'
               }
             },
-            rules: {
-              // With Preact the use of `class` is recommended over `className`,
-              // so we have to add `class` to the ignore list, to prevent:
-              // `Unknown property 'class' found, use 'className' instead`
-              'react/no-unknown-property': ['error', { ignore: ['class'] }]
-            }
-          },
-          {
-            files: ['packages/*/test/*'],
-            rules: {
-              // The tests need to do non-global require() to test the presets.
-              'global-require': 'off',
-              // This rule doesn't handle devDependencies being defined
-              // in the monorepo root package.json.
-              'import/no-extraneous-dependencies': 'off'
+            {
+              files: ['packages/create-project/commands/init/templates/preact/**'],
+              settings: {
+                react: {
+                  pragma: 'h'
+                }
+              },
+              rules: {
+                // With Preact the use of `class` is recommended over `className`,
+                // so we have to add `class` to the ignore list, to prevent:
+                // `Unknown property 'class' found, use 'className' instead`
+                'react/no-unknown-property': ['error', { ignore: ['class'] }]
+              }
+            },
+            {
+              files: ['packages/*/test/*'],
+              rules: {
+                // The tests need to do non-global require() to test the presets.
+                'global-require': 'off',
+                // This rule doesn't handle devDependencies being defined
+                // in the monorepo root package.json.
+                'import/no-extraneous-dependencies': 'off'
+              }
             }
-          }
-        ]
+          ]
+        }
       }
     }]
   ]

diff --git a/docs/migration-guide.md b/docs/migration-guide.md
@@ -226,8 +226,9 @@ must be updated for the new plugin.
 [#826](https://github.com/neutrinojs/neutrino/pull/858). If you wish to force specifying additional Babel
 configuration via a configuration file, you can set it to `true` via `babel: { babelrc: true }`.
 - **BREAKING CHANGE** `@neutrinojs/eslint` and dependent middleware now throw if they are used by Neutrino after
-a compile preset [#839](https://github.com/neutrinojs/neutrino/pull/939). This is to prevent using an unknowingly
-broken configuration.
+a compile preset [#939](https://github.com/neutrinojs/neutrino/pull/939), or if used with unrecognised
+[eslint-loader options](https://github.com/webpack-contrib/eslint-loader#options)
+[#1182](https://github.com/neutrinojs/neutrino/pull/1182). This is to prevent using an unknowingly broken configuration.
 - **BREAKING CHANGE** Most Neutrino middleware now throws if used twice with the same rule IDs. This is to
 prevent mis-configurations seen in the wild (such as using both `@neutrinojs/web` and one of the lower-level
 middleware it uses at the same time) [#1162](https://github.com/neutrinojs/neutrino/pull/1162).
@@ -253,9 +254,10 @@ in the HTML template [#943](https://github.com/neutrinojs/neutrino/pull/943).
 [html-webpack-template](https://github.com/jaketrent/html-webpack-template), which supports a smaller set of options
 but better html-webpack-plugin integration. You can override with your own template for more in-depth customization of
 this template and its content [#1049](https://github.com/neutrinojs/neutrino/pull/1049).
-- **BREAKING CHANGE** The `@neutrinojs/hot` [#981](https://github.com/neutrinojs/neutrino/pull/981) and `@neutrinojs/env`
-[#983](https://github.com/neutrinojs/neutrino/pull/983) middleware have been removed and their relevant
-functionality is now configured directly by its previously-consuming presets.
+- **BREAKING CHANGE** The `@neutrinojs/hot` [#981](https://github.com/neutrinojs/neutrino/pull/981),
+`@neutrinojs/env` [#983](https://github.com/neutrinojs/neutrino/pull/983), and
+`@neutrinojs/loader-merge` [#1182](https://github.com/neutrinojs/neutrino/pull/1182) middleware have been
+removed and their relevant functionality is now configured directly by its previously-consuming presets.
 - **BREAKING CHANGE** `@neutrinojs/node` now compiles to the supported version of Node.js being used to run the build
 [#991](https://github.com/neutrinojs/neutrino/pull/991). Previously this was always defaulted to a particular Node.js v6
 version. For example, if you use Node.js v8 to run webpack, Neutrino will tell Babel to compile your project to support
@@ -269,6 +271,10 @@ overwrite them [#1022](https://github.com/neutrinojs/neutrino/pull/1022).
 - **BREAKING CHANGE** `@neutrinojs/eslint` and its dependent middleware now require ESLint v5, which may mean breaking
 changes to rule configuration and any `parserOptions` modifications [#1025](https://github.com/neutrinojs/neutrino/pull/1025).
 See the [ESLint migration docs](https://eslint.org/docs/user-guide/migrating-to-5.0.0) for more information.
+- **BREAKING CHANGE** `@neutrinojs/eslint` and its dependent middleware now only set loader-related
+default options if `useEslintrc` is `true`, to prevent linting configuration defaults from conflicting
+with values set in `.eslintrc.*` [#1182](https://github.com/neutrinojs/neutrino/pull/1182).
+See the new documentation explaining how to [use your own `.eslintrc.*`](./packages/eslint.md#using-your-own-eslintrc).
 - **BREAKING CHANGE** The `options.mains` Neutrino option can now also accept an object per entry point for setting
 individual options for html-webpack-plugin [#1029](https://github.com/neutrinojs/neutrino/pull/1029). The previous API
 for setting these properties as strings is still supported. This also introduces a breaking API change as accessing an

diff --git a/docs/packages/loader-merge.md b/docs/packages/loader-merge.md
diff --git a/mkdocs.yml b/mkdocs.yml
@@ -58,7 +58,6 @@ nav:
     - html-loader: './packages/html-loader.md'
     - html-template: './packages/html-template.md'
     - image-loader: './packages/image-loader.md'
-    - loader-merge: './packages/loader-merge.md'
     - pwa: './packages/pwa.md'
     - start-server: './packages/start-server.md'
     - style-loader: './packages/style-loader.md'

diff --git a/packages/airbnb-base/README.md b/packages/airbnb-base/README.md
@@ -132,7 +132,6 @@ error: Missing semicolon (semi) at src/index.js:35:51:
   37 |
   38 |
 
-
 1 error found.
 1 error potentially fixable with the `--fix` option.
 ```
@@ -153,12 +152,13 @@ module.exports = {
 
 ## Middleware options
 
-This preset uses the same middleware options as [@neutrinojs/eslint](https://neutrinojs.org/packages/eslint/).
+This preset uses the same middleware options as [@neutrinojs/eslint](https://neutrinojs.org/packages/eslint/#usage).
 If you wish to customize what is included, excluded, or any ESLint options, you can provide an options object with the
 middleware and this will be merged with our internal defaults for this preset. Use an array pair instead of a string
 to supply these options.
 
-_Example: Turn off semicolons from being required as defined by the Airbnb rules._
+_Example: Extend from a custom configuration (it will be applied after Airbnb)
+and turn off semicolons from being required._
 
 ```js
 module.exports = {
@@ -168,64 +168,26 @@ module.exports = {
   use: [
     ['@neutrinojs/airbnb-base', {
       eslint: {
-        rules: {
-          'babel/semi': 'off'
+        // For supported options, see:
+        // https://github.com/webpack-contrib/eslint-loader#options
+        // https://eslint.org/docs/developer-guide/nodejs-api#cliengine
+        // The options under `baseConfig` correspond to those
+        // that can be used in an `.eslintrc.*` file.
+        baseConfig: {
+          extends: [
+            'my-custom-config'
+          ],
+          rules: {
+            'babel/semi': 'off'
+          }
         }
       }
     }]
   ]
 };
 ```
 
-## Customizing
-
-To override the build configuration, start with the documentation on [customization](https://neutrinojs.org/customization/).
-`@neutrinojs/airbnb-base` creates some conventions to make overriding the configuration easier once you are ready to
-make changes.
-
-### Rules
-
-The following is a list of rules and their identifiers which can be overridden:
-
-| Name | Description | NODE_ENV |
-| --- | --- | --- |
-| `lint` | Lints JS and JSX files from the `src` directory using ESLint. Contains a single loader named `eslint`. This is inherited from `@neutrinojs/eslint`. | all |
-
-### Information
-
-If you want your preset or middleware to also extend from another **ESLint configuration or preset** that you have made
-a dependency, you must use `baseConfig.extends` rather than just `extends`. This is a limitation of ESLint, not this
-middleware.
-
-### Override configuration
-
-By following the [customization guide](https://neutrinojs.org/customization/) and knowing the rule and loader IDs above,
-you can also override or augment the build by providing a function to your `.neutrinorc.js` use array. You can also
-make this change from the Neutrino API when using the `use` method.
-
-_Example: Turn off semicolons from being required as defined by the Airbnb rules from `.neutrinorc.js` using a function and the API:_
-
-```js
-module.exports = {
-  options: {
-    root: __dirname
-  },
-  use: [
-    '@neutrinojs/airbnb-base',
-    (neutrino) => neutrino.config.module
-      .rule('lint')
-      .use('eslint')
-      .tap(options => ({
-        ...options,
-        rules: {
-          'babel/semi': 'off'
-        }
-      }))
-  ]
-};
-```
-
-## eslintrc Config
+## Exposing generated lint configuration via `.eslintrc.js`
 
 `@neutrinojs/eslint`, from which this preset inherits, provides an `.eslintrc()` output handler for
 generating the ESLint configuration in a format suitable for use in an `.eslintrc.js` file. This
@@ -259,6 +221,29 @@ specify included and excluded files from the `.eslintrc.js` configuration. Inste
 [.eslintignore](https://eslint.org/docs/user-guide/configuring#ignoring-files-and-directories) file that controls
 which files should be excluded from linting.
 
+## Using your own `.eslintrc.*`
+
+If instead you would prefer to use your own non-generated `.eslintrc.*` file, set `useEslintrc` to `true`.
+This will cause `@neutrinojs/airbnb-base` to only set the loader-specific configuration defaults, and leave
+all other linting configuration to be managed by the standalone `.eslintrc.*` file.
+
+See the `@neutrinojs/eslint` [documentation](https://neutrinojs.org/packages/eslint/#using-your-own-eslintrc)
+for more details.
+
+## Customizing
+
+To override the lint configuration, start with the documentation on [customization](https://neutrinojs.org/customization/).
+`@neutrinojs/airbnb-base` creates some conventions to make overriding the configuration easier once you are ready to
+make changes.
+
+### Rules
+
+The following is a list of rules and their identifiers which can be overridden:
+
+| Name | Description | NODE_ENV |
+| --- | --- | --- |
+| `lint` | By default, lints JS and JSX files from the `src` and `test` directories using ESLint. Contains a single loader named `eslint`. This is inherited from `@neutrinojs/eslint`. | all |
+
 ## Contributing
 
 This preset is part of the [neutrino](https://github.com/neutrinojs/neutrino) repository, a monorepo

diff --git a/packages/airbnb-base/index.js b/packages/airbnb-base/index.js
@@ -1,29 +1,34 @@
 const lint = require('@neutrinojs/eslint');
+const { merge: eslintMerge } = require('eslint/lib/config/config-ops');
 const { rules: airbnbBaseStyle } = require('eslint-config-airbnb-base/rules/style');
 const { rules: airbnbBaseBestPractices } = require('eslint-config-airbnb-base/rules/best-practices');
 
-module.exports = (neutrino, opts = {}) => {
-  neutrino.use(lint, lint.merge({
+module.exports = (neutrino, { eslint = {}, ...opts } = {}) => {
+  neutrino.use(lint, {
+    ...opts,
     eslint: {
-      baseConfig: {
-        extends: [require.resolve('eslint-config-airbnb-base')]
-      },
-      rules: {
-        // Disable rules for which there are eslint-plugin-babel replacements:
-        // https://github.com/babel/eslint-plugin-babel#rules
-        'new-cap': 'off',
-        'no-invalid-this': 'off',
-        'object-curly-spacing': 'off',
-        'semi': 'off',
-        'no-unused-expressions': 'off',
-        // Ensure the replacement rules use the options set by airbnb-base rather than ESLint defaults.
-        'babel/new-cap': airbnbBaseStyle['new-cap'],
-        'babel/no-invalid-this': airbnbBaseBestPractices['no-invalid-this'],
-        'babel/object-curly-spacing': airbnbBaseStyle['object-curly-spacing'],
-        'babel/semi': airbnbBaseStyle.semi,
-        'babel/no-unused-expressions': airbnbBaseBestPractices['no-unused-expressions']
-      }
+      ...eslint,
+      baseConfig: eslintMerge(
+        {
+          extends: [require.resolve('eslint-config-airbnb-base')],
+          rules: {
+            // Disable rules for which there are eslint-plugin-babel replacements:
+            // https://github.com/babel/eslint-plugin-babel#rules
+            'new-cap': 'off',
+            'no-invalid-this': 'off',
+            'object-curly-spacing': 'off',
+            'semi': 'off',
+            'no-unused-expressions': 'off',
+            // Ensure the replacement rules use the options set by airbnb-base rather than ESLint defaults.
+            'babel/new-cap': airbnbBaseStyle['new-cap'],
+            'babel/no-invalid-this': airbnbBaseBestPractices['no-invalid-this'],
+            'babel/object-curly-spacing': airbnbBaseStyle['object-curly-spacing'],
+            'babel/semi': airbnbBaseStyle.semi,
+            'babel/no-unused-expressions': airbnbBaseBestPractices['no-unused-expressions']
+          }
+        },
+        eslint.baseConfig || {}
+      )
     }
-  },
-  opts));
+  });
 };