Skip to content
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

Disallow declaration of global variables #667

Closed
wants to merge 1 commit into from
Closed

Disallow declaration of global variables #667

wants to merge 1 commit into from

Conversation

foray1010
Copy link

'no-shadow': [2, {'builtinGlobals': true, 'hoist': 'functions', 'allow': []}],

@@ -11,7 +11,7 @@ module.exports = {
// disallow shadowing of names such as arguments
'no-shadow-restricted-names': 2,
// disallow declaration of variables already declared in the outer scope
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you mind adding a new comment line linking to the rule definition (http://eslint.org/docs/rules/no-shadow.html)?

@ljharb
Copy link
Collaborator

ljharb commented Jan 8, 2016

This might also be nice to reference somewhere in the actual guide. LGTM overall, pending 2 comments.

@foray1010
Copy link
Author

Updated according to the comments

@foray1010
Copy link
Author

enabling this will disable the use of a lot of variables when env browser is true, such as name and length
See https://github.com/sindresorhus/globals/blob/master/globals.json#L164

My team disable browser because most of our projects are backend
but for airbnb, I think you guys may have different preference
Feel free to close this PR

@ljharb
Copy link
Collaborator

ljharb commented Jan 8, 2016

I don't want the JS builtins shadowed, but that's a good point - given that it includes all environment builtins, perhaps this would be a bit too restrictive.

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

Successfully merging this pull request may close these issues.

None yet

2 participants