-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
fix(eslint-plugin-query): Exhaustive deps rule not working for vue files #10011
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
fix(eslint-plugin-query): Exhaustive deps rule not working for vue files #10011
Conversation
🦋 Changeset detectedLatest commit: 818e9b7 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 |
📝 WalkthroughWalkthroughThe changes fix the exhaustive-deps ESLint rule for Vue single-file components by adding filename context to dependency reference validation. When no function ancestor exists, the rule now specifically validates Vue files and treats undefined definitions and imports as globals, preventing false positives. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
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 818e9b7
☁️ Nx Cloud last updated this comment at |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #10011 +/- ##
===========================================
+ Coverage 45.82% 83.82% +37.99%
===========================================
Files 200 19 -181
Lines 8525 581 -7944
Branches 1977 216 -1761
===========================================
- Hits 3907 487 -3420
+ Misses 4158 72 -4086
+ Partials 460 22 -438
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
��� Nitpick comments (1)
packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts (1)
26-40: LGTM! Vue file handling logic is sound.The Vue-specific logic correctly identifies Vue single-file components and excludes global variables and imports from required dependencies, matching the expected behavior from React's exhaustive-deps rule. This properly handles Vue 3's Composition API script setup where code runs at the module level without a wrapping function.
💡 Optional: Consider adding an explanatory comment
Adding a brief comment would help future maintainers understand why Vue files are treated differently:
} else { + // Vue SFCs with script setup run at module level without a function wrapper. + // Check filename to apply Vue-specific reference resolution. const isVueFile = filename.endsWith('.vue')
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.changeset/slimy-taxes-make.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.ts
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-09-02T17:57:33.184Z
Learnt from: TkDodo
Repo: TanStack/query PR: 9612
File: packages/query-async-storage-persister/src/asyncThrottle.ts:0-0
Timestamp: 2025-09-02T17:57:33.184Z
Learning: When importing from tanstack/query-core in other TanStack Query packages like query-async-storage-persister, a workspace dependency "tanstack/query-core": "workspace:*" needs to be added to the package.json.
Applied to files:
.changeset/slimy-taxes-make.md
📚 Learning: 2025-08-19T03:18:18.303Z
Learnt from: oscartbeaumont
Repo: TanStack/query PR: 9564
File: packages/solid-query-devtools/src/production.tsx:2-3
Timestamp: 2025-08-19T03:18:18.303Z
Learning: In the solid-query-devtools package, the codebase uses a pattern of type-only default imports combined with typeof for component type annotations (e.g., `import type SolidQueryDevtoolsComp from './devtools'` followed by `typeof SolidQueryDevtoolsComp`). This pattern is consistently used across index.tsx and production.tsx files, and the maintainers prefer consistency over changing this approach.
Applied to files:
packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts
📚 Learning: 2025-11-22T09:06:05.219Z
Learnt from: sukvvon
Repo: TanStack/query PR: 9892
File: packages/solid-query-persist-client/src/__tests__/PersistQueryClientProvider.test.tsx:331-335
Timestamp: 2025-11-22T09:06:05.219Z
Learning: In TanStack/query test files, when a queryFn contains side effects (e.g., setting flags for test verification), prefer async/await syntax for clarity; when there are no side effects, prefer the .then() pattern for conciseness.
Applied to files:
packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts
🧬 Code graph analysis (2)
packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts (1)
packages/eslint-plugin-query/src/utils/ast-utils.ts (1)
ASTUtils(5-391)
packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts (1)
packages/eslint-plugin-query/src/__tests__/test-utils.ts (1)
normalizeIndent(3-7)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test
🔇 Additional comments (8)
packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts (2)
11-13: LGTM! Filename context added correctly.The filename parameter is properly added to the function signature and destructured, enabling file-context-aware relevance checks for Vue files.
16-26: LGTM! Existing React/Solid logic preserved correctly.The function ancestor check remains unchanged, maintaining the original behavior for React and Solid components where references must be declared within the component function.
packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.rule.ts (1)
85-85: LGTM! Filename context properly plumbed through.The filename from the ESLint rule context is correctly passed to the utility function, enabling file-based relevance checks.
packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts (5)
551-563: LGTM! Vue valid test case properly structured.This test correctly validates that the rule passes when local variables are properly included in the queryKey for Vue single-file components with script setup.
564-576: LGTM! Import exclusion test is correct.This test properly validates that imported functions (ImportBinding) are correctly excluded from required dependencies in Vue files, matching the expected behavior from React hooks.
577-589: LGTM! Global variable exclusion test is correct.This test properly validates that global variables like
fetchare correctly excluded from required dependencies while local variables likeidare still required, demonstrating the correct distinction in Vue files.
1017-1050: LGTM! Single missing dependency test is comprehensive.This test correctly validates that the rule identifies missing local variables in Vue files and provides an appropriate fix suggestion. The test structure matches existing patterns and the expected output is accurate.
1051-1086: LGTM! Multiple missing dependencies test is comprehensive.This test correctly validates that the rule can identify and suggest fixes for multiple missing dependencies in Vue files, with proper comma-separated output and correct fix suggestions.
🎯 Changes
Fixes: #9962
In this PR, I am identifying vue files and and fixing the exhaustive depths rule not working for them.
Motivation - Thought to start my new year with some OSS contribution
In react, Similar case which works fine
Before this PR
After this PR
✅ Checklist
pnpm run test:pr.🚀 Release Impact