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

Add eslint rule for trailing commas #612

Closed
nylen opened this issue May 2, 2017 · 3 comments
Closed

Add eslint rule for trailing commas #612

nylen opened this issue May 2, 2017 · 3 comments
Labels
[Type] Build Tooling Issues or PRs related to build tooling [Type] Task Issues or PRs that have been broken down into an individual action to take

Comments

@nylen
Copy link
Member

nylen commented May 2, 2017

Let's add a lint rule for trailing commas:

const obj = {
    aubergine: 1,
    tomato: 2,
};

const arr = [
    'salmon',
    'shrimp',
];

There are several advantages to this. From the WordPress coding standards for PHP:

Note the comma after the last array item: this is recommended because it makes it easier to change the order of the array, and makes for cleaner diffs when new items are added.

In the past, this has been avoided in JavaScript due to lack of support in older versions of IE (this didn't work correctly until IE9). However, we don't have to care about this because we have a build process to handle it for us and because WordPress is dropping support for IE 8-10.

@nylen nylen added [Type] Build Tooling Issues or PRs related to build tooling [Type] Task Issues or PRs that have been broken down into an individual action to take labels May 2, 2017
@ntwb
Copy link
Member

ntwb commented May 3, 2017

Yes, I think we should do this, and update the coding standards whilst we're at it 👍

Scattered throughout the JavaScript docs numerous times and to paraphrase quite heavily is:

https://make.wordpress.org/core/handbook/best-practices/coding-standards/javascript
These are for consistency between the PHP and JavaScript files in the WordPress codebase.

I've created an issue to also update this for eslint-plugin-wordpress / eslint-config-wordpress

@youknowriad
Copy link
Contributor

youknowriad commented May 3, 2017

There're too many places in Javascript where we need trailing commas and we tend to always forget them even if it's our way-to-go preference.

I understand the advantages here, but my preference would go for avoiding them.

Anyway, I prefer a strict rule one way or another.

@aduth
Copy link
Member

aduth commented May 3, 2017

This is one of those rules where I personally prefer without dangling commas, but find myself largely in the minority, so wouldn't fight strongly against them.

But I hope we all realize that for the sake of consistency, it doesn't just apply to...

return [
  'nice',
  'simple',
  'keys',
];

...but also...

return {
  not: {
    so: {
      simple: {
        keys: {},
      },
    },
  },
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Build Tooling Issues or PRs related to build tooling [Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

No branches or pull requests

4 participants