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

Enforce dangling commas #2

Merged
merged 1 commit into from Jun 21, 2017
Merged

Conversation

jennasalau
Copy link
Member

I'm kind of expecting a healthy debate over this one.

But I would like to propose we enforce dangling comma's in our lint/style guide for multiline statements.

https://medium.com/@nikgraf/why-you-should-enforce-dangling-commas-for-multiline-statements-d034c98e36f8

What do you all think?

@dkeeghan
Copy link
Member

Previously I would have said no, but considering that we're now not supporting IE8 and given the benefits mentioned in the article I'm sold.

@lachlanmcdonald
Copy link

For arrays\object definitions, I have no complaints. Not a fan of this though:
http://babeljs.io/docs/plugins/syntax-trailing-function-commas/

@dkeeghan
Copy link
Member

@lachlanmcdonald agreed, functions are different

@jennasalau
Copy link
Member Author

I'm glad that at least you're all positive on 80% of it.

@lachlanmcdonald dangling comma's will feel weird until you get used to them. Hopefully at that point I can win you over on function parameters too in a later PR :).. They just so happen to be in proposal for ES7 too https://github.com/tc39/proposal-trailing-function-commas

Just to be clear its only when you split them over multiple lines. The rule is not enforced if everything is on the same line.

I'll update the PR tomorrow to remove the rule from functions for now.

@dkeeghan
Copy link
Member

If it's not on single line functions I'd say leave it in then for multiline for the sake of consistency

@keeganstreet
Copy link
Contributor

I like it for the benefit of cleaner diffs. I would go all out and enforce it on all multi-line statements, including function arguments, for consistency. I think multi-line arguments are quite rare anyway. If a function has that many arguments it probably needs to be simplified.

@jennasalau jennasalau merged commit f97f6e5 into DeloitteAU:master Jun 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants