Opened 11 years ago
Closed 14 months ago
#28236 closed task (blessed) (fixed)
JSHint `onevar` and `trailing` args are deprecated and dropped
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.5 | Priority: | normal |
Severity: | normal | Version: | 3.8 |
Component: | Build/Test Tools | Keywords: | has-patch commit |
Focuses: | javascript | Cc: |
Description
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.
Attachments (2)
Change History (21)
#2
follow-up:
↓ 5
@
11 years ago
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.
#3
@
11 years ago
- Milestone changed from Awaiting Review to Future Release
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.
#4
@
11 years ago
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.
#5
in reply to:
↑ 2
;
follow-up:
↓ 6
@
11 years ago
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? :)
#6
in reply to:
↑ 5
;
follow-up:
↓ 7
@
11 years ago
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.
#7
in reply to:
↑ 6
;
follow-up:
↓ 8
@
11 years ago
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.
#8
in reply to:
↑ 7
@
11 years ago
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.
#10
follow-up:
↓ 15
@
10 years ago
- Summary changed from JSHint `onevar` arg is being deprecated and dropped to JSHint `onevar` and `trailing` args are deprecated and dropped
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
#13
follow-up:
↓ 14
@
10 years ago
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 :)
#14
in reply to:
↑ 13
@
10 years ago
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
#15
in reply to:
↑ 10
@
9 years ago
Replying to netweb:
JSCS includes equivalent options for ease of migrating settings from JSHint to JSCS
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.
- JSHint - This rule maps to the
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
#17
@
14 months ago
- Keywords commit added; dev-feedback removed
- Milestone changed from Future Release to 6.5
- Type changed from defect (bug) to task (blessed)
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.
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