Plotly.js: Update eslint rules

[antoinerg]

Following @etpinard's https://github.com/plotly/plotly.js/pull/3158#discussion_r228292109, we should probably update our linter syntax to reflect the current coding style.

In particular we should probably add "one-var": { "initialized": "never", "uninitialized": "consecutive" } and fix any possible errors.

Update: the "initialized" part got done in https://github.com/plotly/plotly.js/pull/3374

[etpinard]

We should also add the camelcase rule with the {"properties": "never"} option. I've seen a few community developers defining snake-case variables in their PRs.

Done in https://github.com/plotly/plotly.js/pull/3374

[alexcjohnson]

We should also add the camelcase rule

though we do have a few legacy snake_case attributes (paper_bgcolor, error_x...)

[etpinard]

egacy snake_case attributes (paper_bgcolor, error_x...)

Right, but with {"properties": "never"} we would only be looking at variable and function names.

[etpinard]

One more from https://github.com/plotly/plotly.js/pull/3125/files#r227541345:

Maybe we could add https://eslint.org/docs/rules/no-extra-parens to our lintinting rules.

[etpinard]

from https://github.com/plotly/plotly.js/issues/3020

---

consider enabling https://eslint.org/docs/rules/no-shadow - can save us from some very confusing bugs.

As @etpinard mentions in #3017:

That would require some work though, as no-shadow fails in more than 500 times in our codebase.

[etpinard]

Potential new rule: /*eslint quote-props: ["error", "as-needed"]*/

https://eslint.org/docs/rules/quote-props

which fits the style in most (but not all!) of our src files.

[etpinard]

Potential new rule: /*eslint padded-blocks: ["error", "never"]*/

https://eslint.org/docs/rules/padded-blocks

DONE in: https://github.com/plotly/plotly.js/pull/3697

[etpinard]

Maybe update our no-multiple-empty-lines rule down to {"max": 1}

[archmoj]

We previously started to dislike else statements sitting on new lines and tried to fix them in other PRs.

To put all else and catch statements inline with the closing curly braces, I suggest we use "brace-style": [2, "1tbs", {"allowSingleLine": true}], instead of "brace-style": [0, "stroustrup", {"allowSingleLine": true}], then run npm run lint -- --fix on a separate PR (possibly now that we have only a few PRs open).
We could then fix just a few edge cases where there is a comment on top of an else statement.

DONE in: https://github.com/plotly/plotly.js/pull/3757

[etpinard]

I suggest we use "brace-style": [2, "1tbs", {"allowSingleLine": true}],

I'm a fan :ok_hand: Does anyone object?

[archmoj]

Potential rule: https://eslint.org/docs/rules/func-names

[etpinard]

Potential rule: https://eslint.org/docs/rules/max-statements-per-line

[archmoj]

Potential rule: https://eslint.org/docs/rules/max-statements-per-line

I am a fan of it.
Also we may limit the number of characters on each line using: https://eslint.org/docs/rules/max-len.

[archmoj]

Potential rule: https://eslint.org/docs/rules/object-curly-spacing
eslint object-curly-spacing: ["error", "always"]

[etpinard]

https://eslint.org/docs/rules/no-prototype-builtins is now turned on by default in eslint v6, we turned it off in https://github.com/plotly/plotly.js/pull/4075/commits/e61ee25d8cb45f5a7bb836a6544e22f8db0eb516 (as it made npm run lint fail in about ~20 places) but it might be nice turn it back on at some point

[archmoj]

Potential rule: https://eslint.org/docs/rules/global-require