| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
If braces are not required for an arrow function body, omit them. Refs: nodejs#6390 (diff)
|
Not sure about this one. I don't mind omitting the braces for simple, short expressions (I do so myself all the time) but they are quite helpful for readability if the expression is fairly complex. |
Sorry, something went wrong.
|
@jasnell wrote:
True, although you can add outer parentheses in those cases and it probably is more readable than adding braces: (foo) => (foo.bar().baz === ((bip << bap) / 4)) vs. (foo) => { return foo.bar().baz === ((bip << bap) / 4); }
|
Sorry, something went wrong.
|
Heh, I don't find the outer parens easier to read at all ;) |
Sorry, something went wrong.
Fair enough. The other approach, which you may find equally unconvincing, is that if you have a single expression that is so complicated that it needs to be put into a block explicitly for it to be readable, maybe it really shouldn't be a single expression. So: // arguably bad
(foo) => {
return foo.bar().baz === ((bip << bap) / 4);
}
vs. // arguably better
(foo) => {
const blip = foo.bar().baz;
const blap = (bip << bap) / 4;
return blip === blap;
}
Obviously, some of this is aesthetics and reasonable people will have different opinions on what makes sense. If we can reach consensus here on a single consistent style/rule for arrow functions, then great. If we can't, oh well. ¯_(ツ)_/¯ |
Sorry, something went wrong.
|
hmmm.. ;-) yeah, I'd rather not have to do that either. I'll stew on it to see if I can come up with some metric around what makes sense here. |
Sorry, something went wrong.
|
@jasnell is the only person to offer an opinion on this so far and he doesn't support it. I'd like to leave this open for another 24-48 hours to give others a chance to offer other opinions. Will close for sure if there is no one who wants to endorse it after that. As the risk of inviting everyone to an Ultimate Bikeshedding Party: /cc @nodejs/collaborators |
Sorry, something went wrong.
|
Nah, I’d be -1 on this too. Either style can be appropriate, depending on the situation, and sometimes it’s better left to humans to decide what’s more readable for other humans. |
Sorry, something went wrong.
|
In general I prefer the non-ES6 function syntax, so I would prefer to see braces (not parens) enforced as it is closer to non-ES6 function syntax. |
Sorry, something went wrong.
|
I'm a fan of braces always. |
Sorry, something went wrong.
|
-1, compare: arr.filter(function(x) {
return x > 2;
}).map(function(x) {
return x * 3;
}).reduce(function(x,y) {
return x + y;
});With arr.filter(x => x> 2).map(x => x*3).reduce((x,y) => x+y)I find the latter a lot more readable and it's gaining a lot of momentum anyway. |
Sorry, something went wrong.
|
I find the first a lot more readable. I'm +1 for always requiring parens and curly braces |
Sorry, something went wrong.
|
@benjamingr The code you describe as more readable would pass this lint rule. EDIT: Oh, I see, your -1 referred to someone else's comment/suggestion and not the PR. |
Sorry, something went wrong.
Unfortunately, always requiring braces would flag 160 problems in over 50 files in the current code base. The proposed rule here, in contrast, flags just 5 files. |
Sorry, something went wrong.
|
As likely everyone suspected, there's a diversity of opinions and reaching consensus isn't going to happen anytime soon. That's about what I expected, but didn't want to assume. I'm going to go ahead and close this. By all means, re-open if you want to champion this or if you think my assessment of the situation is wrong. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Checklist
Affected core subsystem(s)
tools test lib
Description of change
Enable linting such that if braces are not required for an arrow function body, omit them.
Refs: #6390 (diff) (where there was a nit about this and I'd rather tools tell me code style nits rather than people...)
/cc @bnoordhuis