fix: improve exhaustive dependencies detection#10258
Conversation
🦋 Changeset detectedLatest commit: f643d72 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
📝 WalkthroughWalkthroughThis pull request enhances the exhaustive-deps ESLint rule to improve dependency detection in queryFn callbacks and complex queryKey expressions. The fix introduces utility functions to recursively collect dependencies from queryKey nodes and their nested structures, while refactoring the main rule logic to leverage these utilities for more accurate missing dependency identification. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
View your CI Pipeline Execution ↗ for commit f643d72
☁️ Nx Cloud last updated this comment at |
size-limit report 📦
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts (1)
140-140: Consider using braces to silence the static analysis warning.The static analysis tool flags this line because the arrow function implicitly returns the result of
visit(). Whilevisit()returnsvoid, using braces makes the intent clearer and satisfies the linter.🔧 Suggested fix
- node.arguments.forEach((argument) => visit(argument)) + node.arguments.forEach((argument) => { visit(argument) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts` at line 140, The forEach callback currently uses an implicit return in the line node.arguments.forEach((argument) => visit(argument)); change the arrow function to use a block body to make the intent explicit (e.g., (argument) => { visit(argument); }) so the static analysis warning is silenced; locate the call site using node.arguments.forEach and the visit function in exhaustive-deps.utils.ts and replace the implicit-return arrow with a braced body.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts`:
- Line 140: The forEach callback currently uses an implicit return in the line
node.arguments.forEach((argument) => visit(argument)); change the arrow function
to use a block body to make the intent explicit (e.g., (argument) => {
visit(argument); }) so the static analysis warning is silenced; locate the call
site using node.arguments.forEach and the visit function in
exhaustive-deps.utils.ts and replace the implicit-return arrow with a braced
body.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1417dcbd-9beb-4578-a3ee-751ed43e7917
📒 Files selected for processing (5)
.changeset/perky-seas-flash.mdpackages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.tspackages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.rule.tspackages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.tspackages/eslint-plugin-query/src/utils/ast-utils.ts
🎯 Changes
I updated the lint rule
exhaustive-depsso it now catches dependencies used inside nestedqueryFncallbacks/control flow, and it no longer reports false positives when those dependencies are already encoded in "complex"queryKeyexpressions.✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
Bug Fixes
Tests