fix(linter): useExhaustiveDependencies revert object method call behavior change, fixes #8958#9017
Conversation
…havior change, fixes biomejs#8958
🦋 Changeset detectedLatest commit: c2fa48f 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 |
github-actions
bot
added
A-Linter
mdevils
changed the title
useExhaustiveDependencies revert object method call be…useExhaustiveDependencies revert object method call behavior change, fixes #8958
WalkthroughThis PR reverts a recent behaviour change in the useExhaustiveDependencies linter rule. It removes a conditional block that previously skipped reporting method calls on objects, restoring the original logic where expressions like Possibly related PRs
Suggested labels
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In
`@crates/biome_js_analyze/tests/specs/correctness/useExhaustiveDependencies/issue8802.ts`:
- Around line 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.
| function Component(props: { value: Days }) { | ||
| useMemo(() => { | ||
| return WeekdayValues.filter((day) => props.value[day]); | ||
| }, []); |
There was a problem hiding this comment.
🧩 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.tsRepository: 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=tsRepository: 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=tsRepository: 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 fRepository: 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.tsRepository: 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.tsRepository: 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.tsRepository: 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 -20Repository: 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.tsRepository: 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.tsRepository: 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 -30Repository: 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 -laRepository: 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 -100Repository: 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 -150Repository: 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.snapRepository: 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.
Merging this PR will not alter performance
Comparing 
Footnotes
|
2 participants
Summary
Reverted a behavior change in
useExhaustiveDependenciesthat was accidentally included as part of the #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.Closes #8958
Test Plan
Test was updated.
Docs