feat(eslint-plugin): add allowlist option to exhaustive-deps rule#10295
Merged
feat(eslint-plugin): add allowlist option to exhaustive-deps rule#10295
Conversation
Introduce an `allowlist` option with `variables` and `types` arrays so stable variables and types can be excluded from dependency enforcement. Also report member expression dependencies more granularly for call expressions (e.g. `a.b.foo()` suggests `a.b` instead of only `a`). BREAKING CHANGE: exhaustive-deps now reports member expression deps more granularly, so some previously passing code may now report missing deps. Use the allowlist option to exclude stable variables/types as needed.
Contributor
📝 WalkthroughWalkthroughAdds an Changes
Sequence DiagramsequenceDiagram
actor ESLint as ESLint Engine
participant Rule as exhaustive-deps Rule
participant Utils as ExhaustiveDepsUtils
participant TypeAnalyzer as Type Analysis
ESLint->>Rule: Visit query options object
Rule->>Utils: collectQueryKeyDeps(queryKeyNode)
Utils-->>Rule: { roots, paths }
Rule->>Utils: getQueryFnNodes(queryFn)
Utils-->>Rule: queryFnNodes[]
loop For each queryFn node
Rule->>Utils: find relevant references within node
Utils-->>Rule: requiredRefs[] (identifier, path, root)
end
loop For each requiredRef
Rule->>TypeAnalyzer: variableIsAllowlistedByType(variable)
TypeAnalyzer-->>Utils: allowlistedByType info
Utils-->>Rule: annotated requiredRef
end
Rule->>Utils: computeFilteredMissingPaths(requiredRefs, allowlistedVariables, roots, paths)
Utils-->>Rule: missingPaths[]
alt missingPaths non-empty
Rule->>ESLint: Report violations + suggestions/fixes
else
Rule->>ESLint: No violations
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 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 docstrings
🧪 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 Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
|
View your CI Pipeline Execution ↗ for commit 7382e2a
☁️ Nx Cloud last updated this comment at |
Contributor
🚀 Changeset Version Preview1 package(s) bumped directly, 23 bumped as dependents. 🟨 Minor bumps
🟩 Patch bumps
|
4 tasks
Contributor
size-limit report 📦
|
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.changeset/exhaustive-deps-allowlist.md:
- Around line 2-5: The changeset currently marks '@tanstack/eslint-plugin-query'
as a minor release but introduces a breaking behavioral change for the
`exhaustive-deps` rule; update the changeset to a major release by changing the
release type for '@tanstack/eslint-plugin-query' from "minor" to "major" and
keep the description about the `exhaustive-deps` rule and its new `allowlist`
option (`variables` and `types`) intact so the package is versioned
appropriately.
In `@docs/eslint/exhaustive-deps.md`:
- Around line 39-45: Move the note about { allowlist: { variables: ["todos"] } }
above the very first createTodos() example and explicitly state that the first
"correct" snippet (where todoQueries.detail closes over todos) requires that
allowlist to be enabled; alternatively, if you want the snippet to be valid
without the allowlist, update todoQueries.detail so its queryKey includes the
closed-over variable (e.g., include todos in the key) — reference createTodos,
todoQueries.detail, and Component when making the clarification or code change.
In `@examples/react/eslint-plugin-demo/package.json`:
- Around line 8-16: The package.json currently pins TanStack packages with
semver ranges, so tests may install published releases instead of the workspace
packages; update the dependency entries for "@tanstack/react-query" and
"@tanstack/eslint-plugin-query" in package.json to use workspace refs (e.g.,
"workspace:*" or an appropriate workspace:... spec) so the test:eslint script
runs against the in-repo builds from this PR branch rather than npm releases;
ensure the updated entries are in the "dependencies" and "devDependencies"
sections respectively and run pnpm install to lock the workspace resolution.
In
`@packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.rule.ts`:
- Around line 152-168: The array-suggestion builder currently composes
fixedQueryKeyValue by string replacement (using fixedQueryKeyValue,
queryKeyValue, missingAsText and queryKeyNode) which breaks when arrays have
spaces, trailing commas or multiline formatting; replace this string-based
method with a token-based insertion: use context.sourceCode to locate the
array's closing bracket token (e.g. sourceCode.getLastToken(queryKeyNode) or
getTokenBefore/After) and then produce a suggestion that calls the fixer to
insert the dependency text right before that closing bracket (using
fixer.insertTextBefore/insertTextBeforeRange or similar) so you correctly handle
`[ ]`, trailing commas, and multiline arrays instead of using fixer.replaceText;
update the suggestions push logic for queryKeyNode.type ===
AST_NODE_TYPES.ArrayExpression to use this token-based fixer insertion.
In
`@packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts`:
- Around line 84-96: The loop currently adds every missing path (variable
`missingPaths`) including deeper members like "api.part" even when the root
"api" is missing; change it to collapse descendants per root by tracking the
chosen shortest missing path for each `root` (e.g., maintain a map or set keyed
by `root`) so that if the root itself is missing you skip deeper members, and if
you add a shorter path for a `root` you remove/replace any previously recorded
longer paths for that same `root`; use the existing identifiers (`requiredRefs`,
`root`, `path`, `existingRootIdentifiers`, `allowlistedVariables`,
`existingFullPaths`, `allowlistedByType`, `missingPaths`) to locate and
implement this behavior.
- Around line 412-425: The helper getQueryFnFunctionExpression currently returns
only the consequent for ConditionalExpression branches (unless the consequent is
skipToken), which misses scanning the alternate; change the logic so that when
queryFn.value is a ConditionalExpression and neither branch is the skipToken
identifier, you return the entire ConditionalExpression (queryFn.value) instead
of always returning the consequent; keep the existing special-case that returns
alternate when consequent is skipToken and symmetrically return consequent when
alternate is skipToken, but otherwise return queryFn.value so both branches get
scanned.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 99a8602d-e3e9-4243-948d-6d6bf5dc99f5
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
.changeset/exhaustive-deps-allowlist.mddocs/eslint/exhaustive-deps.mdexamples/react/eslint-plugin-demo/eslint.config.jsexamples/react/eslint-plugin-demo/package.jsonexamples/react/eslint-plugin-demo/src/allowlist-demo.tsxexamples/react/eslint-plugin-demo/tsconfig.jsonpackages/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
packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.rule.ts
Outdated
Show resolved
Hide resolved
packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts
Show resolved
Hide resolved
packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts
Outdated
Show resolved
Hide resolved
…nStack/query into feat-eslint-exhaustive-deps-allowlist
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts (1)
385-411: Consider handlingTSParenthesizedTypefor completeness.The current implementation handles common type patterns (references, unions, intersections, arrays, tuples). For full coverage, you might also want to handle
TSParenthesizedTypewhich wraps a type in parentheses.♻️ Optional enhancement
case AST_NODE_TYPES.TSTupleType: { typeNode.elementTypes.forEach((et) => ExhaustiveDepsUtils.collectTypeIdentifiers(et, out), ) break } + case AST_NODE_TYPES.TSParenthesizedType: { + ExhaustiveDepsUtils.collectTypeIdentifiers(typeNode.typeAnnotation, out) + break + } }🤖 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` around lines 385 - 411, The collectTypeIdentifiers method in ExhaustiveDepsUtils misses TSParenthesizedType nodes; add a case for AST_NODE_TYPES.TSParenthesizedType and recursively call ExhaustiveDepsUtils.collectTypeIdentifiers on the wrapped inner type (use the node's inner property, e.g., typeAnnotation or type, whichever exists) so parenthesized types are traversed like unions/arrays/tuples.
🤖 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`:
- Around line 385-411: The collectTypeIdentifiers method in ExhaustiveDepsUtils
misses TSParenthesizedType nodes; add a case for
AST_NODE_TYPES.TSParenthesizedType and recursively call
ExhaustiveDepsUtils.collectTypeIdentifiers on the wrapped inner type (use the
node's inner property, e.g., typeAnnotation or type, whichever exists) so
parenthesized types are traversed like unions/arrays/tuples.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: dfea3476-2ba0-4e62-96a8-5da983c61dc0
📒 Files selected for processing (4)
docs/eslint/exhaustive-deps.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
✅ Files skipped from review due to trivial changes (1)
- packages/eslint-plugin-query/src/tests/exhaustive-deps.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- docs/eslint/exhaustive-deps.md
TkDodo
approved these changes
Mar 22, 2026
TkDodo
requested changes
Mar 22, 2026
Collaborator
TkDodo
left a comment
There was a problem hiding this comment.
seeing some legit results here when running against the sentry codebase, but also some false positives:
getsentry/sentry/static/app/utils/queryClient.tsx
249:27 error The following dependencies are missing in your queryKey: TResponseData @tanstack/query/exhaustive-deps
this fails because we have annotated the return type of the queryFn - I think types should never be reported?
getsentry/sentry/static/app/components/forms/fields/choiceMapperField.tsx
171:39 error The following dependencies are missing in your queryKey: api, buildQueryParams @tanstack/query/exhaustive-deps
not sure about buildQueryParams - we use the exact same function call in the queryKey, maybe it needs to be extracted to a const?
Collaborator
Author
|
Oh wow, nice way of catching false positives! (addressed in 7382e2a) will definitely re-run on this repo next time. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/exhaustive-deps/exhaustive-deps.utils.ts`:
- Around line 418-437: getQueryFnNodes currently only unwraps one
ConditionalExpression and returns nested ternaries as nodes, which leads
getExternalRefs to call scopeManager.acquire() on ConditionalExpression (no
scope) and miss dependencies; update getQueryFnNodes to recursively flatten
nested ConditionalExpression branches: traverse each consequent/alternate until
reaching non-ConditionalExpression leaves and collect those leaf nodes
(functions/identifiers) into the returned array so getExternalRefs receives real
nodes it can acquire scope for and compute dependencies correctly.
- Around line 249-270: variableIsAllowlistedByType currently relies on
collectTypeIdentifiers but collectTypeIdentifiers only handles simple
identifiers, unions, intersections, arrays and tuples; update
collectTypeIdentifiers to recursively handle additional TS type node shapes so
allowlist detection works for idiomatic TypeScript. Specifically, add cases to
traverse TSTypeReference nodes with TSQualifiedName (extract both left/right
idents), handle TSTypeParameterInstantiation / typeParameters / typeArguments to
recurse into generic type arguments, support TSAsExpression and
TSParenthesizedType by recursing into their inner type/expression type nodes,
and include other nested forms like TSIndexedAccessType and TSMappedType
(recursing into object/index/key types) so variableIsAllowlistedByType (which
calls collectTypeIdentifiers) will pick up qualified/generic/compound type
identifiers such as Api.StableClient, Readonly<StableApi>, and
asserted/satisfies types.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 316ce08d-1d3c-4495-a862-f0acb9246a84
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.tspackages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/eslint-plugin-query/src/tests/exhaustive-deps.test.ts
Comment on lines
+249
to
+270
| variableIsAllowlistedByType(params: { | ||
| allowlistedTypes: Set<string> | ||
| variable: TSESLint.Scope.Variable | null | ||
| }): boolean { | ||
| const { allowlistedTypes, variable } = params | ||
| if (allowlistedTypes.size === 0) return false | ||
| if (!variable) return false | ||
|
|
||
| for (const id of variable.identifiers) { | ||
| if (id.typeAnnotation) { | ||
| const typeIdentifiers = new Set<string>() | ||
| ExhaustiveDepsUtils.collectTypeIdentifiers( | ||
| id.typeAnnotation.typeAnnotation, | ||
| typeIdentifiers, | ||
| ) | ||
| for (const typeIdentifier of typeIdentifiers) { | ||
| if (allowlistedTypes.has(typeIdentifier)) return true | ||
| } | ||
| } | ||
| } | ||
|
|
||
| return false |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's get the full file to understand the context
wc -l packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.tsRepository: TanStack/query
Length of output: 141
🏁 Script executed:
# Read the variableIsAllowlistedByType method and surrounding context
sed -n '240,280p' packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.tsRepository: TanStack/query
Length of output: 1170
🏁 Script executed:
# Read the collectTypeIdentifiers method (lines 386-412)
sed -n '380,420p' packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.tsRepository: TanStack/query
Length of output: 1382
🏁 Script executed:
# Also check for any type annotation extraction logic in the file
rg "typeAnnotation|as |satisfies|destructuring" packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts -nRepository: TanStack/query
Length of output: 295
🏁 Script executed:
# Verify if the code actually examines type parameters or qualified names
ast-grep --pattern 'case AST_NODE_TYPES.TSTypeReference: {
$$$
}'Repository: TanStack/query
Length of output: 40
🏁 Script executed:
# Check if there's any handling for TSQualifiedName elsewhere in the file
rg "TSQualifiedName|typeParameters" packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.tsRepository: TanStack/query
Length of output: 40
🏁 Script executed:
# Look for where allowlist.types is actually used to understand impact
rg "allowlist\\.types|allowlistedTypes" packages/eslint-plugin-query/src/rules/exhaustive-deps/ -A 2 -B 2Repository: TanStack/query
Length of output: 2804
🏁 Script executed:
# Let's examine the specific part of collectTypeIdentifiers that handles TSTypeReference
sed -n '386,405p' packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.tsRepository: TanStack/query
Length of output: 754
🏁 Script executed:
# Check if typeParameters or any qualified name handling exists in the implementation
sed -n '386,420p' packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts | cat -nRepository: TanStack/query
Length of output: 1519
🏁 Script executed:
# Verify what happens with qualified names - check the full switch statement
sed -n '389,415p' packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.tsRepository: TanStack/query
Length of output: 858
🏁 Script executed:
# Check if there's any handling for typeParameters in the codebase
rg "typeParameters" packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts -nRepository: TanStack/query
Length of output: 40
🏁 Script executed:
# Let's verify what AST_NODE_TYPES are available for qualified names
rg "TSQualifiedName|Qualified" packages/eslint-plugin-query/src/rules/exhaustive-deps/ -nRepository: TanStack/query
Length of output: 40
🏁 Script executed:
# Check the TypeScript ESTree types to understand the structure of generic types
# Look for how Readonly<Type> or similar generics would be structured
sed -n '1,50p' packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts | head -30Repository: TanStack/query
Length of output: 1029
🏁 Script executed:
# Let's check the test files to understand the intended scope of allowlist.types
find packages/eslint-plugin-query -name "*.test.ts" -o -name "*.spec.ts" | head -5Repository: TanStack/query
Length of output: 431
🏁 Script executed:
# Search for test cases related to allowlist
rg "allowlist.*types|allowlistedTypes" packages/eslint-plugin-query --type ts -A 3 -B 1 | head -60Repository: TanStack/query
Length of output: 6036
🏁 Script executed:
# Check the PR or recent commits to understand what was intended for this feature
git log --oneline -20 -- packages/eslint-plugin-query/src/rules/exhaustive-deps/Repository: TanStack/query
Length of output: 90
🏁 Script executed:
# Let's look at more comprehensive allowlist.types tests
sed -n '1,100p' packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts | grep -A 50 "allowlist.types"Repository: TanStack/query
Length of output: 40
🏁 Script executed:
# Search for tests with generics, qualified names, or type assertions
rg "Readonly|namespace|Namespace|\bas\b.*Type|satisfies" packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts -C 3Repository: TanStack/query
Length of output: 40
🏁 Script executed:
# Let's see the complete test cases for allowlist.types to understand scope
sed -n '1000,1200p' packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts | grep -A 30 "allowlist.types"Repository: TanStack/query
Length of output: 40
🏁 Script executed:
# Find the line number where allowlist.types tests are defined
rg -n "allowlist.types" packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.tsRepository: TanStack/query
Length of output: 303
🏁 Script executed:
# Get the test file size and examine the allowlist.types test section
wc -l packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.tsRepository: TanStack/query
Length of output: 129
🏁 Script executed:
# Extract the allowlist.types test cases
sed -n '2350,2550p' packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.tsRepository: TanStack/query
Length of output: 40
🏁 Script executed:
# Extract the allowlist.types test section from line 1926 onwards
sed -n '1926,2100p' packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.tsRepository: TanStack/query
Length of output: 5341
🏁 Script executed:
# Let's check if there are any test cases with generics or qualified names we might have missed
grep -n "Readonly\|Record\|namespace\|\bas\s\|satisfies" packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.tsRepository: TanStack/query
Length of output: 2196
🏁 Script executed:
# Check the PR or documentation for the allowlist.types feature
rg -i "allowlist.*types|generic|qualified.*name" packages/eslint-plugin-query --type mdRepository: TanStack/query
Length of output: 40
🏁 Script executed:
# Let's verify the current behavior with a qualified name example by checking TypeScript ESTree AST types
# Search for any documentation or references about TSQualifiedName handling
rg "TSQualifiedName" packages/eslint-plugin-query/srcRepository: TanStack/query
Length of output: 40
allowlist.types misses common explicit TypeScript type forms.
The collectTypeIdentifiers function only handles simple identifier types (TSTypeReference with plain Identifiers), unions, intersections, arrays, and tuples. Qualified names (e.g., const api: Api.StableClient), generic type parameters (e.g., Readonly<StableApi> will extract only "Readonly" but not the inner type), and type assertions (as StableApi) or satisfies expressions are not extracted. This causes the allowlist to silently fail on idiomatic TypeScript code. Broaden the type node handling to recursively extract identifiers from qualified names, generic parameters, and other complex type forms.
🤖 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`
around lines 249 - 270, variableIsAllowlistedByType currently relies on
collectTypeIdentifiers but collectTypeIdentifiers only handles simple
identifiers, unions, intersections, arrays and tuples; update
collectTypeIdentifiers to recursively handle additional TS type node shapes so
allowlist detection works for idiomatic TypeScript. Specifically, add cases to
traverse TSTypeReference nodes with TSQualifiedName (extract both left/right
idents), handle TSTypeParameterInstantiation / typeParameters / typeArguments to
recurse into generic type arguments, support TSAsExpression and
TSParenthesizedType by recursing into their inner type/expression type nodes,
and include other nested forms like TSIndexedAccessType and TSMappedType
(recursing into object/index/key types) so variableIsAllowlistedByType (which
calls collectTypeIdentifiers) will pick up qualified/generic/compound type
identifiers such as Api.StableClient, Readonly<StableApi>, and
asserted/satisfies types.
Comment on lines
+418
to
+437
| getQueryFnNodes(queryFn: TSESTree.Property): Array<TSESTree.Node> { | ||
| if (queryFn.value.type !== AST_NODE_TYPES.ConditionalExpression) { | ||
| return [queryFn.value] | ||
| } | ||
|
|
||
| if ( | ||
| queryFn.value.consequent.type === AST_NODE_TYPES.Identifier && | ||
| queryFn.value.consequent.name === 'skipToken' | ||
| ) { | ||
| return [queryFn.value.alternate] | ||
| } | ||
|
|
||
| if ( | ||
| queryFn.value.alternate.type === AST_NODE_TYPES.Identifier && | ||
| queryFn.value.alternate.name === 'skipToken' | ||
| ) { | ||
| return [queryFn.value.consequent] | ||
| } | ||
|
|
||
| return [queryFn.value.consequent, queryFn.value.alternate] |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd packages/eslint-plugin-query && find . -name "*.ts" -o -name "*.tsx" | head -20Repository: TanStack/query
Length of output: 981
🏁 Script executed:
cat packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts | wc -lRepository: TanStack/query
Length of output: 61
🏁 Script executed:
sed -n '418,437p' packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.tsRepository: TanStack/query
Length of output: 664
🏁 Script executed:
rg -n "isRelevantReference" packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts -A 10Repository: TanStack/query
Length of output: 499
🏁 Script executed:
rg -n "scopeManager.acquire" packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts -B 2 -A 5Repository: TanStack/query
Length of output: 621
🏁 Script executed:
find packages/eslint-plugin-query -name "*.test.*" -o -name "*.spec.*" | grep -i exhaustiveRepository: TanStack/query
Length of output: 124
🏁 Script executed:
sed -n '6,30p' packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.tsRepository: TanStack/query
Length of output: 817
🏁 Script executed:
grep -n "ConditionalExpression" packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.tsRepository: TanStack/query
Length of output: 132
🏁 Script executed:
grep -n "nested\|ternary" packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts -iRepository: TanStack/query
Length of output: 787
🏁 Script executed:
rg -n "skipToken" packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts -B 5 -A 5Repository: TanStack/query
Length of output: 2618
🏁 Script executed:
grep -n "? .* ?" packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts | head -20Repository: TanStack/query
Length of output: 40
🏁 Script executed:
grep -n "consequent.*ConditionalExpression\|alternate.*ConditionalExpression" packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts -iRepository: TanStack/query
Length of output: 40
🏁 Script executed:
sed -n '513,525p' packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.tsRepository: TanStack/query
Length of output: 451
🏁 Script executed:
sed -n '1,50p' packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.tsRepository: TanStack/query
Length of output: 1495
🏁 Script executed:
sed -n '880,910p' packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.tsRepository: TanStack/query
Length of output: 839
🏁 Script executed:
grep -n "should pass when queryFn is ternary with both branches having deps" packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts -A 20Repository: TanStack/query
Length of output: 764
🏁 Script executed:
python3 << 'PYTHON_EOF'
# Check if there are any nested ternaries in the test file
with open('packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts', 'r') as f:
content = f.read()
# Look for patterns like "? ... ? ... : ... :" which indicate nested ternaries
import re
nested_patterns = re.findall(r'queryFn:\s*(?:condition|[a-z]+)\s*\?\s*(?:(?:[^?:]*\?[^:]*:[^?]*)|[^:]*)\s*:\s*', content)
if nested_patterns:
print("Found potential nested ternaries:")
for p in nested_patterns[:5]:
print(f" {p[:80]}...")
else:
print("No nested ternary patterns found in tests")
# Also check the actual getQueryFnNodes implementation behavior
print("\nAnalyzing getQueryFnNodes logic:")
print("1. If not ConditionalExpression: return [node]")
print("2. If consequent is skipToken: return [alternate]")
print("3. If alternate is skipToken: return [consequent]")
print("4. Otherwise: return [consequent, alternate]")
print("\nIf alternate or consequent is another ConditionalExpression:")
print(" - It will be returned as-is from getQueryFnNodes")
print(" - Then isRelevantReference will call scopeManager.acquire() on it")
PYTHON_EOFRepository: TanStack/query
Length of output: 723
🏁 Script executed:
rg "ConditionalExpression" packages/eslint-plugin-query/src -A 5 -B 5 --type tsRepository: TanStack/query
Length of output: 2585
🏁 Script executed:
# Look specifically for how the nodes returned from getQueryFnNodes are used
rg "getQueryFnNodes" packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts -A 10 -B 2Repository: TanStack/query
Length of output: 509
🏁 Script executed:
rg "getQueryFnNodes" packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.rule.ts -B 5 -A 10Repository: TanStack/query
Length of output: 540
🏁 Script executed:
sed -n '140,180p' packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.rule.tsRepository: TanStack/query
Length of output: 1290
🏁 Script executed:
# Search for where the returned nodes from getQueryFnNodes are iterated
rg "getQueryFnNodes\|queryFnNodes" packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.rule.ts -B 3 -A 8Repository: TanStack/query
Length of output: 40
🏁 Script executed:
rg "getExternalRefs" packages/eslint-plugin-query/src/utils/ast-utils.ts -B 3 -A 15Repository: TanStack/query
Length of output: 620
🏁 Script executed:
find packages/eslint-plugin-query/src -name "*.ts" -exec grep -l "getExternalRefs" {} \;Repository: TanStack/query
Length of output: 188
🏁 Script executed:
# Let's look for ASTUtils file
find packages/eslint-plugin-query -name "ast-utils.ts"Repository: TanStack/query
Length of output: 109
🏁 Script executed:
sed -n '1,50p' packages/eslint-plugin-query/src/utils/ast-utils.tsRepository: TanStack/query
Length of output: 1807
🏁 Script executed:
rg "getExternalRefs" packages/eslint-plugin-query/src/utils/ast-utils.ts -A 30Repository: TanStack/query
Length of output: 1019
🏁 Script executed:
# Create a test to verify the issue exists
python3 << 'PYTHON_EOF'
# Let's trace through the logic flow:
print("=== FLOW ANALYSIS ===\n")
print("1. getQueryFnNodes returns nodes from ConditionalExpression branches")
print(" - If both branches are functions: returns [consequent, alternate]")
print(" - If one is skipToken: returns the other branch")
print(" - If neither branch is ConditionalExpression: works fine")
print(" - If a branch IS ConditionalExpression: returns that ConditionalExpression\n")
print("2. For each returned node, getExternalRefs is called:")
print(" - Calls scopeManager.acquire(node)")
print(" - ConditionalExpression nodes don't have their own scope")
print(" - So scopeManager.acquire(ConditionalExpression) returns null\n")
print("3. If scope is null, getExternalRefs returns []:")
print(" - No references are collected")
print(" - Dependencies inside that ConditionalExpression are MISSED\n")
print("=== EXAMPLE CASE ===")
print("queryFn: cond1 ? (cond2 ? () => fetchA(a) : () => fetchB(b)) : () => fetchC(c)")
print("")
print("Step 1: getQueryFnNodes receives cond1 ConditionalExpression")
print(" Returns [cond2 ConditionalExpression, () => fetchC(c) ArrowFunction]")
print("")
print("Step 2: getExternalRefs called on cond2 ConditionalExpression")
print(" scopeManager.acquire(cond2 ConditionalExpression) returns null")
print(" Returns [] <-- MISSES dependencies 'a' and 'b'")
print("")
print("Step 3: getExternalRefs called on () => fetchC(c) ArrowFunction")
print(" Returns references correctly")
print("")
print("Result: Dependencies 'a' and 'b' are NOT detected")
PYTHON_EOFRepository: TanStack/query
Length of output: 1354
🏁 Script executed:
# Check if there are any test cases with nested ternaries
grep -n "cond.*?.*?.*:" packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts | head -5Repository: TanStack/query
Length of output: 40
🏁 Script executed:
# Final confirmation: check if ConditionalExpression has scope in ESLint
python3 << 'PYTHON_EOF'
# ESLint's scopeManager.acquire() returns scope for nodes that create scope:
# - Functions (arrow, regular, async)
# - Blocks (if/for/while, etc.)
# - Class definitions
#
# ConditionalExpression (ternary) does NOT create a scope
# This is a standard JavaScript behavior - ternaries are expressions, not statements
print("ESLint Scope Manager Behavior:")
print("================================")
print("Nodes that CREATE scope:")
print(" - FunctionExpression, ArrowFunctionExpression")
print(" - FunctionDeclaration")
print(" - ClassDeclaration, ClassExpression")
print(" - BlockStatement, IfStatement, etc.")
print("")
print("Nodes that DON'T create scope:")
print(" - ConditionalExpression (ternary)")
print(" - BinaryExpression, LogicalExpression")
print(" - CallExpression, MemberExpression")
print("")
print("When scopeManager.acquire(ConditionalExpression) is called:")
print(" Returns: null")
print(" Result in getExternalRefs: returns []")
EOFRepository: TanStack/query
Length of output: 784
🏁 Script executed:
# Double-check by looking at actual test coverage
grep -c "ConditionalExpression" packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.tsRepository: TanStack/query
Length of output: 59
🏁 Script executed:
sed -n '160,175p' packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.rule.tsRepository: TanStack/query
Length of output: 363
🏁 Script executed:
# Verify the exact usage location where getQueryFnNodes is called
sed -n '145,165p' packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.rule.tsRepository: TanStack/query
Length of output: 697
🏁 Script executed:
# Check if there are any integration tests that might catch this
rg "nested.*ternary|ternary.*nested" packages/eslint-plugin-query/src/__tests__/ -iRepository: TanStack/query
Length of output: 40
Flatten nested queryFn ternaries to avoid missing dependencies.
getQueryFnNodes only unwraps one ConditionalExpression layer. If a branch is itself a ternary, it returns that ConditionalExpression node, and getExternalRefs calls scopeManager.acquire() on it—which returns null because ternaries don't create scopes. When scope is null, getExternalRefs returns an empty array, missing dependencies. A nested case like cond1 ? (cond2 ? () => fetchA(a) : () => fetchB(b)) : () => fetchC(c) will miss a and b.
♻️ Proposed fix
getQueryFnNodes(queryFn: TSESTree.Property): Array<TSESTree.Node> {
- if (queryFn.value.type !== AST_NODE_TYPES.ConditionalExpression) {
- return [queryFn.value]
- }
-
- if (
- queryFn.value.consequent.type === AST_NODE_TYPES.Identifier &&
- queryFn.value.consequent.name === 'skipToken'
- ) {
- return [queryFn.value.alternate]
- }
-
- if (
- queryFn.value.alternate.type === AST_NODE_TYPES.Identifier &&
- queryFn.value.alternate.name === 'skipToken'
- ) {
- return [queryFn.value.consequent]
- }
-
- return [queryFn.value.consequent, queryFn.value.alternate]
+ function collect(node: TSESTree.Node): Array<TSESTree.Node> {
+ if (node.type !== AST_NODE_TYPES.ConditionalExpression) {
+ return [node]
+ }
+
+ if (
+ node.consequent.type === AST_NODE_TYPES.Identifier &&
+ node.consequent.name === 'skipToken'
+ ) {
+ return collect(node.alternate)
+ }
+
+ if (
+ node.alternate.type === AST_NODE_TYPES.Identifier &&
+ node.alternate.name === 'skipToken'
+ ) {
+ return collect(node.consequent)
+ }
+
+ return [...collect(node.consequent), ...collect(node.alternate)]
+ }
+
+ return collect(queryFn.value)
},🤖 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`
around lines 418 - 437, getQueryFnNodes currently only unwraps one
ConditionalExpression and returns nested ternaries as nodes, which leads
getExternalRefs to call scopeManager.acquire() on ConditionalExpression (no
scope) and miss dependencies; update getQueryFnNodes to recursively flatten
nested ConditionalExpression branches: traverse each consequent/alternate until
reaching non-ConditionalExpression leaves and collect those leaf nodes
(functions/identifiers) into the returned array so getExternalRefs receives real
nodes it can acquire scope for and compute dependencies correctly.
Collaborator
|
looks good! all the other 16 errors I’m seeing are legit findings, so let’s ship it. |
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
2 participants
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.
Prior PR - #10258
Relevant issue - #6853
🎯 Changes
The lint rule
exhaustive-depsnow has anallowlistoption so stable variables and types can be excluded from dependency checks. It also reports member expression dependencies in a finer-grained way for call expressions.The
allowlistoption supports:allowlist.variables: variable names to ignore when checking dependenciesallowlist.types: TypeScript type names whose variables are ignored✅ Checklist
pnpm run test:pr.🚀 Release Impact
Summary by CodeRabbit
New Features
Documentation
Examples
Tests