feat(eslint-plugin): add allowlist option to exhaustive-deps rule#10295
Open
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 queryOptions object
Rule->>Utils: collectQueryKeyDeps(queryKeyNode)
Utils-->>Rule: {roots, paths}
Rule->>Utils: getQueryFnNodes(queryFn)
Utils-->>Rule: queryFnNodes[]
Rule->>Utils: For each fnNode, find relevant references
Utils-->>Rule: requiredRefs[] (path, root, allowlistedByType?)
loop For each requiredRef
Rule->>TypeAnalyzer: Check variable type annotations
TypeAnalyzer->>Utils: variableIsAllowlistedByType(variable)
Utils-->>Rule: allowlistedByType boolean
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)
📝 Coding Plan
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 98868ec
☁️ Nx Cloud last updated this comment at |
Contributor
🚀 Changeset Version Preview1 package(s) bumped directly, 0 bumped as dependents. 🟨 Minor 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
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.
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
allowlistoption to the exhaustive-deps rule to ignore specific variables and types.Documentation
Examples
Tests