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

Support Eslint 7 & add rules #15

Merged
merged 35 commits into from
Oct 8, 2020
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
abd5b3d
upgrade eslint-plugin-react-hooks
moroine May 19, 2020
e0e33d4
Fix dependency list in README
moroine May 19, 2020
d44cec5
upgrade eslint-plugin-flowtype
moroine May 19, 2020
0ada918
upgrade eslint-plugin-react
moroine May 19, 2020
90568b4
set `require-atomic-updates`: `error`
moroine May 19, 2020
2f3fa09
Add flowtype rules
moroine May 19, 2020
3d4e63b
add import rules
moroine May 19, 2020
81e002a
enable no-undefined
moroine May 19, 2020
eb56be7
add style rules
moroine May 19, 2020
5ba8acb
Add react rules
moroine May 19, 2020
de32f93
remove eslint-plugin-ramda
moroine Sep 29, 2020
cc03c02
upgrade dependencies
moroine Sep 29, 2020
1ed29e2
Set minimum NodeJs version
moroine Sep 29, 2020
108abf0
disable sort-keys
moroine Sep 29, 2020
cd4f9af
fix lint
moroine Sep 29, 2020
12689b6
Enable 'flowtype/no-existential-type'
moroine Sep 29, 2020
3f4c2c1
Enable flowtype/no-mutable-array
moroine Sep 29, 2020
d353749
enable flowtype/spread-exact-type
moroine Sep 29, 2020
3c7c575
Update gitignore
moroine Sep 29, 2020
a46b862
v1.0.0-alpha.1
moroine Sep 29, 2020
2206a9b
fix react/function-component-definition rule
moroine Sep 29, 2020
405fcae
v1.0.0-alpha.3
moroine Sep 29, 2020
a223833
disable rules without autofix
moroine Sep 30, 2020
2424c1c
disable flowtype/require-types-at-top
moroine Sep 30, 2020
fb718af
v1.0.0-alpha.4
moroine Sep 30, 2020
2427366
remove flowtype/require-compound-type-alias
moroine Oct 1, 2020
833c392
v1.0.0-alpha.5
moroine Oct 1, 2020
cd6f942
feat: consider import starting with $ to be internal
moroine Oct 6, 2020
cc49768
v1.0.0-alpha.6
moroine Oct 6, 2020
eeb2040
chore: update changelog
moroine Oct 6, 2020
ef3bf96
feat: react/jsx-indent check logicExpressions & attributes
moroine Oct 6, 2020
1e102c1
v1.0.0-alpha.7
moroine Oct 6, 2020
70bdcdd
chore: update Changelog
moroine Oct 7, 2020
00b2789
fix: internal path pattern
moroine Oct 7, 2020
8f4b52c
v1.0.0-alpha.8
moroine Oct 7, 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
37 changes: 37 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,40 @@
# Unreleased

## Changes

Copy link
Collaborator

Choose a reason for hiding this comment

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

Missing breaking changes eslint to ^7.10.0.

- **[Breaking]**: upgrade `eslint-plugin-react-hooks` to `^4.0.2`
- **[Breaking]**: upgrade `eslint-plugin-flowtype` to `^5.1.0`
- upgrade `eslint-plugin-react` to `^7.20.0`
- Fix dependency list in README

## Likely to cause new errors

- `require-atomic-updates`
- `no-undefined`
- `array-bracket-newline`
- `func-names`: is now reported as error instead of warning
- `func-style`
- `lines-around-directive`: removed & replaced by `padding-line-between-statements`
- `no-multiple-empty-lines`: `{ max: 1, maxBOF: 0, maxEOF: 0 }`
- `no-restricted-syntax`: allow `for..of` usage
- `sort-keys`
- `flowtype/require-compound-type-alias`: `always`
- `flowtype/require-readonly-react-props`
- `flowtype/require-types-at-top`
- `flowtype/type-import-style`: `identifier`
- `import/no-namespace`
- `import/order`: add `alphabetize: { order: 'asc' 'caseInsensitive': true }` & change `groups` to the following - `order: `builtin`, `external`, `internal`, `parent`, `sibling`
- `import/no-unassigned-import`
- `import/no-anonymous-default-export`: for functions & classes
- `import/exports-last`
- `import/group-exports`
- `import/no-unused-modules`
- `react/function-component-definition`
- `react/jsx-key`
- `react/no-direct-mutation-state`
- `react/state-in-constructor`
- `react/prefer-read-only-props`

# v0.3.1

## Changes
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ We export two ESLint configurations for your usage.

Our default export contains all of our ESLint rules, including ECMAScript 6+, FlowJS and React.

It requires `eslint`, `eslint-plugin-import`, `eslint-plugin-react`, `eslint-plugin-react-hooks`,
It requires `eslint`, `eslint-plugin-import`, `eslint-plugin-react`, `eslint-plugin-ramda`, `eslint-plugin-react-hooks`,
`eslint-plugin-jsx-a11y` and `eslint-plugin-eslint-comments`. If you don't need React,
see `eslint-config-nugit/base`.

Expand All @@ -39,6 +39,6 @@ see `eslint-config-nugit/base`.

Our default export contains all of our ESLint rules, including ECMAScript 6+ and FlowJS.

It requires `eslint` and `eslint-plugin-import`.
It requires `eslint`, `eslint-plugin-import`, `eslint-plugin-flowtype` and `eslint-plugin-ramda`.

- Add `"extends": "nugit/base"` to your .eslintrc
5 changes: 5 additions & 0 deletions base.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ module.exports = {
'jest': true,
'shared-node-browser': true,
},
rules: {
// Reports modules without any exports, or with unused exports
// https://github.com/benmosher/eslint-plugin-import/blob/f63dd261809de6883b13b6b5b960e6d7f42a7813/docs/rules/no-unused-modules.md
'import/no-unused-modules': 'off'
}
},
],
};
12 changes: 6 additions & 6 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,23 @@
"babel-eslint": "^10.0.3",
"eslint": "^6.8.0",
"eslint-plugin-eslint-comments": "^3.1.2",
"eslint-plugin-flowtype": "^4.5.3",
"eslint-plugin-flowtype": "^5.1.0",
"eslint-plugin-import": "^2.19.1",
"eslint-plugin-jsx-a11y": "^6.2.3",
"eslint-plugin-ramda": "^2.5.1",
"eslint-plugin-react": "^7.17.0",
"eslint-plugin-react-hooks": "^2.3.0"
"eslint-plugin-react": "^7.20.0",
"eslint-plugin-react-hooks": "^4.0.2"
},
"peerDependencies": {
"babel-eslint": "^10.0.3",
"eslint": "^6.8.0",
"eslint-plugin-eslint-comments": "^3.1.2",
"eslint-plugin-flowtype": "^4.5.3",
"eslint-plugin-flowtype": "^5.1.0",
"eslint-plugin-import": "^2.19.1",
"eslint-plugin-jsx-a11y": "^6.2.3",
"eslint-plugin-ramda": "^2.5.1",
"eslint-plugin-react": "^7.17.0",
"eslint-plugin-react-hooks": "^2.3.0"
"eslint-plugin-react": "^7.20.0",
"eslint-plugin-react-hooks": "^4.0.2"
},
"engines": {
"node": ">= 6"
Expand Down
3 changes: 1 addition & 2 deletions rules/errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,7 @@ module.exports = {

// Disallow assignments that can lead to race conditions due to usage of await or yield
// https://eslint.org/docs/rules/require-atomic-updates
// TODO: enable, semver-major
'require-atomic-updates': 'off',
'require-atomic-updates': 'error',

// disallow comparisons with the value NaN
'use-isnan': 'error',
Expand Down
23 changes: 20 additions & 3 deletions rules/flow.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,40 @@ module.exports = {
'flowtype/delimiter-dangle': ['error', 'always-multiline', 'always-multiline', 'always-multiline'],
'flowtype/generic-spacing': ['error', 'never'],
'flowtype/newline-after-flow-annotation': ['error', 'always'],
'flowtype/object-type-delimiter': ['error', 'comma'],
'flowtype/no-mixed': 'off',
'flowtype/no-dupe-keys': 'error',
// TODO: enable?
'flowtype/no-existential-type': 'off',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think can be enabled since * is deprecated. But I guess we would have to change * to any.

'flowtype/no-flow-fix-me-comments': 'off',
'flowtype/no-mixed': 'off',
// TODO: enable?
'flowtype/no-mutable-array': 'off',
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if it forces us to only use $ReadOnlyArray and totally disallow [] or Array though. Because in our codebase, I remember we do mutate some stuff for performance purposes.

https://github.com/gajus/eslint-plugin-flowtype/blob/master/.README/rules/no-mutable-array.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We have this

Note that initialization of a variable with an empty array is considered valid (e.g., const values: Array = [];). This behavior resembles the behavior of Flow's unsealed objects, as it is assumed that empty array is intended to be mutated.

And also we will still be able to do perf optimisations using // eslint-disable-next-line it will emphasis the alert on the mutation

'flowtype/no-primitive-constructor-types': 'error',
'flowtype/no-types-missing-file-annotation': 'error',
'flowtype/no-unused-expressions': 'error',
// TODO: disable any?
Copy link
Collaborator

Choose a reason for hiding this comment

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

flowtype/no-weak-types: I think we can keep any for now, else they might be too many changes.

'flowtype/no-weak-types': ['error', { any: false }],
'flowtype/object-type-delimiter': ['error', 'comma'],
'flowtype/require-compound-type-alias': ['error', 'always'],
'flowtype/require-exact-type': 'off',
'flowtype/require-indexer-name': 'off',
'flowtype/require-inexact-type': 'off',
'flowtype/require-parameter-type': 'off',
'flowtype/require-readonly-react-props': 'off',
'flowtype/require-readonly-react-props': 'error',
'flowtype/require-return-type': 'off',
'flowtype/require-types-at-top': 'error',
Copy link
Collaborator

Choose a reason for hiding this comment

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

This might lead to quite a number of changes, need to test it out. Some of our files have this kind of pattern, which we could either move to different files else the types would be difficult to see.

Type A
Function A
Type B
Function B

https://github.com/gajus/eslint-plugin-flowtype/blob/master/.README/rules/require-types-at-top.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, would lead to lot of changes but this has autofix I think

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah, I think so too. But some files might need to reorganise, should be a few I guess.

'flowtype/require-valid-file-annotation': 'off',
'flowtype/sort-keys': ['error', 'asc', { caseSensitive: false, natural: true }],
'flowtype/require-variable-type': 'off',
'flowtype/semi': ['error', 'always'],
'flowtype/sort-keys': ['error', 'asc', { caseSensitive: false, natural: true }],
'flowtype/space-after-type-colon': ['error', 'always'],
'flowtype/space-before-generic-bracket': ['error', 'never'],
'flowtype/space-before-type-colon': ['error', 'never'],
// TODO: enable?
'flowtype/spread-exact-type': 'off',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's because spreading non-exact type is not safe and not properly supported by flow

Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be possible to enable 🤔

'flowtype/type-id-match': 'off',
// Use identifier as declaration imports are not well supported by vscode
'flowtype/type-import-style': ['error', 'identifier'],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Prefer this to be off. As at times, there is not a need to have identifier style when importing types from a type file. Less verbose too.

https://github.com/gajus/eslint-plugin-flowtype/blob/master/.README/rules/type-import-style.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What about forcing declaration style? It might help separating type from non type 🤔

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure, we can force declaration style. Would prefer that to identifier style if you want to enforce one of them.

'flowtype/union-intersection-spacing': ['error', 'always'],
'flowtype/use-flow-type': 'warn',
},
Expand Down
34 changes: 20 additions & 14 deletions rules/imports.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,8 @@ module.exports = {
'import/no-duplicates': 'error',

// disallow namespace imports
// TODO: enable?
// https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-namespace.md
'import/no-namespace': 'off',
'import/no-namespace': 'error',

// Ensure consistent use of file extension within the import path
// https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/extensions.md
Expand All @@ -146,8 +145,18 @@ module.exports = {
// TODO: enforce a stricter convention in module import order?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need to remove this TODO?

'import/order': [
'error', {
'groups': [['builtin', 'external', 'internal']],
'groups': [
['builtin'],
['external'],
['internal'],
['parent'],
['sibling']
],
'newlines-between': 'never',
'alphabetize': {
'order': 'asc',
'caseInsensitive': true,
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

❤️ 😂 🙏 . Though I'm not sure how it will work out with aliasing 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Let's see 😄

},
],

Expand Down Expand Up @@ -195,33 +204,32 @@ module.exports = {
// Prevent unassigned imports
// https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-unassigned-import.md
// importing for side effects is perfectly acceptable, if you need side effects.
'import/no-unassigned-import': 'off',
'import/no-unassigned-import': 'error',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not sure will it affect index files that just imports and do nothing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We will have to use eslint-disable but these files are bad because using side-effects. Best is to remove them as much as we can


// Prevent importing the default as if it were named
// https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-named-default.md
'import/no-named-default': 'error',

// Reports if a module's default export is unnamed
// https://github.com/benmosher/eslint-plugin-import/blob/d9b712ac7fd1fddc391f7b234827925c160d956f/docs/rules/no-anonymous-default-export.md
'import/no-anonymous-default-export': ['off', {
allowArray: false,
'import/no-anonymous-default-export': ['error', {
allowArray: true,
allowArrowFunction: false,
allowAnonymousClass: false,
allowAnonymousFunction: false,
allowLiteral: false,
allowObject: false,
allowLiteral: true,
allowObject: true,
}],

// This rule enforces that all exports are declared at the bottom of the file.
// https://github.com/benmosher/eslint-plugin-import/blob/98acd6afd04dcb6920b81330114e146dc8532ea4/docs/rules/exports-last.md
// TODO: enable?
'import/exports-last': 'off',
'import/exports-last': 'error',

// Reports when named exports are not grouped together in a single export declaration
// or when multiple assignments to CommonJS module.exports or exports object are present
// in a single file.
// https://github.com/benmosher/eslint-plugin-import/blob/44a038c06487964394b1e15b64f3bd34e5d40cde/docs/rules/group-exports.md
'import/group-exports': 'off',
'import/group-exports': 'error',
Comment on lines +235 to +241
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it when these two rules import/exports-last and import/group-exports are enabled together, we wouldn't have that much changes in terms of lines movement in our files?

Cause I'm not sure about import/exports-last as we do have this kind of patterns which will be flagged. And I guess is okay to have exports in the middle as sometimes we only export what we need to.

const bool = true
export default bool
const str = 'foo'

https://github.com/benmosher/eslint-plugin-import/blob/98acd6afd04dcb6920b81330114e146dc8532ea4/docs/rules/exports-last.md#this-will-be-reported

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah but it's easier when navigating. If you need to add export a function then it will be grouped in last statement like

const a = ...
 function b() {}

export { a, b };


// forbid default exports. this is a terrible rule, do not use it.
// https://github.com/benmosher/eslint-plugin-import/blob/44a038c06487964394b1e15b64f3bd34e5d40cde/docs/rules/no-default-export.md
Expand Down Expand Up @@ -256,9 +264,7 @@ module.exports = {

// Reports modules without any exports, or with unused exports
// https://github.com/benmosher/eslint-plugin-import/blob/f63dd261809de6883b13b6b5b960e6d7f42a7813/docs/rules/no-unused-modules.md
// TODO: enable, semver-major
'import/no-unused-modules': ['off', {
ignoreExports: [],
'import/no-unused-modules': ['error', {
missingExports: true,
unusedExports: true,
}],
Expand Down
17 changes: 11 additions & 6 deletions rules/react.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,13 @@ module.exports = {
// https://github.com/yannickcr/eslint-plugin-react/blob/843d71a432baf0f01f598d7cf1eea75ad6896e4b/docs/rules/forbid-dom-props.md
'react/forbid-dom-props': ['off', { forbid: [] }],

// Enforce a specific function type for function components.
// https://github.com/yannickcr/eslint-plugin-react/blob/v7.20.0/docs/rules/function-component-definition.md
'react/function-component-definition': ['error', {
namedComponents: 'function-declaration',
unnamedComponents: 'function-declaration',
}],
Copy link
Collaborator

@limtingzhi limtingzhi Jun 2, 2020

Choose a reason for hiding this comment

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

I'm not sure about this. I prefer to write this way inside a function component, and this would probably prevent it 🤔

var Component = (props) => {
  return <div />;
};

Also, unnamedComponents should be function-expression instead. There is no function-declaration for it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmmms, true 🤔 But means inside class, we would also have to not use arrows then. We can try it out and see how many changes etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No it's just on functions, class arrow functions will be good


// Enforce boolean attributes notation in JSX
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-boolean-value.md
'react/jsx-boolean-value': ['error', 'never', { always: [] }],
Expand Down Expand Up @@ -82,7 +89,7 @@ module.exports = {

// Validate JSX has key prop when in array or iterator
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-key.md
'react/jsx-key': 'off',
'react/jsx-key': ['error', { checkFragmentShorthand: true }],

// Limit maximum of props on a single line in JSX
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-max-props-per-line.md
Expand Down Expand Up @@ -177,7 +184,7 @@ module.exports = {

// Prevent direct mutation of this.state
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/no-direct-mutation-state.md
'react/no-direct-mutation-state': 'off',
'react/no-direct-mutation-state': 'error',

// Prevent usage of isMounted
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/no-is-mounted.md
Expand Down Expand Up @@ -476,12 +483,10 @@ module.exports = {

// Enforce state initialization style
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/state-in-constructor.md
// TODO: set to "never" once babel-preset-airbnb supports public class fields
'react/state-in-constructor': ['off', 'never'],
'react/state-in-constructor': ['error', 'never'],

// Enforces where React component static properties should be positioned
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/static-property-placement.md
// TODO: set to "static public field" once babel-preset-airbnb supports public class fields
'react/static-property-placement': ['error', 'static public field'],

// Disallow JSX props spreading
Expand All @@ -494,7 +499,7 @@ module.exports = {

// Enforce that props are read-only
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/prefer-read-only-props.md
'react/prefer-read-only-props': 'off',
'react/prefer-read-only-props': 'error',
Copy link
Collaborator

Choose a reason for hiding this comment

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

This might lead to a lot of changes but I think is most likely simple fixes. Not sure if it would be too verbose though. 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I'll enable them one-by-one to have separated commits

},

settings: {
Expand Down
21 changes: 7 additions & 14 deletions rules/style.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@ module.exports = {
rules: {
// enforce line breaks after opening and before closing array brackets
// https://eslint.org/docs/rules/array-bracket-newline
// TODO: enable? semver-major
'array-bracket-newline': ['off', 'consistent'], // object option alternative: { multiline: true, minItems: 3 }
'array-bracket-newline': ['error', 'consistent'], // object option alternative: { multiline: true, minItems: 3 }

// enforce line breaks between array elements
// https://eslint.org/docs/rules/array-element-newline
// TODO: enable? semver-major
'array-element-newline': ['off', { multiline: true, minItems: 3 }],

// enforce spacing inside array brackets
Expand Down Expand Up @@ -90,12 +88,11 @@ module.exports = {

// require function expressions to have a name
// https://eslint.org/docs/rules/func-names
'func-names': 'warn',
'func-names': 'error',

// enforces use of function declarations or expressions
// https://eslint.org/docs/rules/func-style
// TODO: enable
'func-style': ['off', 'expression'],
'func-style': ['error', 'declaration', { allowArrowFunctions: true }],

// enforce consistent line breaks inside function parentheses
// https://eslint.org/docs/rules/function-paren-newline
Expand Down Expand Up @@ -163,7 +160,6 @@ module.exports = {

// enforce position of line comments
// https://eslint.org/docs/rules/line-comment-position
// TODO: enable?
'line-comment-position': ['off', {
position: 'above',
ignorePattern: '',
Expand All @@ -183,7 +179,8 @@ module.exports = {

// require or disallow newlines around directives
// https://eslint.org/docs/rules/lines-around-directive
'lines-around-directive': ['error', {
// Deprecated in favor of padding-line-between-statements
'lines-around-directive': ['off', {
before: 'always',
after: 'always',
}],
Expand Down Expand Up @@ -303,7 +300,7 @@ module.exports = {

// disallow multiple empty lines, only one newline at the end, and no new lines at the beginning
// https://eslint.org/docs/rules/no-multiple-empty-lines
'no-multiple-empty-lines': ['error', { max: 2, maxBOF: 1, maxEOF: 0 }],
'no-multiple-empty-lines': ['error', { max: 1, maxBOF: 0, maxEOF: 0 }],

// disallow negated conditions
// https://eslint.org/docs/rules/no-negated-condition
Expand All @@ -327,10 +324,6 @@ module.exports = {
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.',
},
{
selector: 'ForOfStatement',
message: 'iterators/generators require regenerator-runtime, which is too heavyweight for this guide to allow them. Separately, loops should be avoided in favor of array iterations.',
},
{
selector: 'LabeledStatement',
message: 'Labels are a form of GOTO; using them makes code confusing and hard to maintain and understand.',
Expand Down Expand Up @@ -454,7 +447,7 @@ module.exports = {
'semi-style': ['error', 'last'],

// requires object keys to be sorted
'sort-keys': ['off', 'asc', { caseSensitive: false, natural: true }],
'sort-keys': ['error', 'asc', { caseSensitive: false, natural: true }],

// sort variables within the same declaration block
'sort-vars': 'off',
Expand Down
3 changes: 1 addition & 2 deletions rules/variables.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,7 @@ module.exports = {

// disallow use of undefined variable
// https://eslint.org/docs/rules/no-undefined
// TODO: enable?
'no-undefined': 'off',
'no-undefined': 'error',

// disallow declaration of variables that are not used in the code
'no-unused-vars': ['error', { vars: 'all', args: 'after-used', ignoreRestSiblings: true }],
Expand Down