fix(linter): recognize stable hook results declared with let#8927
fix(linter): recognize stable hook results declared with let#8927mdevils merged 4 commits intobiomejs:mainfrom
let#8927Conversation
When `useState` setters or `useRef` values are destructured into `let` bindings (e.g., `let [a, setA] = useState(0)`), they should still be recognized as stable and not require inclusion in dependency arrays. The previous code returned early for non-const declarations without checking if the initializer was a stable hook call. This fix checks stable hook results for both `let` and `const` declarations. Fixes biomejs#8907
🦋 Changeset detectedLatest commit: caa1010 The changes in this PR will be included in the next version bump. This PR includes changesets to release 13 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughThis PR fixes the Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
cc @mdevils |
mdevils
left a comment
There was a problem hiding this comment.
@littleKitchen thank you for your contributions! Please check my comments.
| }; | ||
|
|
||
| // For non-const declarations, only stable hook results (like useState setters | ||
| // or useRef) are considered stable. Other values may be reassigned. |
There was a problem hiding this comment.
Other values may be reassigned.
All lets may be reassigned. eslint rule checks specifically if it was reassigned in case of stable hook results. Please check all writes of the binding to make sure it wasn't rewritten.
crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8907.js
Show resolved
Hide resolved
|
I see, thanks for the review. |
Addresses reviewer feedback: verify that the let binding was not reassigned before treating it as stable. If the binding has any writes, return false (not stable). Also adds test case for reassigned useState setter and useRef.
|
@littleKitchen thank you for your quick response and fixes! Please include a changeset for your changes. Here is an example changeset: https://github.com/biomejs/biome/blob/main/.changeset/ninety-tigers-fetch.md You can create a new changeset using |
Merging this PR will not alter performance
Comparing Footnotes
|
|
@littleKitchen thank you for your contribution! |
|
Thanks! |
Summary
When
useStatesetters oruseRefvalues are destructured intoletbindings, they should still be recognized as stable and not require inclusion in dependency arrays.Problem
The
useExhaustiveDependenciesrule was returning early for non-const declarations without checking if the initializer was a stable hook call.Solution
Check stable hook results for both
letandconstdeclarations. Forletdeclarations, only stable hook results (likeuseStatesetters at index 1, oruseRefidentity) are considered stable.Test Plan
issue8907.jstest caseuseExhaustiveDependenciestests passFixes #8907