diff --git a/.changeset/legal-mugs-brake.md b/.changeset/legal-mugs-brake.md new file mode 100644 index 000000000000..ee8dd96b0f9d --- /dev/null +++ b/.changeset/legal-mugs-brake.md @@ -0,0 +1,5 @@ +--- +"@biomejs/biome": patch +--- + +Fixed #8907: `useExhaustiveDependencies` now correctly recognizes stable hook results (like `useState` setters and `useRef` values) when declared with `let`. diff --git a/crates/biome_js_analyze/src/lint/correctness/use_exhaustive_dependencies.rs b/crates/biome_js_analyze/src/lint/correctness/use_exhaustive_dependencies.rs index 9201c8b1b86b..182672db6cdc 100644 --- a/crates/biome_js_analyze/src/lint/correctness/use_exhaustive_dependencies.rs +++ b/crates/biome_js_analyze/src/lint/correctness/use_exhaustive_dependencies.rs @@ -7,7 +7,7 @@ use biome_analyze::{Rule, RuleDiagnostic, RuleDomain, context::RuleContext, decl use biome_console::markup; use biome_diagnostics::Severity; use biome_js_factory::make; -use biome_js_semantic::{CanBeImportedExported, ClosureExtensions, SemanticModel, is_constant}; +use biome_js_semantic::{CanBeImportedExported, ClosureExtensions, ReferencesExtensions, SemanticModel, is_constant}; use biome_js_syntax::binding_ext::AnyJsIdentifierBinding; use biome_js_syntax::{ AnyJsArrayElement, AnyJsArrowFunctionParameters, AnyJsBinding, AnyJsExpression, @@ -709,19 +709,53 @@ fn is_stable_binding( return true; }; - // Only `const` variables are considered stable - if !declaration.is_const() { - return false; - } - let Some(initializer_expression) = declarator .initializer() .and_then(|initializer| initializer.expression().ok()) else { - // This shouldn't happen because we check for `const` above - return true; + // No initializer - only `const` without initializer is stable + // (this shouldn't happen for valid code, but handle gracefully) + return declaration.is_const(); }; + // For non-const declarations, only stable hook results (like useState setters + // or useRef) are considered stable, AND only if the binding is never reassigned. + if !declaration.is_const() { + // First check if the binding was ever reassigned - if so, it's not stable + if binding.all_writes(model).next().is_some() { + return false; + } + + // Check if this is a stable hook result (e.g., setA from useState) + return match get_single_pattern_member(binding, &declarator) { + GetSinglePatternMemberResult::Member(pattern_member) => { + if member.is_some() { + return false; + } + // Only check if initializer is a stable hook call + if let AnyJsExpression::JsCallExpression(call) = &initializer_expression { + is_react_hook_call_stable( + call, + Some(&pattern_member), + model, + &options.stable_config, + ) + } else { + false + } + } + GetSinglePatternMemberResult::NoPattern => { + // For non-destructured let bindings like `let ref = useRef()` + if let AnyJsExpression::JsCallExpression(call) = &initializer_expression { + is_react_hook_call_stable(call, member, model, &options.stable_config) + } else { + false + } + } + _ => false, + }; + } + match get_single_pattern_member(binding, &declarator) { GetSinglePatternMemberResult::Member(pattern_member) => { if member.is_some() { diff --git a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8907.js b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8907.js new file mode 100644 index 000000000000..3241070cc5d2 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8907.js @@ -0,0 +1,25 @@ +/* should not generate diagnostics */ + +import { useEffect, useState, useRef } from "react"; + +// Issue #8907: useState setters and refs declared with `let` should be stable +export const Component = () => { + let [a, setA] = useState(0); + let b = useRef(""); + + useEffect(() => { + setA(1); + b.current = "test"; + }, []); +}; + +// Also test with const for comparison (should still work) +export const Component2 = () => { + const [a, setA] = useState(0); + const b = useRef(""); + + useEffect(() => { + setA(1); + b.current = "test"; + }, []); +}; diff --git a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8907.js.snap b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8907.js.snap new file mode 100644 index 000000000000..fbc364415834 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8907.js.snap @@ -0,0 +1,33 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: issue8907.js +--- +# Input +```js +/* should not generate diagnostics */ + +import { useEffect, useState, useRef } from "react"; + +// Issue #8907: useState setters and refs declared with `let` should be stable +export const Component = () => { + let [a, setA] = useState(0); + let b = useRef(""); + + useEffect(() => { + setA(1); + b.current = "test"; + }, []); +}; + +// Also test with const for comparison (should still work) +export const Component2 = () => { + const [a, setA] = useState(0); + const b = useRef(""); + + useEffect(() => { + setA(1); + b.current = "test"; + }, []); +}; + +``` diff --git a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8907Reassigned.js b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8907Reassigned.js new file mode 100644 index 000000000000..0dea3dbbf288 --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8907Reassigned.js @@ -0,0 +1,21 @@ +import { useEffect, useState, useRef } from "react"; + +// Issue #8907: If a let binding is reassigned, it's no longer stable +export const Component3 = () => { + let [a, setA] = useState(0); + setA = () => {}; // reassigned! + + useEffect(() => { + setA(1); // should now require setA in deps + }, []); +}; + +// Same for useRef +export const Component4 = () => { + let b = useRef(""); + b = { current: "other" }; // reassigned! + + useEffect(() => { + console.log(b.current); // should now require b in deps + }, []); +}; diff --git a/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8907Reassigned.js.snap b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8907Reassigned.js.snap new file mode 100644 index 000000000000..1d342912da3c --- /dev/null +++ b/crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8907Reassigned.js.snap @@ -0,0 +1,96 @@ +--- +source: crates/biome_js_analyze/tests/spec_tests.rs +expression: issue8907Reassigned.js +--- +# Input +```js +import { useEffect, useState, useRef } from "react"; + +// Issue #8907: If a let binding is reassigned, it's no longer stable +export const Component3 = () => { + let [a, setA] = useState(0); + setA = () => {}; // reassigned! + + useEffect(() => { + setA(1); // should now require setA in deps + }, []); +}; + +// Same for useRef +export const Component4 = () => { + let b = useRef(""); + b = { current: "other" }; // reassigned! + + useEffect(() => { + console.log(b.current); // should now require b in deps + }, []); +}; + +``` + +# Diagnostics +``` +issue8907Reassigned.js:8:3 lint/correctness/useExhaustiveDependencies FIXABLE ━━━━━━━━━━━━━━━━━━━━ + + × This hook does not specify its dependency on setA. + + 6 │ setA = () => {}; // reassigned! + 7 │ + > 8 │ useEffect(() => { + │ ^^^^^^^^^ + 9 │ setA(1); // should now require setA in deps + 10 │ }, []); + + i This dependency is being used here, but is not specified in the hook dependency list. + + 8 │ useEffect(() => { + > 9 │ setA(1); // should now require setA in deps + │ ^^^^ + 10 │ }, []); + 11 │ }; + + i React relies on hook dependencies to determine when to re-compute Effects. + Failing to specify dependencies can result in Effects not updating correctly when state changes. + These "stale closures" are a common source of surprising bugs. + + i Either include it or remove the dependency array. + + i Unsafe fix: Add the missing dependency to the list. + + 10 │ ··},·[setA]); + │ ++++ + +``` + +``` +issue8907Reassigned.js:18:3 lint/correctness/useExhaustiveDependencies FIXABLE ━━━━━━━━━━━━━━━━━━━ + + × This hook does not specify its dependency on b.current. + + 16 │ b = { current: "other" }; // reassigned! + 17 │ + > 18 │ useEffect(() => { + │ ^^^^^^^^^ + 19 │ console.log(b.current); // should now require b in deps + 20 │ }, []); + + i This dependency is being used here, but is not specified in the hook dependency list. + + 18 │ useEffect(() => { + > 19 │ console.log(b.current); // should now require b in deps + │ ^^^^^^^^^ + 20 │ }, []); + 21 │ }; + + i React relies on hook dependencies to determine when to re-compute Effects. + Failing to specify dependencies can result in Effects not updating correctly when state changes. + These "stale closures" are a common source of surprising bugs. + + i Either include it or remove the dependency array. + + i Unsafe fix: Add the missing dependency to the list. + + 20 │ ··},·[b.current]); + │ +++++++++ + +```