Docs: Add valid example that shows vars in a block scope#14230
Docs: Add valid example that shows vars in a block scope#14230nzakas merged 2 commits intoeslint:masterfrom
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.
nzakas
left a comment
There was a problem hiding this comment.
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. |
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?