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

Upgrade to eslint v2 #730

Merged
merged 32 commits into from
Feb 21, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
1bb72ab
[Deps] update `eslint` to v2, `eslint-plugin-react` to v4
hshoff Feb 12, 2016
d49a1ed
[eslint-v2] no-empty-label => no-labels
hshoff Feb 12, 2016
1efb11c
[eslint-v2] space-after/before/return/throw/case => keyword-spacing
hshoff Feb 12, 2016
da4213c
[eslint-v2] fix no-labels rule
hshoff Feb 12, 2016
dff5099
[eslint-v2] set es6 env to true
hshoff Feb 12, 2016
4f32315
[eslint-v2] ecmaFeatures => parserOptions
hshoff Feb 12, 2016
3aa8c58
[eslint-v2] fix keyword-spacing style and sorting
hshoff Feb 12, 2016
75807b9
[eslint-v2] no gens, no dup props in obj literal
hshoff Feb 12, 2016
5109c84
[eslint-v2] fix no-labels rule
hshoff Feb 13, 2016
1cbc628
[eslint-v2][constructors] disallow unnecessary constructors
hshoff Feb 13, 2016
91b8365
[eslint-v2][constructors] fix bad + good examples
hshoff Feb 14, 2016
c05b2da
[eslint-v2] add new rules, disabled:
hshoff Feb 14, 2016
172dffb
[eslint-v2][whitespace] add newline-per-chained-call rule
hshoff Feb 14, 2016
a126d0b
[eslint-v2] enforce yield-start-spacing, no-unused-labels, no-extra-l…
hshoff Feb 14, 2016
fbd9c35
[eslint-v2][arrays] add array-callback-return rule
hshoff Feb 14, 2016
6a03a32
[eslint-v2][arrow functions] add no-confusing-arrow rule
hshoff Feb 14, 2016
ba10353
[eslint-v2][arrow functions] improve examples
hshoff Feb 14, 2016
3762c9a
[eslint-v2] add no-new-symbol rule
hshoff Feb 14, 2016
92df635
[eslint-v2][variables] add no-self-assign
hshoff Feb 15, 2016
e6cbcf4
[eslint-v2][whitespace] add no-whitespace-before-property rule
hshoff Feb 15, 2016
c5b4f05
[eslint-v2] add one-var-declaration-per-line
hshoff Feb 15, 2016
e1a087f
[eslint-v2] add prefer-rest-params
hshoff Feb 15, 2016
1f12a12
[eslint-v2] fix empty constructor example
hshoff Feb 15, 2016
6560937
[eslint-v2][template strings] add template-curly-spacing rule, fix #716
hshoff Feb 15, 2016
e0959d0
[eslint-v2] fix rule links in readme
hshoff Feb 15, 2016
ff0adbe
[eslint-v2][react] acceptTranspilerName => ignoreTranspilerName
hshoff Feb 20, 2016
133fc51
[eslint-v2][react] add static-methods to top of sort-comp order
hshoff Feb 20, 2016
0794853
[eslint-v2][react] set ignoreTranspilerName to false
hshoff Feb 20, 2016
822c0df
[eslint-v2][react] jsx-sort-prop-types => sort-prop-types
hshoff Feb 20, 2016
b79e951
[eslint-v2][arrays] update array-callback-return good/bad ex
hshoff Feb 21, 2016
3983d38
[eslint-v2][arrays] fix example
hshoff Feb 21, 2016
0f32b96
[changelog] update changelog for 6.0.0
hshoff Feb 20, 2016
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
105 changes: 101 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -346,6 +346,54 @@ Other Style Guides
const nodes = Array.from(foo);
```

- [4.5](#4.5) <a name='4.5'></a> Use return statements in array method callbacks. It's ok to omit the return if the function body consists of a single statement following [8.2](#8.2). eslint: [`array-callback-return`](http://eslint.org/docs/rules/array-callback-return)

```javascript
// good
[1, 2, 3].map((x) => {
const y = x + 1;
return x * y;
});

// good
[1, 2, 3].map(x => x + 1);

// bad
const flat = {};
[[0, 1], [2, 3], [4, 5]].reduce((memo, item, index) => {
const flatten = memo.concat(item);
flat[index] = memo.concat(item);
});

// good
const flat = {};
[[0, 1], [2, 3], [4, 5]].reduce((memo, item, index) => {
const flatten = memo.concat(item);
flat[index] = flatten;
return flatten;
});

// bad
inbox.filter((msg) => {
const { subject, author } = msg;
if (subject === 'Mockingbird') {
return author === 'Harper Lee';
} else {
return false;
}
});

// good
inbox.filter((msg) => {
const { subject, author } = msg;
if (subject === 'Mockingbird') {
return author === 'Harper Lee';
}

return false;
});
Copy link
Contributor

Choose a reason for hiding this comment

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

While I agree with this example, it doesn't seem to pertain to the text of the rule here.

Also, this example could be simplified to:

[1, 2, 3].filter((x) => {
  return x > 2;
});

or even:

[1, 2, 3].filter(x => x > 2);

so I'm not sure it is the best to use here.

Choose a reason for hiding this comment

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

Second one is better in my opinion

```

**[⬆ back to top](#table-of-contents)**

## Destructuring
Expand Down Expand Up @@ -447,7 +495,7 @@ Other Style Guides
```

<a name="es6-template-literals"></a>
- [6.4](#6.4) <a name='6.4'></a> When programmatically building up strings, use template strings instead of concatenation. eslint: [`prefer-template`](http://eslint.org/docs/rules/prefer-template.html) jscs: [`requireTemplateStrings`](http://jscs.info/rule/requireTemplateStrings)
- [6.4](#6.4) <a name='6.4'></a> When programmatically building up strings, use template strings instead of concatenation. eslint: [`prefer-template`](http://eslint.org/docs/rules/prefer-template.html) [`template-curly-spacing`](http://eslint.org/docs/rules/template-curly-spacing) jscs: [`requireTemplateStrings`](http://jscs.info/rule/requireTemplateStrings)

> Why? Template strings give you a readable, concise syntax with proper newlines and string interpolation features.

Expand All @@ -462,6 +510,11 @@ Other Style Guides
return ['How are you, ', name, '?'].join();
}

// bad
function sayHi(name) {
return `How are you, ${ name }?`;
}

// good
function sayHi(name) {
return `How are you, ${name}?`;
Expand Down Expand Up @@ -535,7 +588,7 @@ Other Style Guides
```

<a name="es6-rest"></a>
- [7.6](#7.6) <a name='7.6'></a> Never use `arguments`, opt to use rest syntax `...` instead.
- [7.6](#7.6) <a name='7.6'></a> Never use `arguments`, opt to use rest syntax `...` instead. [`prefer-rest-params`](http://eslint.org/docs/rules/prefer-rest-params)

> Why? `...` is explicit about which arguments you want pulled. Plus rest arguments are a real Array and not Array-like like `arguments`.

Expand Down Expand Up @@ -771,6 +824,19 @@ Other Style Guides
});
```

- [8.5](#8.5) <a name='8.5'></a> Avoid confusing arrow function syntax (`=>`) with comparison operators (`<=`, `>=`). eslint: [`no-confusing-arrow`](http://eslint.org/docs/rules/no-confusing-arrow)

```js
// bad
const itemHeight = item => item.height > 256 ? item.largeSize : item.smallSize;

// bad
const itemHeight = (item) => item.height > 256 ? item.largeSize : item.smallSize;

// good
const itemHeight = item => { return item.height > 256 ? item.largeSize : item.smallSize; }
```

**[⬆ back to top](#table-of-contents)**


Expand Down Expand Up @@ -883,6 +949,34 @@ Other Style Guides
}
```

- [9.5](#9.5) <a name='9.5'></a> Classes have a default constructor if one is not specified. An empty constructor function or one that just delegates to a parent class is unnecessary. [`no-useless-constructor`](http://eslint.org/docs/rules/no-useless-constructor)

```javascript
// bad
class Jedi {
constructor() {}

getName() {
return this.name;
}
}

// bad
class Rey extends Jedi {
constructor(...args) {
super(args);
}
}

// good
class Rey extends Jedi {
constructor(...args) {
super(args);
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the args object be spread into the super constructor? I.e. super(...args).

Copy link

Choose a reason for hiding this comment

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

It depends on what your parent takes :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Right, but in this case that would make this class pretty weird. I mean it's a made up example, so it doesn't really matter.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Fixed in 8a6f009

this.name = 'Rey';
}
}
```

**[⬆ back to top](#table-of-contents)**


Expand Down Expand Up @@ -1592,8 +1686,8 @@ Other Style Guides
})(this);↵
```

- [18.6](#18.6) <a name='18.6'></a> Use indentation when making long method chains. Use a leading dot, which
emphasizes that the line is a method call, not a new statement.
- [18.6](#18.6) <a name='18.6'></a> Use indentation when making long method chains (more than 2 method chains). Use a leading dot, which
emphasizes that the line is a method call, not a new statement. eslint: [`newline-per-chained-call`](http://eslint.org/docs/rules/newline-per-chained-call) [`no-whitespace-before-property`](http://eslint.org/docs/rules/no-whitespace-before-property)

```javascript
// bad
Expand Down Expand Up @@ -1630,6 +1724,9 @@ Other Style Guides
.append('svg:g')
.attr('transform', 'translate(' + (radius + margin) + ',' + (radius + margin) + ')')
.call(tron.led);

// good
const leds = stage.selectAll('.led').data(data);
```

- [18.7](#18.7) <a name='18.7'></a> Leave a blank line after blocks and before the next statement. jscs: [`requirePaddingNewLinesAfterBlocks`](http://jscs.info/rule/requirePaddingNewLinesAfterBlocks)
Expand Down
18 changes: 18 additions & 0 deletions packages/eslint-config-airbnb/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,21 @@
6.0.0 / 2016-02-21
==================
- [breaking] enable `array-callback-return`
- [breaking] enable `no-confusing-arrow`
- [breaking] enable `no-new-symbol`
- [breaking] enable `no-restricted-imports`
- [breaking] enable `no-useless-constructor`
- [breaking] enable `prefer-rest-params`
- [breaking] enable `template-curly-spaces`
- [breaking] enable `newline-per-chained-call`
- [breaking] enable `one-var-declaration-per-line`
- [breaking] enable `no-self-assign`
- [breaking] enable `no-whitespace-before-property`
- [breaking] [react] enable `react/jsx-space-before-closing`
- [breaking] [react] enable `static-methods` at top of `react/sort-comp`
- [breaking] [react] don't `ignoreTranspilerName` for `react/display-name`
- [peer+dev deps] update `eslint`, `eslint-plugin-react`

5.0.1 / 2016-02-13
==================
- [fix] `eslint` peerDep should not include breaking changes
Expand Down
8 changes: 4 additions & 4 deletions packages/eslint-config-airbnb/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@
"homepage": "https://github.com/airbnb/javascript",
"devDependencies": {
"babel-tape-runner": "1.2.0",
"eslint": "^1.10.3",
"eslint-plugin-react": "^3.16.1",
"eslint": "^2.2.0",
"eslint-plugin-react": "^4.0.0",
"react": "^0.14.7",
"tape": "^4.4.0",
"parallelshell": "^2.0.0"
},
"peerDependencies": {
"eslint": "^1.0.0",
"eslint-plugin-react": "^3.16.1"
"eslint": "^2.2.0",
"eslint-plugin-react": "^4.0.0"
}
}
21 changes: 17 additions & 4 deletions packages/eslint-config-airbnb/rules/best-practices.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ module.exports = {
'rules': {
// enforces getter/setter pairs in objects
'accessor-pairs': 0,
// enforces return statements in callbacks of array's methods
// http://eslint.org/docs/rules/array-callback-return
'array-callback-return': 2,
// treat var statements as if they were block scoped
'block-scoped-var': 2,
// specify the maximum cyclomatic complexity allowed in a program
Expand All @@ -20,6 +23,9 @@ module.exports = {
'eqeqeq': 2,
// make sure for-in loops have an if statement
'guard-for-in': 2,
// Blacklist certain identifiers to prevent them being used
// http://eslint.org/docs/rules/id-blacklist
'id-blacklist': 0,
// disallow the use of alert, confirm, and prompt
'no-alert': 1,
// disallow use of arguments.caller or arguments.callee
Expand All @@ -31,8 +37,9 @@ module.exports = {
'no-div-regex': 0,
// disallow else after a return in an if
'no-else-return': 2,
// disallow use of labels for anything other then loops and switches
'no-empty-label': 2,
// disallow Unnecessary Labels
// http://eslint.org/docs/rules/no-extra-label
'no-extra-label': 2,
// disallow comparisons to null without a type-checking operator
'no-eq-null': 0,
// disallow use of eval()
Expand All @@ -53,8 +60,8 @@ module.exports = {
'no-invalid-this': 0,
// disallow usage of __iterator__ property
'no-iterator': 2,
// disallow use of labeled statements
'no-labels': 2,
// disallow use of labels for anything other then loops and switches
'no-labels': [2, { 'allowLoop': false, 'allowSwitch': false }],
// disallow unnecessary nested blocks
'no-lone-blocks': 2,
// disallow creation of functions within loops
Expand Down Expand Up @@ -96,8 +103,14 @@ module.exports = {
'no-sequences': 2,
// restrict what can be thrown as an exception
'no-throw-literal': 2,
// disallow unmodified conditions of loops
// http://eslint.org/docs/rules/no-unmodified-loop-condition
'no-unmodified-loop-condition': 0,
// disallow usage of expressions in statement position
'no-unused-expressions': 2,
// disallow unused labels
// http://eslint.org/docs/rules/no-unused-labels
'no-unused-labels': 2,
// disallow unnecessary .call() and .apply()
'no-useless-call': 0,
// disallow use of void operator
Expand Down
54 changes: 34 additions & 20 deletions packages/eslint-config-airbnb/rules/es6.js
Original file line number Diff line number Diff line change
@@ -1,25 +1,15 @@
module.exports = {
'env': {
'es6': false
'es6': true
},
'ecmaFeatures': {
'arrowFunctions': true,
'blockBindings': true,
'classes': true,
'defaultParams': true,
'destructuring': true,
'forOf': true,
'generators': false,
'modules': true,
'objectLiteralComputedProperties': true,
'objectLiteralDuplicateProperties': false,
'objectLiteralShorthandMethods': true,
'objectLiteralShorthandProperties': true,
'restParams': true,
'spread': true,
'superInFunctions': true,
'templateStrings': true,
'jsx': true
'parserOptions': {
'ecmaVersion': 6,
'sourceType': 'module',
'ecmaFeatures': {
'jsx': true,
'generators': false,
'objectLiteralDuplicateProperties': false
}
},
'rules': {
// enforces no braces where they can be omitted
Expand All @@ -38,12 +28,24 @@ module.exports = {
'generator-star-spacing': 0,
// disallow modifying variables of class declarations
'no-class-assign': 0,
// disallow arrow functions where they could be confused with comparisons
// http://eslint.org/docs/rules/no-confusing-arrow
'no-confusing-arrow': 2,
// disallow modifying variables that are declared using const
'no-const-assign': 2,
// disallow symbol constructor
// http://eslint.org/docs/rules/no-new-symbol
'no-new-symbol': 2,
// disallow specific imports
// http://eslint.org/docs/rules/no-restricted-imports
'no-restricted-imports': 0,
// disallow to use this/super before super() calling in constructors.
'no-this-before-super': 0,
// require let or const instead of var
'no-var': 2,
// disallow unnecessary constructor
// http://eslint.org/docs/rules/no-useless-constructor
'no-useless-constructor': 2,
// require method and property shorthand syntax for object literals
// https://github.com/eslint/eslint/blob/master/docs/rules/object-shorthand.md
'object-shorthand': [2, 'always'],
Expand All @@ -55,10 +57,22 @@ module.exports = {
'prefer-spread': 0,
// suggest using Reflect methods where applicable
'prefer-reflect': 0,
// use rest parameters instead of arguments
// http://eslint.org/docs/rules/prefer-rest-params
'prefer-rest-params': 2,
// suggest using template literals instead of string concatenation
// http://eslint.org/docs/rules/prefer-template
'prefer-template': 2,
// disallow generator functions that do not have yield
'require-yield': 0
'require-yield': 0,
// import sorting
// http://eslint.org/docs/rules/sort-imports
'sort-imports': 0,
// enforce usage of spacing in template strings
// http://eslint.org/docs/rules/template-curly-spacing
'template-curly-spacing': 2,
// enforce spacing around the * in yield* expressions
// http://eslint.org/docs/rules/yield-star-spacing
'yield-star-spacing': [2, 'after']
}
};
10 changes: 7 additions & 3 deletions packages/eslint-config-airbnb/rules/react.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ module.exports = {
'rules': {
// Prevent missing displayName in a React component definition
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/display-name.md
'react/display-name': [0, { 'acceptTranspilerName': false }],
'react/display-name': [0, { 'ignoreTranspilerName': false }],
Copy link
Member Author

Choose a reason for hiding this comment

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

@ljharb @lencioni did I get this right?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sadly this setting doesn't appear to differentiate between ES6 classes/stateless functional components - where we do want to use the constructor's/function's name rather than having an explicit displayName - and React.createClass, where we want an explicit displayName.

Given that, and that other rules forbid React.createClass anyways, I think false is the best setting here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

This is the name of the option, so LGTM. This does happen to invert what it used to be, but since the rule is disabled, it doesn't make much difference if it is true or false. However, I am in favor of keeping this as false as a recommendation.

// Forbid certain propTypes (any, array, object)
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/forbid-prop-types.md
'react/forbid-prop-types': [0, { 'forbid': ['any', 'array', 'object'] }],
Expand Down Expand Up @@ -54,8 +54,8 @@ module.exports = {
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-pascal-case.md
'react/jsx-pascal-case': 0,
// Enforce propTypes declarations alphabetical sorting
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-sort-prop-types.md
'react/jsx-sort-prop-types': [0, {
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/sort-prop-types.md
'react/sort-prop-types': [0, {
'ignoreCase': false,
'callbacksLast': false,
}],
Expand Down Expand Up @@ -116,10 +116,14 @@ module.exports = {
// Prevent extra closing tags for components without children
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/self-closing-comp.md
'react/self-closing-comp': 2,
// Enforce spaces before the closing bracket of self-closing JSX elements
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/jsx-space-before-closing.md
'react/jsx-space-before-closing': [2, 'always'],
// Enforce component methods order
// https://github.com/yannickcr/eslint-plugin-react/blob/master/docs/rules/sort-comp.md
'react/sort-comp': [2, {
'order': [
'static-methods',
'lifecycle',
'/^on.+$/',
'/^(get|set)(?!(InitialState$|DefaultProps$|ChildContext$)).+$/',
Expand Down
Loading