Skip to content

no-unused-vars rule don't work while extending the airbnb's config #773

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
artptr opened this issue Mar 3, 2016 · 10 comments
Closed

no-unused-vars rule don't work while extending the airbnb's config #773

artptr opened this issue Mar 3, 2016 · 10 comments

Comments

@artptr
Copy link

artptr commented Mar 3, 2016

This repo reproduces the bug in eslint-config-airbnb.
There is an error while checking the test.js:

  1:5  error  'Foo' is defined but never used  no-unused-vars

The error appears despite that the parent config contains appropriate no-unused-vars setting.
It disappears only if we remove the airbnb's config (default or legacy) from the extends property.

The error appears on the ESLint 1.10.3 and 2.2.0

@ljharb
Copy link
Collaborator

ljharb commented Mar 3, 2016

According to the eslint docs, setting the "vars" option should still warn about unused local vars - it seems that error is correct, and that var is unused, meaning that line could be deleted and your program would work the same.

@ljharb
Copy link
Collaborator

ljharb commented Mar 3, 2016

( I'm referring both to https://github.com/artptr/eslint-airbnb-no-unused-vars-bug/blob/master/.eslint-project-rules.json#L3 and to the redundant https://github.com/artptr/eslint-airbnb-no-unused-vars-bug/blob/master/.eslintrc.json#L8 )

btw thanks for the reproduction repo, that makes it super easy to understand what's going on!

@vkrol
Copy link

vkrol commented Mar 3, 2016

@ljharb Foo is the global variable, not local.

@ljharb
Copy link
Collaborator

ljharb commented Mar 3, 2016

Every file in node is in a module context, which means it's not at all a global variable.

@ljharb
Copy link
Collaborator

ljharb commented Mar 3, 2016

@vkrol you can verify that by checking global.Foo against Foo inside that file.

@artptr
Copy link
Author

artptr commented Mar 3, 2016

So the airbnb/legacy is not for a browser environment, but for an old Node.js environment, right?

@ljharb
Copy link
Collaborator

ljharb commented Mar 3, 2016

@artptr it's for both. but eslint runs with node, and assumes your files are run with node. You should be using browserify or the like to wrap your files anyway such that nothing in your browser runs directly in the global scope.

@artptr
Copy link
Author

artptr commented Mar 3, 2016

@ljharb but if we remove the "airbnb/legacy" string from extends property then all will be ok and local "no-unused-vars": [2, { vars: "local" } ] rule will be work because it will decide that Foo is a global variable in a script in a browser.

@artptr
Copy link
Author

artptr commented Mar 3, 2016

Also it may be returned to assumed behavior by adding

"env": {
  "node": false
}

@ljharb
Copy link
Collaborator

ljharb commented Mar 3, 2016

Right - this isn't a bug, because in 2016 everything should be run in a node context, and only after running through browserify/webpack should it be shipped to a browser.

You're certainly free to override the env settings in your eslint config, however!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants