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

Proposal: change no-redeclare default behavior #11405

Closed
mysticatea opened this issue Feb 16, 2019 · 2 comments

Comments

Projects
4 participants
@mysticatea
Copy link
Member

commented Feb 16, 2019

What rule do you want to change?

Does this change cause the rule to produce more or fewer warnings?

  • More Warnings.

How will the change be implemented? (New option, new default behavior, etc.)?

  • It changes the default value of builtinGlobals option to true.

Please provide some example code that this change will affect:

/*eslint-env browser */
/*eslint no-redeclare: error */

var top = 100

What does the rule currently do for this code?

What will the rule do after it's changed?

Are you willing to submit a pull request to implement this change?

Yes.

Context:

  • We cannot redeclare the top variable, browsers ignore the var top declaration silently. This can cause confusion and I believe that it's worth to warn by default.
  • Originate no-undef rule had warned redeclarations of globals by default, but it was removed in #4504. I think that we can re-enable it on no-redeclare rule.

@mysticatea mysticatea added this to Needs discussion in v6.0.0 Feb 16, 2019

@mysticatea mysticatea self-assigned this Feb 16, 2019

@platinumazure

This comment has been minimized.

Copy link
Member

commented Feb 16, 2019

👍 Yes! This is a sensible default. My guess is we had to implement the option with a default disabled state so it wasn't a breaking change, but we should absolutely change this in the next major release.

@nzakas nzakas added accepted and removed evaluating labels Feb 28, 2019

@nzakas nzakas moved this from Needs discussion to Accepted, ready to implement in v6.0.0 Feb 28, 2019

@not-an-aardvark

This comment has been minimized.

Copy link
Member

commented Mar 1, 2019

This issue was accepted in today's TSC meeting.

mysticatea added a commit that referenced this issue Mar 15, 2019

@mysticatea mysticatea moved this from Accepted, ready to implement to Implemented, pending review in v6.0.0 Mar 15, 2019

v6.0.0 automation moved this from Implemented, pending review to Done Apr 25, 2019

mysticatea added a commit that referenced this issue Apr 25, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.