JSHint `onevar` and `trailing` args are deprecated and dropped

[georgestephanis]

As per ​http://www.jshint.com/docs/options/#onevar

These options are deprecated and will be removed soon. DO NOT use them.

This was brought up by rase- on GitHub to Jetpack here: ​https://github.com/Automattic/jetpack/pull/432#issuecomment-42783894

Unsure of the best route forward.

[georgestephanis]

For internal links, it's currently here:

https://core.trac.wordpress.org/browser/trunk/.jshintrc#L10

and jQuery uses it here:

​https://github.com/jquery/jquery/blob/master/.jshintrc#L9

We've used it since https://core.trac.wordpress.org/ticket/25187#comment:11

[scott.gonzalez]

JSHint is removing options that are purely style related. The use of onevar doesn't affect actual code quality or prevent bugs, so it's being removed. There are a few options for dealing with this.

You can move to another tool for style checks, such as ​JSCS or ​esformatter. Both are being constantly improved and have a focus on handling the style related options being removed form JSHint. jQuery team members are currently investing in both solutions (we now have at least one maintainer on each project).

Another option is to rebuild the style options as extensions for JSHint. JSHint's API is being redesigned to make extensions/plugins easy to add, so this will likely happen by someone in the community anyway.

A final option is to just do nothing. As long as JSHint 2.x is working well for you, there's no real need to upgrade. Waiting to see if the community settles on a single solution seems like a legitimate option.

[jorbin]

I think the best solution is going to be to wait and see how this settles out. I'm more familiar with JSCS, but the potential for this to be a jshint plugin might make the most sense. JSHint is working fine for us now and we don't need to upgrade it just yet.

When we need to upgrade JShint to a new version, we can make a decision then. Until then, I don't think we need to rush to make one.

[kadamwhite]

Agreed with "wait and see." I too have used (and like) JSCS, and JSCS or equivalent would also let us specify some of the other stylistic rules (whitespace around function arguments, etc) that currently exist by convention only; however, jumping into that water may be premature right now. We'd be better served to focus on enforcing curly or eqeqeq than worrying about upgrading our toolset.

[azaozz]

Replying to scott.gonzalez:

The use of onevar doesn't affect actual code quality or prevent bugs...

That's not exactly true. Example:

var a = 1;
function test() {
  console.log(a);
  var a = 2;
}
test();

Can you guess what a would be before it is defined in local scope? :)

[scott.gonzalez]

Replying to azaozz:

Replying to scott.gonzalez:

The use of onevar doesn't affect actual code quality or prevent bugs...

That's not exactly true. Example:

That's not related to onevar since you have two different functions, each one containing a single var statement.

[azaozz]

Replying to scott.gonzalez:

OK, expanded example:

a = 1;
function test() {
  var b = 2;
  // more code
  
  if ( a === 1 ) {
    // can't do that, a is unreachable
  }

  var c = 3;
  // more code

  var a = 4;
  // more code
}

It's somewhat comparable to the "Yoda" conditionals, etc. Agree this is very rare, but not impossible.

[scott.gonzalez]

Replying to azaozz:

Replying to scott.gonzalez:

OK, expanded example:

JSHint will actually complain that a is already defined in that scenario. Anyway, if your intention is to show that this block doesn't do what you expect because you're overwriting a, using onevar won't stop that logical mistake.

If you want to prove that onevar isn't a style-related option, you should test it against JSHint and make your case to the maintainers of JSHint. Arguing it here isn't going to help the situation in any way.

[netweb]

A few things here....

Since we are now using JSHint v2.5 via grunt-contrib-jshint v0.10.0 in our package.json the onevar and trailing options in .jshintrc are pointless, support for both of these was removed in JSHint v2.5.

I've had my head in JSCS for the past couple of days and I'll create a JSCS ticket and link back here once done to investigate JSCS options further and to add support for these now removed onevar and trailing options via JSCS.

JSCS includes equivalent options for ease of migrating settings from JSHint to JSCS

• onevar - ​http://jscs.info/rules.html#requiremultiplevardecl
• trailing - ​http://jscs.info/rules.html#disallowtrailingwhitespace

[netweb]

Related: #31823 Add ESLint integration

[afercia]

just ran grunt jshint:core --file=updates.js and didn't get a double var, "onevar" option was removed in ​https://github.com/jshint/jshint/releases/tag/2.5.0 but still set on WP .jshintrc
Just as update :)

[netweb]

Replying to afercia:

just ran grunt jshint:core --file=updates.js and didn't get a double var, "onevar" option was removed in ​https://github.com/jshint/jshint/releases/tag/2.5.0 but still set on WP .jshintrc
Just as update :)

Thanks, and you are correct, JSHint doesn't actually throw a warning notice for deprecated options.
​https://github.com/jshint/jshint/blob/ee436b160ad5ce146f730b7f8ea394ff4bb864a1/src/options.js#L939

[netweb]

Replying to netweb:

JSCS includes equivalent options for ease of migrating settings from JSHint to JSCS

• onevar - ​http://jscs.info/rules.html#requiremultiplevardecl
• trailing - ​http://jscs.info/rules.html#disallowtrailingwhitespace

The equivalent ESLint rules:

• one-var - ​http://eslint.org/docs/rules/one-var
  • JSHint - This rule maps to the onevar JSHint rule, but allows let and const to be configured separately.
• no-trailing-spaces - ​http://eslint.org/docs/rules/no-trailing-spaces

Edit: Both the above ESLint rules will be included in eslint-config-wordpress v1.1.0

[netweb]

#43135 was marked as a duplicate.

[swissspidy]

This just came up again while reviewing ​https://github.com/WordPress/wordpress-develop/pull/6165 for #54370

I really want to tackle ESLint soon (#31823), but in the meantime, fixing this outdated config makes sense. It's silly to prevent things like trailing commas in new code. Also, we now have Prettier for code formatting as well.

[swissspidy]

In 57702:

Build/Test Tools: Update JSHint config to remove deprecated options.

Removes deprecated options that no longer have any effect, and updates the targeted ES version in line with WordPress’ browser support.

This change mostly allows new code to properly use trailing commas, as added by the Prettier formatter.

Future efforts should rather go towards adopting ESLint for code formatting, see #31823.

Props netweb.
Fixes #28236.