-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
feat: support explicit resource management in no-loop-func
#19895
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
Conversation
✅ Deploy Preview for docs-eslint ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
docs/src/rules/no-loop-func.md
Outdated
for (var i=10; i; i--) { | ||
using foo = getsomething(i); | ||
var a = function() { return foo; }; // OK, all references are referring to block scoped variables in the loop. | ||
a(); | ||
} | ||
|
||
for (var i=10; i; i--) { | ||
await using foo = getsomething(i); | ||
var a = function() { return foo; }; // OK, all references are referring to block scoped variables in the loop. | ||
a(); | ||
} |
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.
I think we will need to pass ECMA version 2026 in the code block to correctly open this in the playground.
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.
Doesn't it already work the same with the default latest
?
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.
Yeah, that would work too. Anyway I also added 2026 as an option in eslint/eslint.org#737
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!
Leaving it open for the 2nd review.
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.
Prerequisites checklist
What is the purpose of this pull request? (put an "X" next to an item)
[ ] Documentation update
[ ] Bug fix (template)
[ ] New rule (template)
[x] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:
Refs #19792, adds support for explicit resource management to the
no-loop-func
rule.What changes did you make? (Give an overview)
Updates the
no-loop-func
rule to not report on references tousing
andawait using
variables, because they are constant.Is there anything you'd like reviewers to focus on?
For
using
/await using
variables declared inside the loop body, it might make sense to warn when they are referenced from a function because they might be used after the loop, which would be after the disposal. However, I don't think this potential error is specific to loops, but to any blocks. Thus, I think the mentioned is out of scope for this rule, which targets cases where the referenced variables are reassigned.