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
23 changes: 23 additions & 0 deletions .changeset/dry-files-sniff.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
---
"@biomejs/biome": patch
---

Fixed [#8802](https://github.com/biomejs/biome/issues/8802): `useExhaustiveDependencies` now correctly suggests dependencies without including callback-scoped variables or method names.

When accessing object properties with a callback-scoped variable, only the object path is suggested:

```js
// Now correctly suggests `props.value` instead of `props.value[day]`
useMemo(() => {
return WeekdayValues.filter((day) => props.value[day]);
}, [props.value]);
```

When calling methods on objects, only the object is suggested as a dependency:

```js
// Now correctly suggests `props.data` instead of `props.data.forEach`
useMemo(() => {
props.data.forEach((item) => console.log(item));
}, [props.data]);
```
16 changes: 16 additions & 0 deletions .changeset/ninety-tigers-fetch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
"@biomejs/biome": patch
---

Fixed [#8883](https://github.com/biomejs/biome/issues/8883): `useExhaustiveDependencies` no longer produces false positives when props are destructured in the function body of arrow function components without parentheses around the parameter.

```tsx
type Props = { msg: string };

// Arrow function without parentheses around `props`
const Component: React.FC<Props> = props => {
const { msg } = props;
// Previously, this incorrectly reported `msg` as unnecessary
useEffect(() => console.log(msg), [msg]);
};
```
19 changes: 19 additions & 0 deletions .changeset/tame-eggs-judge.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
---
"@biomejs/biome": patch
---

Fixed [#8885](https://github.com/biomejs/biome/issues/8885): `useExhaustiveDependencies` no longer incorrectly reports variables as unnecessary dependencies when they are derived from expressions containing post/pre-increment operators (`++`/`--`) or compound assignment operators (`+=`, `-=`, etc.).

```js
let renderCount = 0;

export const MyComponent = () => {
// `count` is now correctly recognized as a required dependency
// because `renderCount++` can produce different values between renders
const count = renderCount++;

useEffect(() => {
console.log(count);
}, [count]); // no longer reports `count` as unnecessary
};
```
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,15 @@ 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};
use biome_js_semantic::{CanBeImportedExported, ClosureExtensions, SemanticModel, is_constant};
use biome_js_syntax::binding_ext::AnyJsIdentifierBinding;
use biome_js_syntax::{
AnyJsArrayElement, AnyJsExpression, AnyJsMemberExpression, AnyJsObjectBindingPatternMember,
JsArrayBindingPattern, JsArrayBindingPatternElement, JsArrayBindingPatternElementList,
JsArrayExpression, JsComputedMemberExpression, JsObjectBindingPattern,
JsObjectBindingPatternPropertyList, JsReferenceIdentifier, JsVariableDeclarator, T,
TsTypeofType, is_transparent_expression_wrapper,
AnyJsArrayElement, AnyJsArrowFunctionParameters, AnyJsBinding, AnyJsExpression,
AnyJsMemberExpression, AnyJsObjectBindingPatternMember, JsArrayBindingPattern,
JsArrayBindingPatternElement, JsArrayBindingPatternElementList, JsArrayExpression,
JsComputedMemberExpression, JsObjectBindingPattern, JsObjectBindingPatternPropertyList,
JsReferenceIdentifier, JsVariableDeclarator, T, TsTypeofType,
is_transparent_expression_wrapper,
};
use biome_js_syntax::{
JsCallExpression, JsSyntaxKind, JsSyntaxNode, JsVariableDeclaration, TextRange,
Expand Down Expand Up @@ -540,8 +541,8 @@ fn get_expression_candidates(node: JsSyntaxNode) -> Vec<AnyExpressionCandidate>
parent.kind(),
JsSyntaxKind::JS_SHORTHAND_PROPERTY_OBJECT_MEMBER
) {
if let Some(sequence_expression) = AnyExpressionCandidate::cast_ref(&prev_node) {
result.push(sequence_expression.clone());
if let Some(expression) = AnyExpressionCandidate::cast_ref(&prev_node) {
result.push(expression.clone());
}
return result;
}
Expand All @@ -557,7 +558,27 @@ fn get_expression_candidates(node: JsSyntaxNode) -> Vec<AnyExpressionCandidate>

if let Some(computed_member_expression) = JsComputedMemberExpression::cast_ref(&parent)
&& let Ok(object) = computed_member_expression.object()
&& !prev_node.eq(object.syntax())
{
if !prev_node.eq(object.syntax()) {
return result;
}
// collect only constant member access expressions
if let Ok(member) = computed_member_expression.member()
&& !is_constant(&member)
{
return result;
}
}

// Follow eslint React plugin behavior:
// When calling a method of an object, only the object should be included in the dependency list.
if matches!(
parent.kind(),
JsSyntaxKind::JS_STATIC_MEMBER_EXPRESSION | JsSyntaxKind::JS_COMPUTED_MEMBER_EXPRESSION
) && let Some(wrapper) = parent.parent()
&& let Some(call_expression) = JsCallExpression::cast(wrapper)
&& let Ok(callee) = call_expression.callee()
&& callee.syntax().eq(&parent)
{
return result;
}
Expand Down Expand Up @@ -849,8 +870,20 @@ fn is_stable_expression(
&& let Some(binding) = model.binding(&name)
{
let binding = &binding.tree();
if let Some(declaration_node) =
&binding.declaration().map(|decl| decl.syntax().clone())
let declaration = binding.declaration();
let declaration_node = if let Some(
AnyJsBindingDeclaration::JsArrowFunctionExpression(arrow_function),
) = &declaration
&& let Ok(AnyJsArrowFunctionParameters::AnyJsBinding(
AnyJsBinding::JsIdentifierBinding(identifier),
)) = arrow_function.parameters()
&& identifier.syntax().eq(binding.syntax())
{
Some(identifier.syntax().clone())
} else {
declaration.map(|decl| decl.syntax().clone())
};
if let Some(declaration_node) = &declaration_node
&& identifier
.syntax()
.ancestors()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import { useMemo } from "react";

export interface Days {
mon: boolean;
tue: boolean;
wed: boolean;
thu: boolean;
fri: boolean;
}

const WeekdayValues: (keyof Days)[] = ["mon", "tue", "wed", "thu", "fri"];

// "day" doesn't exist outside the memoized function.
// Biome should report "props.value" as a missing dependency,
// NOT "props.value[day]" since "day" is a callback-scoped variable.
function Component1(props: { value: Days }) {
useMemo(() => {
return WeekdayValues.filter((day) => props.value[day]);
}, []);
}

// Example with forEach and index
// Biome should report "props.data" as missing dependency,
// NOT "props.data.forEach" to follow eslint React plugin behavior.
function Component2(props: { data: number[] }) {
useMemo(() => {
props.data.forEach((value, index) => {
console.log(value, index);
});
}, []);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 152
expression: issue8802.ts
---
# Input
```ts
import { useMemo } from "react";

export interface Days {
mon: boolean;
tue: boolean;
wed: boolean;
thu: boolean;
fri: boolean;
}

const WeekdayValues: (keyof Days)[] = ["mon", "tue", "wed", "thu", "fri"];

// "day" doesn't exist outside the memoized function.
// Biome should report "props.value" as a missing dependency,
// NOT "props.value[day]" since "day" is a callback-scoped variable.
function Component1(props: { value: Days }) {
useMemo(() => {
return WeekdayValues.filter((day) => props.value[day]);
}, []);
}

// Example with forEach and index
// Biome should report "props.data" as missing dependency,
// NOT "props.data.forEach" to follow eslint React plugin behavior.
function Component2(props: { data: number[] }) {
useMemo(() => {
props.data.forEach((value, index) => {
console.log(value, index);
});
}, []);
}

```

# Diagnostics
```
issue8802.ts:17:5 lint/correctness/useExhaustiveDependencies FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× This hook does not specify its dependency on props.value.

15 │ // NOT "props.value[day]" since "day" is a callback-scoped variable.
16 │ function Component1(props: { value: Days }) {
> 17 │ useMemo(() => {
│ ^^^^^^^
18 │ return WeekdayValues.filter((day) => props.value[day]);
19 │ }, []);

i This dependency is being used here, but is not specified in the hook dependency list.

16 │ function Component1(props: { value: Days }) {
17 │ useMemo(() => {
> 18 │ return WeekdayValues.filter((day) => props.value[day]);
│ ^^^^^^^^^^^
19 │ }, []);
20 │ }

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.

19 │ ····},·[props.value]);
│ +++++++++++

```

```
issue8802.ts:26:5 lint/correctness/useExhaustiveDependencies FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× This hook does not specify its dependency on props.data.

24 │ // NOT "props.data.forEach" to follow eslint React plugin behavior.
25 │ function Component2(props: { data: number[] }) {
> 26 │ useMemo(() => {
│ ^^^^^^^
27 │ props.data.forEach((value, index) => {
28 │ console.log(value, index);

i This dependency is being used here, but is not specified in the hook dependency list.

25 │ function Component2(props: { data: number[] }) {
26 │ useMemo(() => {
> 27 │ props.data.forEach((value, index) => {
│ ^^^^^^^^^^
28 │ console.log(value, index);
29 │ });

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.

30 │ ····},·[props.data]);
│ ++++++++++

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import React, { useEffect } from 'react'

type ExampleProps = {
msg: string;
};

const DestructuredInParams: React.FC<ExampleProps> = ({ msg }) => {
useEffect(() => console.log(msg), [msg]); // correct behavior
};

const DestructuredInBody1: React.FC<ExampleProps> = props => {
const { msg } = props;
useEffect(() => console.log(msg), [msg]); // should NOT trigger useExhaustiveDependencies
};

const DestructuredInBody2: React.FC<ExampleProps> = props => {
const { msg } = props;
useEffect(() => console.log(msg), []); // should trigger useExhaustiveDependencies
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
---
source: crates/biome_js_analyze/tests/spec_tests.rs
assertion_line: 152
expression: issue8883.tsx
---
# Input
```tsx
import React, { useEffect } from 'react'

type ExampleProps = {
msg: string;
};

const DestructuredInParams: React.FC<ExampleProps> = ({ msg }) => {
useEffect(() => console.log(msg), [msg]); // correct behavior
};

const DestructuredInBody1: React.FC<ExampleProps> = props => {
const { msg } = props;
useEffect(() => console.log(msg), [msg]); // should NOT trigger useExhaustiveDependencies
};

const DestructuredInBody2: React.FC<ExampleProps> = props => {
const { msg } = props;
useEffect(() => console.log(msg), []); // should trigger useExhaustiveDependencies
};

```

# Diagnostics
```
issue8883.tsx:18:5 lint/correctness/useExhaustiveDependencies FIXABLE ━━━━━━━━━━━━━━━━━━━━━━━━━━━━

× This hook does not specify its dependency on msg.

16 │ const DestructuredInBody2: React.FC<ExampleProps> = props => {
17 │ const { msg } = props;
> 18 │ useEffect(() => console.log(msg), []); // should trigger useExhaustiveDependencies
│ ^^^^^^^^^
19 │ };
20 │

i This dependency is being used here, but is not specified in the hook dependency list.

16 │ const DestructuredInBody2: React.FC<ExampleProps> = props => {
17 │ const { msg } = props;
> 18 │ useEffect(() => console.log(msg), []); // should trigger useExhaustiveDependencies
│ ^^^
19 │ };
20 │

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.

18 │ ····useEffect(()·=>·console.log(msg),·[msg]);·//·should·trigger·useExhaustiveDependencies
│ +++

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/* should not generate diagnostics */
import { useEffect } from "react";

let renderCount = 0;

// This should not report `count` as unnecessary dependency.
// `count` is derived from a global variable and can change between renders.
export const MyComponent = () => {
const count = renderCount++;

useEffect(() => {
console.log(count);
}, [count]);
};

// Reading from a global variable without modification is considered stable.
let globalConfig = { debug: false };

export const Component2 = () => {
const debug = globalConfig.debug;

useEffect(() => {
if (debug) {
console.log("Debug mode enabled");
}
}, []);
};
Loading
Loading