feat(eslint-plugin-query): add prefer-query-options rule#10184
Open
danielpza wants to merge 23 commits intoTanStack:mainfrom
Open
feat(eslint-plugin-query): add prefer-query-options rule#10184danielpza wants to merge 23 commits intoTanStack:mainfrom
danielpza wants to merge 23 commits intoTanStack:mainfrom
Conversation
🦋 Changeset detectedLatest commit: 15c05dd 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 |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a new ESLint rule "prefer-query-options" to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
danielpza
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (1)
packages/eslint-plugin-query/src/__tests__/prefer-query-options.test.ts (1)
7-32: Add test cases for the other hooks inuseQueryHooks.Only
useQueryis exercised here. The rule'suseQueryHookslist includesuseInfiniteQuery,useSuspenseQuery,useSuspenseInfiniteQuery,useQueries, anduseSuspenseQueries. A test for at least one additional hook (e.g.,useInfiniteQuery) would confirm the hook-matching logic works across the full list.🧪 Suggested additional test cases
// useQuery hooks ruleTester.run(rule.name, rule, { valid: [ { code: `useQuery(usersQuery)` }, { code: `useQuery({ ...usersQuery })` }, { code: `useQuery({ ...usersQuery() })` }, { code: `useQuery({ ...usersQuery, meta: {} })` }, + { code: `useInfiniteQuery(usersQuery)` }, + { code: `useSuspenseQuery(usersQuery)` }, ], invalid: [ { code: `useQuery({ queryKey: [] })`, errors: [{ messageId: 'no-inline-query-hook' }], }, + { + code: `useInfiniteQuery({ queryKey: [], queryFn: () => {} })`, + errors: [{ messageId: 'no-inline-query-hook' }], + }, + { + code: `useSuspenseQuery({ queryKey: [] })`, + errors: [{ messageId: 'no-inline-query-hook' }], + }, // ... existing cases ], })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/eslint-plugin-query/src/__tests__/prefer-query-options.test.ts` around lines 7 - 32, Add tests exercising other hooks from the rule's useQueryHooks list so the matching logic is covered beyond useQuery: update the ruleTester.run call in prefer-query-options.test.ts to include valid and invalid cases that mirror the existing useQuery examples but using at least one other hook name such as useInfiniteQuery (and optionally useSuspenseQuery, useSuspenseInfiniteQuery, useQueries, useSuspenseQueries) — e.g., add entries like `{ code: \`useInfiniteQuery(usersQuery)\` }` to valid and `{ code: \`useInfiniteQuery({ queryFn: () => {} })\`, errors: [{ messageId: 'no-inline-query-hook' }] }` to invalid so the rule's hook-matching logic is verified across the hook list.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/eslint-plugin-query/src/__tests__/prefer-query-options.test.ts`:
- Around line 1-4: The test is missing RuleTester static properties setup
required by Vitest; add the three assignments RuleTester.afterAll = afterAll,
RuleTester.describe = describe, and RuleTester.it = it immediately after the
imports and before creating the new RuleTester() instance so the RuleTester
static hooks are wired up for the prefer-query-options tests (look for
RuleTester and the instantiation new RuleTester()).
In
`@packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.rule.ts`:
- Around line 32-40: The helper isInvalidateQueriesCallExpression currently only
matches when the callee object is an Identifier named "queryClient"; update it
to detect any MemberExpression whose property is an Identifier named
"invalidateQueries" (keep the checks that callee.type ===
AST_NODE_TYPES.MemberExpression and callee.property.type ===
AST_NODE_TYPES.Identifier) and remove the strict check on callee.object.name, so
calls like client.invalidateQueries(...) or qc.invalidateQueries(...) are
recognized; ensure you still return false for non-member calls.
- Around line 1-8: The import block has two lint issues: the inline `type`
specifier on `ExtraRuleDocs` and the import order; fix by changing the inline
`type` import to a top-level type import (use `import type { ExtraRuleDocs }
from '../../types'`) and reorder imports so that the local sibling import
`./prefer-query-options.utils` comes before the parent-level `../../types`;
ensure symbols like AST_NODE_TYPES, ESLintUtils, TSESTree, getDocsUrl, and
detectQueryOptionsInObject remain imported and unchanged.
- Around line 12-20: The rule lists useQueries and useSuspenseQueries in the
useQueryHooks array but detectQueryOptionsInObject only inspects top-level
properties of arguments[0], so the rule never detects options nested under the {
queries: [...] } array; update the rule in prefer-query-options.rule.ts by
either removing 'useQueries' and 'useSuspenseQueries' from useQueryHooks or
(preferred) add handling in the visitor that when the callee name is
'useQueries' or 'useSuspenseQueries' you locate arguments[0].properties.find(p
=> p.key.name === 'queries'), ensure its value is an ArrayExpression, iterate
its elements and call detectQueryOptionsInObject (or the function that checks
option objects) on each ArrayExpression element so nested query option objects
are analyzed the same way as single-query hooks.
In
`@packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.utils.ts`:
- Around line 1-2: The two imports from '@typescript-eslint/utils' are in the
wrong order: the type-only import TSESTree appears before the non-type import
AST_NODE_TYPES; swap them so the non-type import (AST_NODE_TYPES) comes first
and the type import (TSESTree) follows, or combine into a single import while
ensuring the non-type specifier precedes the type-only specifier to satisfy the
import/order rule.
- Around line 15-22: The function currently uses queryNode.properties.find(...)
which yields an ObjectLiteralElement or undefined; change that to
queryNode.properties.some(...) so detectQueryOptionsInObject returns a boolean
as documented. Update the predicate that checks property.type ===
AST_NODE_TYPES.Property && property.key.type === AST_NODE_TYPES.Identifier &&
MAIN_QUERY_PROPERTIES.includes(property.key.name) to be passed to .some(),
ensuring the function returns true/false and all call-sites receive a boolean.
---
Nitpick comments:
In `@packages/eslint-plugin-query/src/__tests__/prefer-query-options.test.ts`:
- Around line 7-32: Add tests exercising other hooks from the rule's
useQueryHooks list so the matching logic is covered beyond useQuery: update the
ruleTester.run call in prefer-query-options.test.ts to include valid and invalid
cases that mirror the existing useQuery examples but using at least one other
hook name such as useInfiniteQuery (and optionally useSuspenseQuery,
useSuspenseInfiniteQuery, useQueries, useSuspenseQueries) — e.g., add entries
like `{ code: \`useInfiniteQuery(usersQuery)\` }` to valid and `{ code:
\`useInfiniteQuery({ queryFn: () => {} })\`, errors: [{ messageId:
'no-inline-query-hook' }] }` to invalid so the rule's hook-matching logic is
verified across the hook list.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.changeset/perky-comics-dig.mdpackages/eslint-plugin-query/src/__tests__/prefer-query-options.test.tspackages/eslint-plugin-query/src/index.tspackages/eslint-plugin-query/src/rules.tspackages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.rule.tspackages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.utils.ts
packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.rule.ts
Outdated
Show resolved
Hide resolved
packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.rule.ts
Show resolved
Hide resolved
Comment on lines
+32
to
+40
| function isInvalidateQueriesCallExpression(node: TSESTree.CallExpression) { | ||
| return ( | ||
| node.callee.type === AST_NODE_TYPES.MemberExpression && | ||
| node.callee.object.type === AST_NODE_TYPES.Identifier && | ||
| node.callee.object.name === 'queryClient' && | ||
| node.callee.property.type === AST_NODE_TYPES.Identifier && | ||
| node.callee.property.name === 'invalidateQueries' | ||
| ) | ||
| } |
Contributor
There was a problem hiding this comment.
isInvalidateQueriesCallExpression hardcodes the receiver to the variable name 'queryClient'.
This will silently miss every real-world pattern where the QueryClient instance has any other name:
const client = useQueryClient()
client.invalidateQueries({ queryKey: ['users'] }) // ❌ not caught
const qc = new QueryClient()
qc.invalidateQueries({ queryKey: ['users'] }) // ❌ not caughtThe most conservative broadening that avoids type-information requirements is to match any MemberExpression whose property name is invalidateQueries, regardless of the object name. This does introduce a small risk of false positives from other libraries, but invalidateQueries is a sufficiently unique method name within the TanStack Query ecosystem.
🔧 Proposed fix
function isInvalidateQueriesCallExpression(node: TSESTree.CallExpression) {
return (
node.callee.type === AST_NODE_TYPES.MemberExpression &&
- node.callee.object.type === AST_NODE_TYPES.Identifier &&
- node.callee.object.name === 'queryClient' &&
node.callee.property.type === AST_NODE_TYPES.Identifier &&
node.callee.property.name === 'invalidateQueries'
)
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.rule.ts`
around lines 32 - 40, The helper isInvalidateQueriesCallExpression currently
only matches when the callee object is an Identifier named "queryClient"; update
it to detect any MemberExpression whose property is an Identifier named
"invalidateQueries" (keep the checks that callee.type ===
AST_NODE_TYPES.MemberExpression and callee.property.type ===
AST_NODE_TYPES.Identifier) and remove the strict check on callee.object.name, so
calls like client.invalidateQueries(...) or qc.invalidateQueries(...) are
recognized; ensure you still return false for non-member calls.
packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.utils.ts
Outdated
Show resolved
Hide resolved
packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.utils.ts
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.rule.ts (2)
9-17:useQueriesanduseSuspenseQuerieslisted but the rule won't fire for them.The first argument to these hooks is
{ queries: Array<QueryOptions> }, not a flat options object, sodetectQueryOptionsInObject(node.arguments[0])will never match.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.rule.ts` around lines 9 - 17, The rule lists useQueries and useSuspenseQueries but detectQueryOptionsInObject(node.arguments[0]) never matches because those hooks expect a single object with a queries array; update the detection logic in prefer-query-options.rule.ts so that when the callee name is 'useQueries' or 'useSuspenseQueries' you check if the first argument is an object with a 'queries' property and then iterate over its elements, invoking detectQueryOptionsInObject for each query element (instead of treating the whole arg as a flat options object); keep the existing detection for useQuery/useInfiniteQuery/etc., and ensure you reference the AST node for the 'queries' property and array elements when calling detectQueryOptionsInObject.
29-37:isInvalidateQueriesCallExpressionstill hardcodes the receiver name'queryClient'.Any real-world usage such as
client.invalidateQueries(...)orqc.invalidateQueries(...)will be silently missed.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.rule.ts` around lines 29 - 37, The helper isInvalidateQueriesCallExpression currently only matches when the member expression receiver is the identifier named 'queryClient', which misses calls like client.invalidateQueries or qc.invalidateQueries; update the function to stop hardcoding the receiver name and instead accept any Identifier (i.e., keep the checks that node.callee.type === AST_NODE_TYPES.MemberExpression and node.callee.object.type === AST_NODE_TYPES.Identifier but remove the check node.callee.object.name === 'queryClient'), and ensure you still validate node.callee.property.type === AST_NODE_TYPES.Identifier && node.callee.property.name === 'invalidateQueries' so all identifier receivers calling invalidateQueries are detected.
🧹 Nitpick comments (1)
packages/eslint-plugin-query/src/index.ts (1)
51-68:flat/recommendedStrictduplicates all rules fromflat/recommended.Extracting a shared
baseRulesconstant would avoid the two lists diverging silently on future changes.♻️ Proposed refactor
+const baseRules = { + '@tanstack/query/exhaustive-deps': 'error', + '@tanstack/query/no-rest-destructuring': 'warn', + '@tanstack/query/stable-query-client': 'error', + '@tanstack/query/no-unstable-deps': 'error', + '@tanstack/query/infinite-query-property-order': 'error', + '@tanstack/query/no-void-query-fn': 'error', + '@tanstack/query/mutation-property-order': 'error', +} as const 'flat/recommended': [ { name: 'tanstack/query/flat/recommended', plugins: { '@tanstack/query': {} }, - rules: { - '@tanstack/query/exhaustive-deps': 'error', - '@tanstack/query/no-rest-destructuring': 'warn', - '@tanstack/query/stable-query-client': 'error', - '@tanstack/query/no-unstable-deps': 'error', - '@tanstack/query/infinite-query-property-order': 'error', - '@tanstack/query/no-void-query-fn': 'error', - '@tanstack/query/mutation-property-order': 'error', - }, + rules: { ...baseRules }, }, ], 'flat/recommendedStrict': [ { name: 'tanstack/query/flat/recommendedStrict', plugins: { '@tanstack/query': {} }, - rules: { - '@tanstack/query/exhaustive-deps': 'error', - '@tanstack/query/no-rest-destructuring': 'warn', - '@tanstack/query/stable-query-client': 'error', - '@tanstack/query/no-unstable-deps': 'error', - '@tanstack/query/infinite-query-property-order': 'error', - '@tanstack/query/no-void-query-fn': 'error', - '@tanstack/query/mutation-property-order': 'error', - '@tanstack/query/prefer-query-options': 'warn', - }, + rules: { ...baseRules, '@tanstack/query/prefer-query-options': 'warn' }, }, ],🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/eslint-plugin-query/src/index.ts` around lines 51 - 68, The two rule lists are duplicated; define a shared constant (e.g., baseRules) containing the common rules and then reference it in both configurations (replace the inline rules object in 'flat/recommended' and 'flat/recommendedStrict' with ...baseRules), keeping any strict-only differences merged on top of baseRules if needed; update the plugin config entries that currently list rules (the objects assigning '@tanstack/query/exhaustive-deps', '@tanstack/query/no-rest-destructuring', etc.) to use this shared baseRules constant so future changes stay in one place.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/eslint-plugin-query/src/index.ts`:
- Line 31: The flat/recommended config is missing the
'@tanstack/query/prefer-query-options' rule that exists in the legacy
recommended config, which will cause users to lose coverage when migrating;
update the exported flat/recommended configuration in
packages/eslint-plugin-query/src/index.ts to include
'@tanstack/query/prefer-query-options': 'warn' (matching the legacy recommended
setting) and mirror the same addition for the other rule entries noted around
lines 34-50 so both configs remain functionally equivalent for migrating users;
alternatively, if you prefer documentation, add a clear note that
flat/recommendedStrict is the functional equivalent of legacy recommended, but
the preferred fix is to add the rule to flat/recommended.
- Around line 51-68: The flat/recommendedStrict config leaves
plugins['@tanstack/query'] as an empty object so ESLint can't find the rules;
mirror the same wiring applied to the 'flat/recommended' config by assigning the
actual plugin object to the plugins['@tanstack/query'] slot for the
'flat/recommendedStrict' entry (i.e., set the plugins['@tanstack/query']
reference to the same plugin object you register for 'flat/recommended' so the
rules listed in 'flat/recommendedStrict' resolve).
---
Duplicate comments:
In
`@packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.rule.ts`:
- Around line 9-17: The rule lists useQueries and useSuspenseQueries but
detectQueryOptionsInObject(node.arguments[0]) never matches because those hooks
expect a single object with a queries array; update the detection logic in
prefer-query-options.rule.ts so that when the callee name is 'useQueries' or
'useSuspenseQueries' you check if the first argument is an object with a
'queries' property and then iterate over its elements, invoking
detectQueryOptionsInObject for each query element (instead of treating the whole
arg as a flat options object); keep the existing detection for
useQuery/useInfiniteQuery/etc., and ensure you reference the AST node for the
'queries' property and array elements when calling detectQueryOptionsInObject.
- Around line 29-37: The helper isInvalidateQueriesCallExpression currently only
matches when the member expression receiver is the identifier named
'queryClient', which misses calls like client.invalidateQueries or
qc.invalidateQueries; update the function to stop hardcoding the receiver name
and instead accept any Identifier (i.e., keep the checks that node.callee.type
=== AST_NODE_TYPES.MemberExpression and node.callee.object.type ===
AST_NODE_TYPES.Identifier but remove the check node.callee.object.name ===
'queryClient'), and ensure you still validate node.callee.property.type ===
AST_NODE_TYPES.Identifier && node.callee.property.name === 'invalidateQueries'
so all identifier receivers calling invalidateQueries are detected.
---
Nitpick comments:
In `@packages/eslint-plugin-query/src/index.ts`:
- Around line 51-68: The two rule lists are duplicated; define a shared constant
(e.g., baseRules) containing the common rules and then reference it in both
configurations (replace the inline rules object in 'flat/recommended' and
'flat/recommendedStrict' with ...baseRules), keeping any strict-only differences
merged on top of baseRules if needed; update the plugin config entries that
currently list rules (the objects assigning '@tanstack/query/exhaustive-deps',
'@tanstack/query/no-rest-destructuring', etc.) to use this shared baseRules
constant so future changes stay in one place.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/eslint-plugin-query/src/index.tspackages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.rule.tspackages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.utils.ts
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.rule.ts (1)
29-36:⚠️ Potential issue | 🟠 MajorDon’t hardcode the
queryClientidentifier.Matching only
queryClient.invalidateQueriesmisses common patterns where the QueryClient instance has another variable name (e.g.,client.invalidateQueries). This reduces the rule’s usefulness.🔧 Suggested fix
function isInvalidateQueriesCallExpression(node: TSESTree.CallExpression) { return ( node.callee.type === AST_NODE_TYPES.MemberExpression && - node.callee.object.type === AST_NODE_TYPES.Identifier && - node.callee.object.name === 'queryClient' && node.callee.property.type === AST_NODE_TYPES.Identifier && node.callee.property.name === 'invalidateQueries' ) }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.rule.ts` around lines 29 - 36, The helper isInvalidateQueriesCallExpression currently only matches when the object identifier is literally named "queryClient"; update it to match any object (not hardcoded name) while still ensuring the callee is a MemberExpression whose property identifier is "invalidateQueries". In practice, remove the check for callee.object.name === 'queryClient' and instead accept callee.object of type Identifier or MemberExpression (and support optional chaining if needed) and only assert callee.property.type === AST_NODE_TYPES.Identifier && callee.property.name === 'invalidateQueries' so calls like client.invalidateQueries(...) or some.nested.client.invalidateQueries(...) are detected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In
`@packages/eslint-plugin-query/src/rules/prefer-query-options/prefer-query-options.rule.ts`:
- Around line 29-36: The helper isInvalidateQueriesCallExpression currently only
matches when the object identifier is literally named "queryClient"; update it
to match any object (not hardcoded name) while still ensuring the callee is a
MemberExpression whose property identifier is "invalidateQueries". In practice,
remove the check for callee.object.name === 'queryClient' and instead accept
callee.object of type Identifier or MemberExpression (and support optional
chaining if needed) and only assert callee.property.type ===
AST_NODE_TYPES.Identifier && callee.property.name === 'invalidateQueries' so
calls like client.invalidateQueries(...) or
some.nested.client.invalidateQueries(...) are detected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🎯 Changes
#8515 (comment)
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Tests
Chore