Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

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

Merged
merged 1 commit into from
Oct 21, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
98 changes: 47 additions & 51 deletions .neutrinorc.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
}
}
]
]
}
}
}]
]
Expand Down
16 changes: 11 additions & 5 deletions docs/migration-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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
Expand All @@ -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
Expand Down
1 change: 0 additions & 1 deletion docs/packages/loader-merge.md

This file was deleted.

1 change: 0 additions & 1 deletion mkdocs.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
93 changes: 39 additions & 54 deletions packages/airbnb-base/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
```
Expand All @@ -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 = {
Expand All @@ -168,64 +168,26 @@ module.exports = {
use: [
['@neutrinojs/airbnb-base', {
eslint: {
rules: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's worth noting that this form still works, it's just not what we're using for the defaults / what we're recommending. Any preset that pass in the old form will still correctly override the values under baseConfig, since ESLint's CLIEngine (plus our eslintrc conversion function) merges them in such a way that the top-level options take precedence.

'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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now covered by all of the defaults above being listed under baseConfig instead

a dependency, you must use `baseConfig.extends` rather than just `extends`. This is a limitation of ESLint, not this
middleware.

### Override configuration
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section is unnecessary IMO (given most people should not be using this method of customisation), and given the pain points we see around lint configuration, I think we need to take a strict "less is more" approach to what we put in the docs for it.


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
Expand Down Expand Up @@ -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
Expand Down
49 changes: 27 additions & 22 deletions packages/airbnb-base/index.js
Original file line number Diff line number Diff line change
@@ -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));
});
};