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/dry-spies-go.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@biomejs/biome": patch
---

Reverted a behavior change in [`useExhaustiveDependencies`](https://biomejs.dev/linter/rules/use-exhaustive-dependencies/) that was accidentally included as part of the [#8802](https://github.com/biomejs/biome/issues/8802) fix. The change made method calls on objects (e.g., `props.data.forEach(...)`) report only the object (`props.data`) as a missing dependency instead of the full member expression. This behavior change will be reconsidered separately.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@ 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, ReferencesExtensions, 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 @@ -587,19 +589,6 @@ fn get_expression_candidates(node: JsSyntaxNode) -> Vec<AnyExpressionCandidate>
}
}

// 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;
}

if matches!(
parent.kind(),
JsSyntaxKind::JS_IDENTIFIER_EXPRESSION
Expand Down Expand Up @@ -1627,12 +1616,13 @@ impl Rule for UseExhaustiveDependencies {
let new_elements = captures.first().into_iter().filter_map(|node| {
if let Some(jsx_ref) = JsxReferenceIdentifier::cast_ref(node) {
return Some(AnyJsArrayElement::AnyJsExpression(
make::js_identifier_expression(
make::js_reference_identifier(jsx_ref.value_token().ok()?)
).into()
make::js_identifier_expression(make::js_reference_identifier(
jsx_ref.value_token().ok()?,
))
.into(),
));
}

node.ancestors()
.find_map(|node| match JsReferenceIdentifier::cast_ref(&node) {
Some(node) => Some(make::js_identifier_expression(node).into()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,8 @@ 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 }) {
function Component(props: { value: Days }) {
useMemo(() => {
return WeekdayValues.filter((day) => props.value[day]);
}, []);
Comment on lines +16 to 19
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's look at the actual test file to see what was there
cat crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8802.ts

Repository: biomejs/biome

Length of output: 611


🏁 Script executed:

# Search for forEach patterns in the useExhaustiveDependencies test directory
rg -rn 'forEach' crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/ --type=ts

Repository: biomejs/biome

Length of output: 39


🏁 Script executed:

# Also search for issue `#8958` references in the codebase
rg -rn '8958' crates/biome_js_analyze/tests/ --type=ts

Repository: biomejs/biome

Length of output: 39


🏁 Script executed:

# Search more broadly for `#8958` or issue8958 references
rg -rn '8958' crates/biome_js_analyze/

Repository: biomejs/biome

Length of output: 39


🏁 Script executed:

# Search for forEach in all useExhaustiveDependencies related files
fd 'useExhaustiveDependencies' crates/biome_js_analyze/ --type f

Repository: biomejs/biome

Length of output: 39


🏁 Script executed:

# Check git history to see what Component2 contained before removal
cd crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies && git log --oneline -n 20 issue8802.ts

Repository: biomejs/biome

Length of output: 161


🏁 Script executed:

# Check the specific commit to see what Component2 was and why it was removed
git show c2fa48f1:crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8802.ts

Repository: biomejs/biome

Length of output: 611


🏁 Script executed:

# Check the current commit to see if there are multiple components or just one
git log --oneline -n 5 crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8802.ts

Repository: biomejs/biome

Length of output: 161


🏁 Script executed:

# Check what the file looked like before the fixing commit
git show c2fa48f1^:crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8802.ts 2>/dev/null || echo "File may not have existed before"

Repository: biomejs/biome

Length of output: 89


🏁 Script executed:

# Get more detailed git log to see if there were previous versions
git log --all --oneline -- crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8802.ts | head -20

Repository: biomejs/biome

Length of output: 270


🏁 Script executed:

# Check the previous commit f788cff2 to see what Component2 contained
git show f788cff2:crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8802.ts

Repository: biomejs/biome

Length of output: 955


🏁 Script executed:

# Check the diff between the two commits to see exactly what was removed
git diff f788cff2 c2fa48f1 -- crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8802.ts

Repository: biomejs/biome

Length of output: 1294


🏁 Script executed:

# Search more broadly for forEach patterns in the entire useExhaustiveDependencies test suite
find crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies -name "*.ts" -exec grep -l "forEach" {} \;

Repository: biomejs/biome

Length of output: 39


🏁 Script executed:

# Check if there's a dedicated test for issue `#8958`
find crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies -name "*8958*"

Repository: biomejs/biome

Length of output: 39


🏁 Script executed:

# Look at the commit message more carefully to understand the intent
git show c2fa48f1 --stat | head -30

Repository: biomejs/biome

Length of output: 1780


🏁 Script executed:

# Check if there are any other tests for forEach or method calls in the useExhaustiveDependencies directory
find crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies -name "*.ts" | xargs ls -la

Repository: biomejs/biome

Length of output: 1141


🏁 Script executed:

# Look at the linter code change to understand what was reverted
git show c2fa48f1 -- crates/biome_js_analyze/src/correctness/use_exhaustive_dependencies.rs | head -100

Repository: biomejs/biome

Length of output: 39


🏁 Script executed:

# Let's check the actual linter implementation to understand what behavior was reverted
git show c2fa48f1 crates/biome_js_analyze/src/correctness/use_exhaustive_dependencies.rs 2>&1 | head -150

Repository: biomejs/biome

Length of output: 307


🏁 Script executed:

# Look at the expected test output files to understand what the test validates
cat crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8802.ts.snap

Repository: biomejs/biome

Length of output: 1987


Add a test for the forEach method call pattern from issue #8958.

The removed Component2 tested the specific props.data.forEach(...) pattern that triggered the original regression. Without it, that scenario no longer has regression test coverage. Worth re-adding to prevent re-introducing the same issue.

🤖 Prompt for AI Agents
In
`@crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8802.ts`
around lines 16 - 19, Reintroduce a test that covers the props.data.forEach call
pattern by adding back a Component2 (or equivalent) that calls
props.data.forEach(...) inside a hook callback (e.g., useMemo or useEffect) with
an empty dependency array so the analyzer must detect the dependency;
specifically, create a Component2 function that accepts props.data and invokes
props.data.forEach(item => { /* access item or mutate local */ }) inside the
hook body (mirroring the original regression case) to ensure the forEach pattern
is exercised by the test suite.

}

// 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
Expand Up @@ -20,23 +20,12 @@ 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 }) {
function Component(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
Expand All @@ -46,15 +35,15 @@ 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 }) {
16 │ function Component(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 }) {
16 │ function Component(props: { value: Days }) {
17 │ useMemo(() => {
> 18 │ return WeekdayValues.filter((day) => props.value[day]);
│ ^^^^^^^^^^^
Expand All @@ -73,37 +62,3 @@ issue8802.ts:17:5 lint/correctness/useExhaustiveDependencies FIXABLE ━━━
│ +++++++++++

```

```
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]);
│ ++++++++++

```