Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign uporder of variable declaration is not taken into account. #636
Comments
This comment has been minimized.
This comment has been minimized.
|
We currently do not enable the |
feross
added
blocked
enhancement
labels
Sep 26, 2016
feross
modified the milestone:
standard v9
Sep 26, 2016
feross
closed this
in
webtorrent/webtorrent-desktop@be08925
Sep 27, 2016
This comment has been minimized.
This comment has been minimized.
|
This wasn't meant to be closed, right? |
LinusU
reopened this
Sep 27, 2016
This comment has been minimized.
This comment has been minimized.
|
Yikes! Yeah, this was not intentional. I used the phrase "until this bug is fixed: #636" in another repo and since I have permissions on this repo, it closed the issue. Heh. |
This comment has been minimized.
This comment has been minimized.
|
I personally get bitten by this issue so often I've enabled it in my global |
This comment has been minimized.
This comment has been minimized.
|
@dcousens My plan is once/if that eslint bug gets fixed, enable the rule so that:
Not allowed: console.log(foo)
var foo = 5Allowed: function printFoo() {
console.log(foo)
}
var foo = 5This is allowed since it is possible to use When we start enforcing |
This comment has been minimized.
This comment has been minimized.
|
I completely agree, it makes recursive or circular functions completely impossible otherwise. function foo (x) {
return x ? bar(x) : 1
}
function bar (x) {
return foo(x - 1)
}No valid ordering exists with the rule as it stands AFAIK. edit: actually, |
This comment has been minimized.
This comment has been minimized.
|
Enabling "no-use-before-define": [2, "nofunc"],
"no-unused-vars": [2, { "vars": "all", "args": "all", "argsIgnorePattern": "^_[0-9]?$" }]Locally in a project recently, uncovered 5+ critical errors, and 30+ warnings overall that had gone unmissed in a refactoring... bleh I know its blocked, but, it is overall frustrating when the potential errors here are quite frustrating and prolific... Running the same thing against an OSS project I run, and it already found 1 potentially critical error (thankfully not released!) - bitcoinjs/bitcoinjs-lib@fd9c70d |
This comment has been minimized.
This comment has been minimized.
|
@dcousens Are you proposing we just enable the overly aggressive rule even before the eslint issue is addressed? We could do that, but I'm worried about the effect of all the warnings on valid code. We don't want users to think that pleasing I just pinged the team to ask about the status of the blocked issue eslint/eslint#7111 - hopefully this gets it some movement. |
This comment has been minimized.
This comment has been minimized.
|
The issue at eslint/eslint#7111 was just accepted by the team! |
This comment has been minimized.
This comment has been minimized.
|
@feross it was more of a notice for others whom might be waiting on eslint/eslint#7111, but wanted to increase safety in their own projects until it is fixed. |
This comment has been minimized.
This comment has been minimized.
|
This was just merged. Let's include it in the standard v9 release :) |
feross
removed
the
blocked
label
Jan 27, 2017
This comment has been minimized.
This comment has been minimized.
|
This will be part of standard v9. No ecosystem impact. |
feross
added a commit
to standard/eslint-config-standard
that referenced
this issue
Feb 9, 2017
feross
closed this
Feb 9, 2017
feross
added a commit
to standard/eslint-config-standard
that referenced
this issue
Feb 9, 2017
This comment has been minimized.
This comment has been minimized.
caesarsol
commented
Feb 9, 2017
|
Sorry, is this distinguishing between var and let/const? Doe the behaviour change in any way? |
This comment has been minimized.
This comment has been minimized.
|
This rule will prevent clearly wrong code, like: console.log(x)
const x = 5But it will allow code like: function foo () {
console.log(x)
}
const x = 5
foo()Since it works fine :) |
kilianc commentedSep 24, 2016
Not sure what rule is missing but this passes validation: