-
-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Docs: Add valid example that shows vars in a block scope #14230
Conversation
Hi @edg2s!, thanks for the Pull Request The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
Hi @edg2s!, thanks for the Pull Request The first commit message isn't properly formatted. We ask that you update the message to match this format, as we use it to generate changelogs and automate releases.
Read more about contributing to ESLint here |
All the current valid examples show hoisting the var to the function scope. This example makes is clear that this rule allows you to declare vars inside a block scope, just not use them out of that scope.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. Can you add a corresponding example in the “invalid” section, too? Note that all of the current examples in the “valid” section are rewritten versions of the code in the “invalid” section, and that is by design.
I did notice that, but I don't think there is a functionally equivalent invalid example for this function. I could move the console.log down but that would be different functionality. |
You may want to consider a different example, then, that can show the same thing. |
Any invalid case would have a variable being used outside the scope in which it was defined. In order to fix that without changing the functionality you would have to move the variable declaration further up, but the whole point of this PR is to add an example of a var in a block scope. |
And there are many other places where the examples don't (or can't) match up perfectly, e.g. https://eslint.org/docs/rules/no-compare-neg-zero, https://eslint.org/docs/rules/valid-typeof, https://eslint.org/docs/rules/dot-location, ... |
The goal isn’t necessarily to show a bad example and then the fixed version, it’s to show an example of what the rule will flag and then a similar example that shows what the rule won’t flag. Despite other rules not always following this pattern, this rule’s documentation does, so I’d prefer to stick with it. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks!
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[x] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofixing to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
What changes did you make? (Give an overview)
All the current valid examples show hoisting the var to the function scope. This example makes is clear that this rule allows you to declare vars inside a block scope, just not use them out of that scope.
Is there anything you'd like reviewers to focus on?