Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 5.0 #49

Merged
merged 28 commits into from
Jul 15, 2020
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
651299d
Add TypeScript support.
amannn Jul 7, 2020
38ecf38
Remove peer deps.
amannn Jul 7, 2020
49853f3
Remove commas for easier copy-paste
amannn Jul 7, 2020
24988c4
Auto fix for exhaustive deps.
amannn Jul 7, 2020
cb8ff06
Enforce declaration style (fixes #27)
amannn Jul 7, 2020
d3fdf51
Update CHANGELOG.
amannn Jul 7, 2020
9241fa8
Replace `jsx-a11y/label-has-for` with `jsx-a11y/label-has-associated-…
amannn Jul 7, 2020
5796fe5
Enable func-names (fixes #29)
amannn Jul 7, 2020
d206377
More TypeScript config
amannn Jul 8, 2020
3652092
Warn about confusing browser globals.
amannn Jul 8, 2020
c643fff
Warn about confusing browser globals (fixes #30).
amannn Jul 8, 2020
fcdafaf
Sort destructured keys (fixes #31)
amannn Jul 8, 2020
5816aca
Remove peer dependencies in favour of actual dependencies.
amannn Jul 8, 2020
805619a
Remove no-lonely-if (resolves #43)
amannn Jul 8, 2020
3e0d8df
Alphabetize imports (fixes #44)
amannn Jul 9, 2020
e418780
Add array-callback-return (fixes #46)
amannn Jul 9, 2020
940630a
Allow for/of statements (fixes #48)
amannn Jul 9, 2020
4f7fbd9
Improve React sort comp (fixes #12).
amannn Jul 9, 2020
3b72259
Update lockfile.
amannn Jul 9, 2020
727f525
Add `react/button-has-type`.
amannn Jul 9, 2020
4c591a9
Use eslint patch to work with dependencies.
amannn Jul 9, 2020
4148618
Add useful rules from eslint-plugin-unicorn (fixes #13)
amannn Jul 9, 2020
b0676a1
Update changelog.
amannn Jul 9, 2020
510819d
Use strict mode for TS.
amannn Jul 9, 2020
84c17a2
Allow non-null assertions.
amannn Jul 9, 2020
ee36d4e
Improve changelog.
amannn Jul 9, 2020
c767155
Add goals section to README.
amannn Jul 14, 2020
b1fc45a
Disable one more TS rule.
amannn Jul 15, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
module.exports = Object.assign({}, require('./index.js'), {
module.exports = Object.assign({}, require('./javascript.js'), {
env: {
node: true,
jest: true
Expand Down
34 changes: 34 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# CHANGELOG

## 5.0.0

### Features

- TypeScript support.
- Remove peer dependencies in favour of actual dependencies.
- Add automatic version detection for React.
- Allow more recent versions of dependencies.
- Warn about [confusing browser globals](https://www.npmjs.com/package/confusing-browser-globals) when accessing them without `window`.
- Sort desctructured keys when using the React config (useful for destructured props to match the prop type definition).
- Add useful rules from [eslint-plugin-unicorn](https://github.com/sindresorhus/eslint-plugin-unicorn): `unicorn/explicit-length-check`, `unicorn/import-index`, `unicorn/no-abusive-eslint-disable`
- Imports are alphabetized (within individual groups: dependencies, internal, local, etc.).
- Validate that array methods like `map` and `filter` return something.
- Improve React component method sorting if you're using classes.
- Add `react/button-has-type`.

### Fixes

- Removed `no-lonely-if` since there are valid cases for this pattern.
- Keep `trailingComma: 'none'` for newer prettier versions.
- Keep auto fix behaviour of `react-hooks/exhaustive-deps` in more recent versions.
- Replace `jsx-a11y/label-has-for` with `jsx-a11y/label-has-associated-control`.
- Allow for/of statements as there are valid use cases.

### Breaking changes

- The base JavaScript config `molindo` has been renamed to `molindo/javascript`.
- The base JavaScript config is no longer included in the React config. Use `"extends": ["molindo/javascript", "molindo/react"]` to use both.
- Require a minimum version of Node.js 10.
- Use `eslint@^7.0.0`.
- A declaration style is now enforced for functions. Since there's no auto fix, it's probably the best option to override this rule in existing code bases which use a different style extensively.
- This config now installs the relevant plugins automatically. However you have to use the JavaScript version of the ESLint config (`.eslintrc.js`) and add `require('eslint-config-molindo/setupPlugins')` there. Alternatively you can still install all necessary plugins yourself.
51 changes: 41 additions & 10 deletions README.md
Original file line number Diff line number Diff line change
@@ -1,26 +1,57 @@
# eslint-config-molindo

Molindo ESLint config that implements our styleguide and helps to catch errors.
## Goals

1. Find errors that are detectable with static analysis.
2. Make reading code easier by providing consistent code style.
3. Make writing code faster by leveraging auto fix wherever possible.

## Usage

1. `yarn add eslint-config-molindo --dev`
2. `touch .eslintrc`, if there isn't already one.
3. Add `{"extends": "molindo"}` or `{"extends": "molindo/react"}` to `.eslintrc`
4. As [shared ESLint configs can't include dependencies themselves](https://github.com/eslint/eslint/issues/3458) yet, please install all peer dependencies as dev dependencies. The peers can be found here: `npm info eslint-config-molindo peerDependencies --json`.
5. Happy linting!
2. Setup your project config in `.eslintrc.js`:

```js
// This enables ESLint to use dependencies of this config
// (see https://github.com/eslint/eslint/issues/3458)
require('eslint-config-molindo/setupPlugins');

module.exports = {
extends: 'molindo/javascript',
// or
extends: 'molindo/typescript',
// or
extends: ['molindo/javascript', 'molindo/react'],
// or
extends: ['molindo/typescript', 'molindo/react']
}
```

3. If you use TypeScript, add `"extends": "eslint-config-molindo/tsconfig.json"` to your `tsconfig.json`.
4. Happy linting!

## Further configuration

- Since the linting of the app should refer to the local copy of eslint and not a potentially globally installed one, it's helpful to add an npm task for this (e.g. `"lint": "eslint src"`).
### Environment

Set the [`env` in `.eslintrc`](https://eslint.org/docs/user-guide/configuring#specifying-environments) as necessary so ESLint doesn't report missing globals.

E.g.:

- Set the [`env` in `.eslintrc`](https://eslint.org/docs/user-guide/configuring#specifying-environments) as necessary (e.g. `{"browser": true, "node": true, "es6": true, "jest": true}`) so ESLint doesn't report missing globals.
```json
{
"browser": true,
"node": true,
"es6": true,
"jest": true
}
```

## Editor integration
### Editor integration

It's strongly recommended to use an eslint integration for your editor of choice (e. g. [linter-eslint](https://atom.io/packages/linter-eslint) for [Atom](https://atom.io/)) so you see warnings and errors while writing code. Also the setting to auto fix errors on save should be turned on, so purely stylistic errors such as the ones reported by `prettier` are fixed automatically.
It's strongly recommended to use an eslint integration for your editor of choice (e. g. [`dbaeumer.vscode-eslint`](https://marketplace.visualstudio.com/items?itemName=dbaeumer.vscode-eslint) for [VSCode](https://code.visualstudio.com/) so you see warnings and errors while writing code. Also the setting to auto fix errors on save should be turned on, so purely stylistic errors such as the ones reported by `prettier` are fixed automatically.

If your linter plugin checks your code as you type (before you save) it can be helpful to silence stylistic errors to reduce noise and let the formatting happen on save. E.g. `linter-eslint` for Atom has the setting "Ignore fixable rules while typing".
If your linter plugin checks your code as you type (before you save) it can be helpful to silence stylistic errors to reduce noise and let the formatting happen on save.

## Versioning

Expand Down
5 changes: 0 additions & 5 deletions __tests__/index-test.js

This file was deleted.

5 changes: 5 additions & 0 deletions __tests__/javascript-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const javascript = require('../javascript');

it('exports a valid config', () => {
expect(javascript).toBeTruthy();
});
5 changes: 5 additions & 0 deletions __tests__/typescript-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const typescript = require('../typescript');

it('exports a valid config', () => {
expect(typescript).toBeTruthy();
});
30 changes: 20 additions & 10 deletions index.js → javascript.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
const confusingBrowserGlobals = require('confusing-browser-globals');
const config = require('./config');

const ERROR = 'error';
Expand All @@ -6,7 +7,7 @@ const OFF = 'off';
module.exports = {
parser: 'babel-eslint',

plugins: ['import', 'prettier'],
plugins: ['import', 'prettier', 'unicorn'],

settings: {
'import/resolver': 'node'
Expand All @@ -19,17 +20,24 @@ module.exports = {
],

rules: {
'array-callback-return': ERROR,
'arrow-body-style': [ERROR, 'as-needed'],
curly: [ERROR, 'multi-line'],
'dot-notation': ERROR,
eqeqeq: [ERROR, 'always', {null: 'ignore'}],
'func-names': [ERROR, 'as-needed'],
// See discussion in https://github.com/molindo/eslint-config-molindo/issues/27
'func-style': [ERROR, 'declaration', {allowArrowFunctions: false}],
'import/newline-after-import': ERROR,
'import/no-unresolved': [ERROR, {commonjs: true}],
'import/no-named-as-default': OFF,
'import/no-extraneous-dependencies': [
ERROR,
{
devDependencies: config.testFiles.concat('webpack.config.js')
devDependencies: config.testFiles.concat(
'webpack.config.js',
'.eslintrc.js'
)
}
],
'import/order': [
Expand All @@ -43,22 +51,18 @@ module.exports = {
'sibling',
'index'
],
'newlines-between': 'never'
'newlines-between': 'never',
alphabetize: {order: 'asc'}
Copy link
Member Author

Choose a reason for hiding this comment

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

The only downside here is currently this IMO: import-js/eslint-plugin-import#1378 (comment). However I think it's still better that this is taken care of automatically, than us trying to keep the imports in order manually.

}
],
'no-console': [ERROR, {allow: ['warn', 'error']}],
'no-lonely-if': ERROR,
'no-restricted-globals': [ERROR].concat(confusingBrowserGlobals),
'no-restricted-syntax': [
ERROR,
{
selector: 'ForInStatement',
message:
'for … in loops iterate over the entire prototype chain, which is virtually never what you want. Use Object.{keys,values,entries} and iterate over the resulting array. Iteration based on arrays usually shows the intent of the loop clearer and works better with chaining. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array#Iteration_methods'
},
{
selector: 'ForOfStatement',
message:
'for … of loops should be avoided in favor of array iteration methods. Iteration based on arrays usually shows the intent of the loop clearer and works better with chaining. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array#Iteration_methods'
}
],
'no-shadow': ERROR,
Expand All @@ -70,7 +74,13 @@ module.exports = {
'object-shorthand': ERROR,
'prefer-arrow-callback': ERROR,
'prefer-const': ERROR,
'prettier/prettier': [ERROR, {singleQuote: true, bracketSpacing: false}],
'prettier/prettier': [
ERROR,
{singleQuote: true, bracketSpacing: false, trailingComma: 'none'}
],
'unicorn/explicit-length-check': ERROR,
'unicorn/import-index': ERROR,
'unicorn/no-abusive-eslint-disable': ERROR,
'valid-jsdoc': [
ERROR,
{
Expand Down
44 changes: 27 additions & 17 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
{
"name": "eslint-config-molindo",
"version": "4.0.2",
"version": "5.0.0",
"description": "Molindo ESLint config that implements our styleguide and helps to catch errors.",
"main": "index.js",
"repository": "https://github.com/molindo/eslint-config-molindo",
"author": "Jan Amann <jam@molindo.at>",
"license": "MIT",
Expand All @@ -16,27 +15,38 @@
"prepublish": "npm run lint && npm run test"
},
"files": [
"index.js",
"setupPlugins.js",
"javascript.js",
"typescript.js",
"config.js",
"react.js"
"react.js",
"tsconfig.json"
],
"devDependencies": {
"babel-eslint": "10.0.1",
"eslint": "5.12.0",
"eslint-plugin-import": "2.14.0",
"eslint-plugin-prettier": "3.0.1",
"jest": "23.6.0",
"prettier": "1.15.3"
"eslint": "7.4.0",
"jest": "26.1.0"
},
"peerDependencies": {
"eslint": "^7.0.0"
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm really glad that all the peer dependencies are now gone.

},
"dependencies": {
"@rushstack/eslint-patch": "1.0.2",
"@typescript-eslint/eslint-plugin": "^3.6.0",
"@typescript-eslint/parser": "^3.0.0",
"babel-eslint": "^10.0.0",
"eslint": "^5.12.0 || ^6.0.0",
"confusing-browser-globals": "^1.0.0",
"eslint-plugin-css-modules": "^2.11.0",
"eslint-plugin-import": "^2.14.0",
"eslint-plugin-jsx-a11y": "^6.1.2",
"eslint-plugin-prettier": "^3.0.1",
"eslint-plugin-react": "^7.12.3",
"eslint-plugin-react-hooks": "^1.6.1",
"prettier": "^1.15.3"
"eslint-plugin-import": "^2.22.0",
"eslint-plugin-jsx-a11y": "^6.3.1",
"eslint-plugin-prettier": "^3.1.4",
"eslint-plugin-react": "^7.20.3",
"eslint-plugin-react-hooks": "^4.0.6",
"eslint-plugin-sort-destructure-keys": "^1.3.5",
"eslint-plugin-unicorn": "^20.1.0",
"prettier": "^2.0.0",
"typescript": "^3.0.0"
},
"engines": {
"node": ">=10"
}
}
66 changes: 38 additions & 28 deletions react.js
Original file line number Diff line number Diff line change
@@ -1,42 +1,46 @@
const config = require('./config');
const base = require('./index');

const ERROR = 'error';
const OFF = 'off';

module.exports = Object.assign({}, base, {
plugins: base.plugins.concat(
module.exports = {
plugins: [
'css-modules',
'jsx-a11y',
'react',
'react-hooks'
),
'react-hooks',
'sort-destructure-keys'
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 was the last missing piece so that everything related to React props is sorted (JSX, prop types, prop destructuring). sort-keys from ESLint is a bit too aggressive IMO.

],

settings: Object.assign({}, base.settings, {
settings: {
react: {
version: 'detect'
},
'import/resolver': {
node: {
paths: ['node_modules', 'src']
}
}
}),
},

extends: (base.extends || []).concat(
extends: [
'plugin:css-modules/recommended',
'plugin:jsx-a11y/recommended',
'plugin:react/recommended'
),
],

rules: Object.assign({}, base.rules, {
'jsx-a11y/label-has-for': [
ERROR,
{
required: {every: ['nesting']},
allowChildren: true
}
],
rules: {
'jsx-a11y/label-has-associated-control': ERROR,
'react/button-has-type': ERROR,
'react/default-props-match-prop-types': ERROR,
'react-hooks/rules-of-hooks': ERROR,
'react-hooks/exhaustive-deps': ERROR,
'react-hooks/exhaustive-deps': [
ERROR,
// Since we throw on missing deps, it's not a question of whether or not
// the deps are added, but rather if you have to do it manually or
// automatically. Therefore go for the automatic fix.
{enableDangerousAutofixThisMayCauseInfiniteLoops: true}
],
'react/jsx-boolean-value': [ERROR, 'never', {always: []}],
'react/jsx-curly-brace-presence': [
ERROR,
Expand Down Expand Up @@ -69,7 +73,10 @@ module.exports = Object.assign({}, base, {
ERROR,
{
order: [
'type-annotations',
'static-variables',
'static-methods',
'instance-variables',
'lifecycle',
'/^on.+$/',
'getters',
Expand All @@ -81,15 +88,18 @@ module.exports = Object.assign({}, base, {
]
}
],
'react/sort-prop-types': ERROR
}),
'react/sort-prop-types': ERROR,
'sort-destructure-keys/sort-destructure-keys': ERROR
},

overrides: (base.overrides || []).concat({
files: config.testFiles,
rules: {
'css-modules/no-unused-class': OFF,
'react/prop-types': OFF,
'react/display-name': OFF
overrides: [
{
files: config.testFiles,
rules: {
'css-modules/no-unused-class': OFF,
'react/prop-types': OFF,
'react/display-name': OFF
}
}
})
});
]
};
3 changes: 3 additions & 0 deletions setupPlugins.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// This enables ESLint to use dependencies of this config
// (see https://github.com/eslint/eslint/issues/3458)
require('@rushstack/eslint-patch/modern-module-resolution');
Copy link
Member Author

Choose a reason for hiding this comment

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

ESLint wants to handle this itself one day, but in the meantime there are some good guys from Microsoft providing a patch.

6 changes: 6 additions & 0 deletions tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"compilerOptions": {
"strict": true,
"forceConsistentCasingInFileNames": true
}
}
Loading