fix: allow useQueries with combine property in no-unstable-deps rule #9720
Conversation
- Add hasCombineProperty function to detect combine property in useQueries calls - Skip tracking useQueries with combine as unstable since combine makes result stable - Add tests for useQueries with and without combine property - Fixes TanStack#9718
|
WalkthroughAdds handling so Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer Code
participant ESLint as no-unstable-deps Rule
participant AST as AST Walker
Dev->>ESLint: Run lint on file
ESLint->>AST: Visit VariableDeclarator (init is CallExpression)
alt callee is useQueries with combine
note right of ESLint #a9d18e: Detect combine on first argument (treated stable)
ESLint-->>AST: Skip tracking variable
else callee is useQueries without combine
note right of ESLint #f4c7c3: No combine (treated unstable)
ESLint->>AST: Track variable as unstable
end
AST->>ESLint: Visit React Hook call with deps
alt deps reference tracked (unstable) variable
ESLint-->>Dev: Report noUnstableDeps diagnostic
else deps reference skipped/stable variable
ESLint-->>Dev: No diagnostic
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
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}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/eslint-plugin-query/src/rules/no-unstable-deps/no-unstable-deps.rule.ts (1)
70-81: Consider edge cases in combine property detection.The function correctly identifies standard
combineproperties, but may not handle all edge cases:
- Spread properties:
{ ...config, queries: [...] }won't be detected ifcombineis in the spread- Computed property names:
{ ['combine']: fn }won't match since you checkprop.key.type === AST_NODE_TYPES.Identifier- Property shorthand: While
{ combine }should work, the function assumes the property key name is what mattersFor robustness, consider adding a check for computed properties:
function hasCombineProperty(callExpression: TSESTree.CallExpression): boolean { if (callExpression.arguments.length === 0) return false const firstArg = callExpression.arguments[0] if (!firstArg || firstArg.type !== AST_NODE_TYPES.ObjectExpression) return false return firstArg.properties.some(prop => prop.type === AST_NODE_TYPES.Property && + !prop.computed && prop.key.type === AST_NODE_TYPES.Identifier && prop.key.name === 'combine' ) }This ensures computed properties like
{ ['combine']: fn }are excluded (though they're unlikely in practice).packages/eslint-plugin-query/src/__tests__/no-unstable-deps.test.ts (1)
110-134: Consider adding invalid test case foruseSuspenseQuerieswithout combine.The invalid test case correctly verifies that
useQuerieswithoutcombinetriggers the rule. For completeness and symmetry with the valid test cases, consider adding a corresponding invalid test foruseSuspenseQuerieswithoutcombine.Add this test case to the invalid array:
{ name: `result of useSuspenseQueries without combine is passed to ${reactHookInvocation} as dependency`, code: ` ${reactHookImport} import { useSuspenseQueries } from "@tanstack/react-query"; function Component() { const queries = useSuspenseQueries({ queries: [ { queryKey: ['test'], queryFn: () => 'test' } ] }); const callback = ${reactHookInvocation}(() => { queries[0]?.data }, [queries]); return; } `, errors: [ { messageId: 'noUnstableDeps', data: { reactHook: reactHookAlias, queryHook: 'useSuspenseQueries' }, }, ], },
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/eslint-plugin-query/src/__tests__/no-unstable-deps.test.ts(2 hunks)packages/eslint-plugin-query/src/rules/no-unstable-deps/no-unstable-deps.rule.ts(2 hunks)
packages/eslint-plugin-query/src/__tests__/no-unstable-deps.test.ts
Outdated
Show resolved
Hide resolved
packages/eslint-plugin-query/src/rules/no-unstable-deps/no-unstable-deps.rule.ts
Show resolved
Hide resolved
|
View your CI Pipeline Execution ↗ for commit 28e98fa
☁️ Nx Cloud last updated this comment at |
@tanstack/query/no-unstable-depsESLint rule with combine property #9718Changes
Checklist
pnpm test:pr.Release Impact
Summary by CodeRabbit
Bug Fixes
Tests