Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/legal-mugs-brake.md
Original file line number Diff line number Diff line change
@@ -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`.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
@@ -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";
}, []);
};
Original file line number Diff line number Diff line change
@@ -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";
}, []);
};

```
Original file line number Diff line number Diff line change
@@ -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
}, []);
};
Original file line number Diff line number Diff line change
@@ -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]);
│ +++++++++

```
Loading