-
-
Notifications
You must be signed in to change notification settings - Fork 564
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
Curly all rule #237
Comments
I agree with this rule, it adds consistency! 👍 |
I'm not so sure about this one. It's stricter for sure, but |
I think it can also improve readability as the reader's eyes always know where to look for the "what happens if" piece. I don't think it is just about being more strict, but improving predictability when reading other devs' code and reducing PR review time. We can change this ourselves since we aren't using standard directly, but I thought this rule could help more teams than just ours. |
I think some initial checks in a function can be more readable as one liners, as it puts emphasis on the if-statements rather than the return value const foo = (value) => {
if (value === undefined) return 'foo';
if (typeof value === 'string') return false;
if (typeof value !== 'number') throw new TypeError('Wrong input');
// ...
}; vs const foo = (value) => {
if (value === undefined) {
return 'foo';
}
if (typeof value === 'string') {
return false;
}
if (typeof value !== 'number') {
throw new TypeError('Wrong input');
}
// ...
}; But when it comes to changes like this its also a matter of how breaking it would be to the wider community. We don't want to ship a breaking change without helping projects onboard that change. |
I can't believe you put semicolons in there. Are you even using standard? 😝 Yeah, I can def see those situations might be a good place for them as long as the if statements are shorter. |
@flippidippi I'm using |
What version of this package are you using?
17.0.0
What problem do you want to solve?
Have a tighter rule around
curly
for better consistency.Currently, the rule is set to "multi-line", which can still end up with code that isn't consistent or harder to read.
For example of consistency, both of these are ok and I think it's better to have a tighter rule around this.
Another example if when the if statement is long, and the reader has to scan far to understand what will be done (usually this would happen with longer variables names)
What do you think is the correct solution to this problem?
Change rule to
curly: "all"
Are you willing to submit a pull request to implement this change?
Yes
The text was updated successfully, but these errors were encountered: