Change "one-var" requirement for es5 (and related rules)

[Krinkle]

In the ES6 preset, we turned off the one-var rule because the "one big comma-separated statement" style is uncommon in modern JavaScript and needlessly obfuscates code when writing or reviewing code. The reason it is uncommon is that let and const feature a "temporary dead-zone" in JavaScript, which means the caveat that motivated the "one-var" style, is only applicable to var and not other variable types like let and const.

I'm not sure why we kept the rule even for ES5 code, but I'm guessing it's because the underlying concern (variable hoisting) is of course still valid for actual var statements. However, I think this is really a very unlikely edge case to be worth keeping the rule for. The concern is basically to avoid code like this:

function foo() {
 var x = 1;
 bar(y + 1);

 var y = 2;
}

If it weren't for style rules like this, the code would pass ESLint and the code would not fail with an (obvious) exception at run-time either, because the engine hoists var y, declared as undefined, to the start of the function. In ES6, when using let or const this is not a concern as those are not hoisted (leading to the "temporal dead zone", to ensure an error is thrown if you access them earlier).

The way we currently avoid this mistake is through the one-var rule. The intention being to bring the var statements together, so that you write code like this instead:

function foo() {
 var x = 1,
     y = 2;
 bar(y + 1);
}

... or, if you are unable to compute y that early yet, then this will force you to realize there is a bug and deal with that, or to acknowledge that it is okay to use as undefined or some other default value, by declaring explicitly as such at the start:

function foo() {
 var x = 1,
     y = 0;
 bar(y, 1);

 y = 2;
}

... Note that the one-var rule not only requires bringing the var statements together, it also currently enforces writing them as a single akwardly-indented comma-separated statement, and (to my surprise) I could not find anything at https://eslint.org/docs/rules/one-var that accmodates the style of "together, but as distinct var statements".

My guess is, the only reason it is like this historically is because it was easier to write such rule in JSHint, or that someone tried to optimise a few bytes in the output, or that simply nobody has enough recently about this rule to fix it.

Proposal

I think the one-var rule is obsoleted by the no-use-before-define rule and so I'd like us to remove it, and allow developers to start writing code like such:

function foo() {
 var x = 1;
 var y = 0;
 bar(y, 1);

 y = 2;
}

Existing code using the comma is totally fine, and may be preferred e.g. to group together variables that have no assignment:

function foo() {
 var a, b, c;
 var x = 1;
 var y = 0;
 bar(y, 1);

 y = 2;
}

It does mean there's more than one way to write a statement, but I think this small amount of consistency will help authors to choose which style is most readable for the given code at hand, and also to slowly get used to this more modern coding style (if they want to).

My main motivation for this is that ESLint overrides and TypeScript declarations look less obtuse when they are not sandwhiched within a statement. As well as documentation comments more generally.

Compare:

function foo() {
  var
    // Foo
    foo = 1,
    bar = 2;
}

function bar() {
  // Foo
  var foo = 1,
    // bar      <!-- indented, but not actually a logically nested block
    bar = 2;
}

function baz() {
  var /** @type {Object} */ foo = 1, // Help me
    // eslint-next-line-disable some-rule-here
    bar = 2;
}

To:

function foo() {
  // Foo
  var foo = 1;
  var bar = 2;
}

function bar() {
  // Foo
  var foo = 1;
  // bar
  bar = 2;
}

function baz() {
  /** @type {Object} */
  var foo = 1;
  // eslint-next-line-disable some-rule-here
  var bar = 2;
}

[edg2s]

I think if no-use-before-define prevents people from writing broken code then everything else is just stylistic. I'm used to it now, but as multiple vars/lets is the style we are moving to with ES6, then either way seems fine.

We should see if any repos have disabled no-use-before-define because of the effect on functions, and ensure it is still enabled for variables.

... to allow

I think this means turning it to "off".

[catrope]

+1 to what Ed said. I would want this change even so that I could write:

function foo( param ) {
    if ( !param ) {
        return false;
    }
    var a = param.x,
        b = param.getY();
    ...
}

instead of

function foo( param ) {
    var a, b;
    if ( !param ) {
        return false;
    }
    a = param.x;
    b = param.getY();
    ...
}

and generally move the declarations of variables that are only used in small parts of large functions to the top of the block where they're first used.

[catrope]

We would also have to disable the vars-on-top rule.

[jdforrester]

The proper venue for discussion of changing Wikimedia's JS coding standards is https://www.mediawiki.org/wiki/Manual_talk:Coding_conventions/JavaScript and I think it'd be better to mostly continue this there given the large number of people and teams that changing this would impact. (I don't have a particular view on the proposal as such.)

[Krinkle]

Done. See Thread on mediawiki.org.

[edg2s]

This code would become possible:

for ( ... ) {
   var x = f( y );
}
output( x );

but could be prevented by enabling block-scoped-var.

[MusikAnimal]

Should https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript#Declarations be reworded? It currently states "Variables … should be the first statement in any file or function"

[jdforrester]

Should https://www.mediawiki.org/wiki/Manual:Coding_conventions/JavaScript#Declarations be reworded? It currently states "Variables … should be the first statement in any file or function"

Oops, yes, good point. We started the conversation on https://www.mediawiki.org/wiki/Topic:W5dh34nv202fzq7f but didn't resolve it and update. Will do.