Skip to content

[eslint config][major][guide] Always require parens around arrow function arguments #1863

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

Merged
merged 1 commit into from
Aug 9, 2019
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 27 additions & 19 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -418,7 +418,7 @@ Other Style Guides
});

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

// bad - no returned value means `acc` becomes undefined after the first iteration
[[0, 1], [2, 3], [4, 5]].reduce((acc, item, index) => {
Expand Down Expand Up @@ -955,13 +955,16 @@ Other Style Guides

```javascript
// bad
[1, 2, 3].map(number => {
[1, 2, 3].map((number) => {
const nextNumber = number + 1;
`A string containing the ${nextNumber}.`;
});

// bad
[1, 2, 3].map((number) => `A string containing the ${number}.`);

// good
[1, 2, 3].map(number => `A string containing the ${number + 1}.`);
[1, 2, 3].map((number) => `A string containing the ${number + 1}.`);

// good
[1, 2, 3].map((number) => {
Expand Down Expand Up @@ -1000,14 +1003,14 @@ Other Style Guides

```javascript
// bad
['get', 'post', 'put'].map(httpMethod => Object.prototype.hasOwnProperty.call(
['get', 'post', 'put'].map((httpMethod) => Object.prototype.hasOwnProperty.call(
httpMagicObjectWithAVeryLongName,
httpMethod,
)
);

// good
['get', 'post', 'put'].map(httpMethod => (
['get', 'post', 'put'].map((httpMethod) => (
Object.prototype.hasOwnProperty.call(
httpMagicObjectWithAVeryLongName,
httpMethod,
Expand All @@ -1016,22 +1019,27 @@ Other Style Guides
```

<a name="arrows--one-arg-parens"></a><a name="8.4"></a>
- [8.4](#arrows--one-arg-parens) If your function takes a single argument and doesn’t use braces, omit the parentheses. Otherwise, always include parentheses around arguments for clarity and consistency. Note: it is also acceptable to always use parentheses, in which case use the [“always” option](https://eslint.org/docs/rules/arrow-parens#always) for eslint. eslint: [`arrow-parens`](https://eslint.org/docs/rules/arrow-parens.html)
- [8.4](#arrows--one-arg-parens) Always include parentheses around arguments for clarity and consistency. eslint: [`arrow-parens`](https://eslint.org/docs/rules/arrow-parens.html)

> Why? Less visual clutter.
> Why? Minimizes diff churn when adding or removing arguments.
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 a much better reason!


```javascript
// bad
[1, 2, 3].map((x) => x * x);

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

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

// bad
[1, 2, 3].map(number => (
`A long string with the ${number}. It’s so long that we don’t want it to take up space on the .map line!`
));

// good
[1, 2, 3].map((number) => (
`A long string with the ${number}. It’s so long that we don’t want it to take up space on the .map line!`
));

// bad
[1, 2, 3].map(x => {
const y = x + 1;
Expand All @@ -1050,13 +1058,13 @@ Other Style Guides

```javascript
// bad
const itemHeight = item => item.height <= 256 ? item.largeSize : item.smallSize;
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 => (item.height <= 256 ? item.largeSize : item.smallSize);
const itemHeight = (item) => (item.height <= 256 ? item.largeSize : item.smallSize);

// good
const itemHeight = (item) => {
Expand All @@ -1070,16 +1078,16 @@ Other Style Guides

```javascript
// bad
foo =>
(foo) =>
bar;

foo =>
(foo) =>
(bar);

// good
foo => bar;
foo => (bar);
foo => (
(foo) => bar;
(foo) => (bar);
(foo) => (
bar
)
```
Expand Down Expand Up @@ -1448,7 +1456,7 @@ Other Style Guides
});

// best (keeping it functional)
const increasedByOne = numbers.map(num => num + 1);
const increasedByOne = numbers.map((num) => num + 1);
```

<a name="generators--nope"></a><a name="11.2"></a>
Expand Down Expand Up @@ -3028,7 +3036,7 @@ Other Style Guides
// bad - raises exception
const luke = {}
const leia = {}
[luke, leia].forEach(jedi => jedi.father = 'vader')
[luke, leia].forEach((jedi) => jedi.father = 'vader')

// bad - raises exception
const reaction = "No! That’s impossible!"
Expand Down
4 changes: 1 addition & 3 deletions packages/eslint-config-airbnb-base/rules/es6.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ module.exports = {

// require parens in arrow function arguments
// https://eslint.org/docs/rules/arrow-parens
'arrow-parens': ['error', 'as-needed', {
requireForBlockBody: true,
}],
'arrow-parens': ['error', 'always'],

// require space before/after arrow function's arrow
// https://eslint.org/docs/rules/arrow-spacing
Expand Down
2 changes: 1 addition & 1 deletion packages/eslint-config-airbnb-base/test/test-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Object.keys(files).forEach((

// scan rules for react/ and fail if any exist
const reactRuleIds = Object.keys(config.rules)
.filter(ruleId => ruleId.indexOf('react/') === 0);
.filter((ruleId) => ruleId.indexOf('react/') === 0);
t.deepEquals(reactRuleIds, [], 'there are no react/ rules');
});
});
2 changes: 1 addition & 1 deletion packages/eslint-config-airbnb/test/test-base.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Object.keys(files).forEach((name) => {

// scan rules for react/ and fail if any exist
const reactRuleIds = Object.keys(config.rules)
.filter(ruleId => ruleId.indexOf('react/') === 0);
.filter((ruleId) => ruleId.indexOf('react/') === 0);
t.deepEquals(reactRuleIds, [], 'there are no react/ rules');
});
});