search

expressjs / csurf

CSRF token middleware
MIT License
2.3k stars 217 forks source link

fix JSHint warnings #16

Closed niftylettuce closed 10 years ago

dougwilson commented 10 years ago

We don't use JSHint, and the removal of that var makes a global variable leak.

niftylettuce commented 10 years ago

You should still have strict === comparison at least. Redefining variables isn't great practice either afaik. On Jun 11, 2014 11:53 PM, "Douglas Christopher Wilson" < notifications@github.com> wrote:

We don't use JSHint, and the removal of that var makes a global variable leak.

— Reply to this email directly or view it on GitHub https://github.com/expressjs/csurf/pull/16#issuecomment-45827609.

dougwilson commented 10 years ago

Yes, === would be better, but it'll be changed with something else in a future update, since it doesn't particularly matter. The var doesn't make a difference, because it's not actually redefining variables, if you read the ECMAScript spec. JSHint is just a tool that gives opinions, it does not detect actual errors, just things you configured it to yell about.

tj commented 10 years ago

=== doesn't matter in tons of cases, typeof, strings that can't possibly produce false positives etc. the linters suck basically

niftylettuce commented 10 years ago

Good to know, thanks all On Jun 12, 2014 12:18 AM, "TJ Holowaychuk" notifications@github.com wrote:

=== doesn't matter in tons of cases, typeof, strings that can't possibly produce false positives etc. the linters suck basically

— Reply to this email directly or view it on GitHub https://github.com/expressjs/csurf/pull/16#issuecomment-45828625.

dougwilson commented 10 years ago

@niftylettuce FWIW at $work we strictly adhere to things like JSHint (because it's good practice and helps when you have a bunch of people with different skill sets), but it doesn't apply to everywhere :)