build(eslint-plugin-query): add typescript-aware detection for custom query hooks#10376
Open
chinhkrb113 wants to merge 1 commit intoTanStack:mainfrom
Open
Conversation
…query hooks Extend the no-rest-destructuring rule to also report when rest destructuring is used on custom hooks (functions starting with "use") that return a TanStack Query result type. This requires the TypeScript type checker, so the logic is only executed when @typescript-eslint/parser provides type information. Affected files: noRestDestructuring.ts Signed-off-by: ChinhLee <76194645+chinhkrb113@users.noreply.github.com>
Contributor
📝 WalkthroughWalkthroughA new ESLint rule Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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/rules/noRestDestructuring.ts`:
- Around line 46-47: The current check uses typeName.includes(...) which causes
false positives; change the condition in the noRestDestructuring rule so it only
matches exact type names (e.g., replace queryResultTypes.some((t) =>
typeName.includes(t)) with queryResultTypes.some((t) => t === typeName) or use
queryResultTypes.includes(typeName)) so only exact matches of typeName trigger
the return true in that if branch.
- Around line 79-85: The hook-detection currently accepts call expressions named
'useQuery' or 'useInfiniteQuery' by name even when type information is
available; change the logic so that when parserServices?.hasTypeInformation is
true you always set isQueryHook by calling isTanstackQueryResult(init) (using
the existing isTanstackQueryResult function), and only fall back to the
name-based check (checking callee.type === 'Identifier' and callee.name in
['useQuery','useInfiniteQuery'] or startsWith('use')) when
parserServices?.hasTypeInformation is falsy; update the branch around callee,
parserServices, isQueryHook and isTanstackQueryResult accordingly.
- Around line 4-5: The exported symbol noRestDestructuring (created via
createRule) in this file appears to be unused because the plugin registry
imports the rule from ./rules/no-rest-destructuring/no-rest-destructuring.rule;
inspect the rules.ts registry and the subdirectory implementation to determine
if this file is a duplicate or part of an incomplete refactor, then either (a)
remove this stale export/file if the canonical implementation lives in
no-rest-destructuring/no-rest-destructuring.rule, or (b) update the registry
(rules.ts) to import and expose this noRestDestructuring export instead of the
subfolder version so there is a single source of truth; ensure the unique symbol
noRestDestructuring and createRule usage are removed or unified accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d30a09fc-c387-47fc-bb0b-14604512e2a0
📒 Files selected for processing (1)
packages/eslint-plugin-query/src/rules/noRestDestructuring.ts
Comment on lines
+4
to
+5
| export const noRestDestructuring = createRule<[], 'noRestDestructuring'>({ | ||
| name: 'no-rest-destructuring', |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Read-only verification of rule wiring and export usage
rg -n -C2 "noRestDestructuring|no-rest-destructuring" packages/eslint-plugin-query/src
fd -i "noRestDestructuring.ts" packages/eslint-plugin-query/src
fd -i "no-rest-destructuring.rule.ts" packages/eslint-plugin-query/srcRepository: TanStack/query
Length of output: 5834
The rule export in this file is not wired into plugin registry; the plugin imports from a different file structure.
The plugin's rules.ts imports the rule from ./rules/no-rest-destructuring/no-rest-destructuring.rule, not from this file. This means the noRestDestructuring export here is unused and the rule's implementation must be provided by the separate no-rest-destructuring/ subdirectory file. Verify whether this file is a duplicate, stale, or represents an incomplete refactor.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/eslint-plugin-query/src/rules/noRestDestructuring.ts` around lines 4
- 5, The exported symbol noRestDestructuring (created via createRule) in this
file appears to be unused because the plugin registry imports the rule from
./rules/no-rest-destructuring/no-rest-destructuring.rule; inspect the rules.ts
registry and the subdirectory implementation to determine if this file is a
duplicate or part of an incomplete refactor, then either (a) remove this stale
export/file if the canonical implementation lives in
no-rest-destructuring/no-rest-destructuring.rule, or (b) update the registry
(rules.ts) to import and expose this noRestDestructuring export instead of the
subfolder version so there is a single source of truth; ensure the unique symbol
noRestDestructuring and createRule usage are removed or unified accordingly.
Comment on lines
+46
to
+47
| if (queryResultTypes.some((t) => typeName.includes(t))) { | ||
| return true |
Contributor
There was a problem hiding this comment.
Use exact type-name matching to reduce false positives.
typeName.includes(...) can match unrelated symbols (e.g., wrapper/mock types). Exact match is safer here.
Proposed fix
- if (queryResultTypes.some((t) => typeName.includes(t))) {
+ if (queryResultTypes.includes(typeName)) {
return true
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (queryResultTypes.some((t) => typeName.includes(t))) { | |
| return true | |
| if (queryResultTypes.includes(typeName)) { | |
| return true | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/eslint-plugin-query/src/rules/noRestDestructuring.ts` around lines
46 - 47, The current check uses typeName.includes(...) which causes false
positives; change the condition in the noRestDestructuring rule so it only
matches exact type names (e.g., replace queryResultTypes.some((t) =>
typeName.includes(t)) with queryResultTypes.some((t) => t === typeName) or use
queryResultTypes.includes(typeName)) so only exact matches of typeName trigger
the return true in that if branch.
Comment on lines
+79
to
+85
| if (callee.type === 'Identifier' && callee.name.startsWith('use')) { | ||
| const name = callee.name | ||
| if (['useQuery', 'useInfiniteQuery'].includes(name)) { | ||
| isQueryHook = true | ||
| } else if (parserServices?.hasTypeInformation) { | ||
| isQueryHook = isTanstackQueryResult(init) | ||
| } |
Contributor
There was a problem hiding this comment.
Don’t short-circuit on hook name when type info is available.
On Line 81–83, useQuery/useInfiniteQuery are accepted by name only, which can report non-TanStack hooks with the same names. Prefer type-based gating whenever parser services are available.
Proposed fix
- if (callee.type === 'Identifier' && callee.name.startsWith('use')) {
- const name = callee.name
- if (['useQuery', 'useInfiniteQuery'].includes(name)) {
- isQueryHook = true
- } else if (parserServices?.hasTypeInformation) {
- isQueryHook = isTanstackQueryResult(init)
- }
- }
+ if (callee.type === 'Identifier' && callee.name.startsWith('use')) {
+ if (parserServices?.hasTypeInformation) {
+ isQueryHook = isTanstackQueryResult(init)
+ } else {
+ isQueryHook = ['useQuery', 'useInfiniteQuery'].includes(callee.name)
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/eslint-plugin-query/src/rules/noRestDestructuring.ts` around lines
79 - 85, The hook-detection currently accepts call expressions named 'useQuery'
or 'useInfiniteQuery' by name even when type information is available; change
the logic so that when parserServices?.hasTypeInformation is true you always set
isQueryHook by calling isTanstackQueryResult(init) (using the existing
isTanstackQueryResult function), and only fall back to the name-based check
(checking callee.type === 'Identifier' and callee.name in
['useQuery','useInfiniteQuery'] or startsWith('use')) when
parserServices?.hasTypeInformation is falsy; update the branch around callee,
parserServices, isQueryHook and isTanstackQueryResult accordingly.
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.
New Feature
Problem
Extend the no-rest-destructuring rule to also report when rest destructuring is used on custom hooks (functions starting with "use") that return a TanStack Query result type. This requires the TypeScript type checker, so the logic is only executed when @typescript-eslint/parser provides type information.
Severity:
highFile:
packages/eslint-plugin-query/src/rules/noRestDestructuring.tsSolution
Extend the no-rest-destructuring rule to also report when rest destructuring is used on custom hooks (functions starting with "use") that return a TanStack Query result type. This requires the TypeScript type checker, so the logic is only executed when @typescript-eslint/parser provides type information.
Changes
packages/eslint-plugin-query/src/rules/noRestDestructuring.ts(modified)🎯 Changes
✅ Checklist
pnpm run test:pr.🚀 Release Impact
Contributed by Lê Thành Chỉnh
Code is a tool. Mindset is the real value.
Closes #8951
Summary by CodeRabbit
no-rest-destructuringESLint rule to the query plugin that prevents problematic rest destructuring patterns when using TanStack Query hooks likeuseQueryanduseInfiniteQuery. The rule features intelligent TypeScript type-aware detection to identify various query result types, helping developers avoid common destructuring mistakes and maintain better code quality.