Skip to content

Commit f65ebe7

Browse files
schiller-manuelschiller-manuel
authoredAug 20, 2024··
feat(eslint-plugin-query): add rule that disallows putting the result of query hooks directly in a React hook dependency array (#7911)
* feat(eslint-plugin-query): add rule that disallows putting the result of useMutation directly in a React hook dependency array * add doc * add rule to recommended rules * generalize to all affected query hooks * rename rule to "no-unstable-deps"
·
v5.74.7v5.52.0
1 parent 683c85e commit f65ebe7

File tree

7 files changed

+325
-0
lines changed

7 files changed

+325
-0
lines changed
 

‎docs/config.json

+4
Original file line numberDiff line numberDiff line change
@@ -792,6 +792,10 @@
792792
{
793793
"label": "No Rest Destructuring",
794794
"to": "eslint/no-rest-destructuring"
795+
},
796+
{
797+
"label": "No Unstable Deps",
798+
"to": "eslint/no-unstable-deps"
795799
}
796800
]
797801
},

‎docs/eslint/eslint-plugin-query.md

+1
Original file line numberDiff line numberDiff line change
@@ -96,3 +96,4 @@ Alternatively, add `@tanstack/eslint-plugin-query` to the plugins section, and c
9696
- [@tanstack/query/exhaustive-deps](../exhaustive-deps)
9797
- [@tanstack/query/no-rest-destructuring](../no-rest-destructuring)
9898
- [@tanstack/query/stable-query-client](../stable-query-client)
99+
- [@tanstack/query/no-unstable-deps](../no-unstable-deps.md)

‎docs/eslint/no-unstable-deps.md

+56
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
---
2+
id: no-unstable-deps
3+
title: Disallow putting the result of query hooks directly in a React hook dependency array
4+
---
5+
6+
The object returned from the following query hooks is **not** referentially stable:
7+
8+
- `useQuery`
9+
- `useSuspenseQuery`
10+
- `useQueries`
11+
- `useSuspenseQueries`
12+
- `useInfiniteQuery`
13+
- `useSuspenseInfiniteQuery`
14+
- `useMutation`
15+
16+
The object returned from those hooks should **not** be put directly into the dependency array of a React hook (e.g. `useEffect`, `useMemo`, `useCallback`).
17+
Instead, destructure the return value of the query hook and pass the destructured values into the dependency array of the React hook.
18+
19+
## Rule Details
20+
21+
Examples of **incorrect** code for this rule:
22+
23+
```tsx
24+
/* eslint "@tanstack/query/no-unstable-deps": "warn" */
25+
import { useCallback } from 'React'
26+
import { useMutation } from '@tanstack/react-query'
27+
28+
function Component() {
29+
const mutation = useMutation({ mutationFn: (value: string) => value })
30+
const callback = useCallback(() => {
31+
mutation.mutate('hello')
32+
}, [mutation])
33+
return null
34+
}
35+
```
36+
37+
Examples of **correct** code for this rule:
38+
39+
```tsx
40+
/* eslint "@tanstack/query/no-unstable-deps": "warn" */
41+
import { useCallback } from 'React'
42+
import { useMutation } from '@tanstack/react-query'
43+
44+
function Component() {
45+
const { mutate } = useMutation({ mutationFn: (value: string) => value })
46+
const callback = useCallback(() => {
47+
mutate('hello')
48+
}, [mutate])
49+
return null
50+
}
51+
```
52+
53+
## Attributes
54+
55+
- [x] ✅ Recommended
56+
- [ ] 🔧 Fixable

‎packages/eslint-plugin-query/src/__tests__/no-unstable-deps.test.ts

+132
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
import { RuleTester } from '@typescript-eslint/rule-tester'
2+
import {
3+
reactHookNames,
4+
rule,
5+
useQueryHookNames,
6+
} from '../rules/no-unstable-deps/no-unstable-deps.rule'
7+
8+
const ruleTester = new RuleTester({
9+
parser: '@typescript-eslint/parser',
10+
settings: {},
11+
})
12+
13+
interface TestCase {
14+
reactHookImport: string
15+
reactHookInvocation: string
16+
reactHookAlias: string
17+
}
18+
const baseTestCases = {
19+
valid: ({ reactHookImport, reactHookInvocation, reactHookAlias }: TestCase) =>
20+
[
21+
{
22+
name: `should pass when destructured mutate is passed to ${reactHookAlias} as dependency`,
23+
code: `
24+
${reactHookImport}
25+
import { useMutation } from "@tanstack/react-query";
26+
27+
function Component() {
28+
const { mutate } = useMutation({ mutationFn: (value: string) => value });
29+
const callback = ${reactHookInvocation}(() => { mutate('hello') }, [mutate]);
30+
return;
31+
}
32+
`,
33+
},
34+
].concat(
35+
useQueryHookNames.map((queryHook) => ({
36+
name: `should pass result of ${queryHook} is passed to ${reactHookInvocation} as dependency`,
37+
code: `
38+
${reactHookImport}
39+
import { ${queryHook} } from "@tanstack/react-query";
40+
41+
function Component() {
42+
const { refetch } = ${queryHook}({ queryFn: (value: string) => value });
43+
const callback = ${reactHookInvocation}(() => { query.refetch() }, [refetch]);
44+
return;
45+
}
46+
`,
47+
})),
48+
),
49+
invalid: ({
50+
reactHookImport,
51+
reactHookInvocation,
52+
reactHookAlias,
53+
}: TestCase) =>
54+
[
55+
{
56+
name: `result of useMutation is passed to ${reactHookInvocation} as dependency `,
57+
code: `
58+
${reactHookImport}
59+
import { useMutation } from "@tanstack/react-query";
60+
61+
function Component() {
62+
const mutation = useMutation({ mutationFn: (value: string) => value });
63+
const callback = ${reactHookInvocation}(() => { mutation.mutate('hello') }, [mutation]);
64+
return;
65+
}
66+
`,
67+
errors: [
68+
{
69+
messageId: 'noUnstableDeps',
70+
data: { reactHook: reactHookAlias, queryHook: 'useMutation' },
71+
},
72+
],
73+
},
74+
].concat(
75+
useQueryHookNames.map((queryHook) => ({
76+
name: `result of ${queryHook} is passed to ${reactHookInvocation} as dependency`,
77+
code: `
78+
${reactHookImport}
79+
import { ${queryHook} } from "@tanstack/react-query";
80+
81+
function Component() {
82+
const query = ${queryHook}({ queryFn: (value: string) => value });
83+
const callback = ${reactHookInvocation}(() => { query.refetch() }, [query]);
84+
return;
85+
}
86+
`,
87+
errors: [
88+
{
89+
messageId: 'noUnstableDeps',
90+
data: { reactHook: reactHookAlias, queryHook },
91+
},
92+
],
93+
})),
94+
),
95+
}
96+
97+
const testCases = (reactHookName: string) => [
98+
{
99+
reactHookImport: 'import * as React from "React";',
100+
reactHookInvocation: `React.${reactHookName}`,
101+
reactHookAlias: reactHookName,
102+
},
103+
{
104+
reactHookImport: `import { ${reactHookName} } from "React";`,
105+
reactHookInvocation: reactHookName,
106+
reactHookAlias: reactHookName,
107+
},
108+
{
109+
reactHookImport: `import { ${reactHookName} as useAlias } from "React";`,
110+
reactHookInvocation: 'useAlias',
111+
reactHookAlias: 'useAlias',
112+
},
113+
]
114+
115+
reactHookNames.forEach((reactHookName) => {
116+
testCases(reactHookName).forEach(
117+
({ reactHookInvocation, reactHookAlias, reactHookImport }) => {
118+
ruleTester.run('no-unstable-deps', rule, {
119+
valid: baseTestCases.valid({
120+
reactHookImport,
121+
reactHookInvocation,
122+
reactHookAlias,
123+
}),
124+
invalid: baseTestCases.invalid({
125+
reactHookImport,
126+
reactHookInvocation,
127+
reactHookAlias,
128+
}),
129+
})
130+
},
131+
)
132+
})

‎packages/eslint-plugin-query/src/index.ts

+2
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ Object.assign(plugin.configs, {
2828
'@tanstack/query/exhaustive-deps': 'error',
2929
'@tanstack/query/no-rest-destructuring': 'warn',
3030
'@tanstack/query/stable-query-client': 'error',
31+
'@tanstack/query/no-unstable-deps': 'error',
3132
},
3233
},
3334
'flat/recommended': [
@@ -39,6 +40,7 @@ Object.assign(plugin.configs, {
3940
'@tanstack/query/exhaustive-deps': 'error',
4041
'@tanstack/query/no-rest-destructuring': 'warn',
4142
'@tanstack/query/stable-query-client': 'error',
43+
'@tanstack/query/no-unstable-deps': 'error',
4244
},
4345
},
4446
],

‎packages/eslint-plugin-query/src/rules.ts

+2
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import * as exhaustiveDeps from './rules/exhaustive-deps/exhaustive-deps.rule'
22
import * as stableQueryClient from './rules/stable-query-client/stable-query-client.rule'
33
import * as noRestDestructuring from './rules/no-rest-destructuring/no-rest-destructuring.rule'
4+
import * as noUnstableDeps from './rules/no-unstable-deps/no-unstable-deps.rule'
45
import type { ESLintUtils } from '@typescript-eslint/utils'
56
import type { ExtraRuleDocs } from './types'
67

@@ -16,4 +17,5 @@ export const rules: Record<
1617
[exhaustiveDeps.name]: exhaustiveDeps.rule,
1718
[stableQueryClient.name]: stableQueryClient.rule,
1819
[noRestDestructuring.name]: noRestDestructuring.rule,
20+
[noUnstableDeps.name]: noUnstableDeps.rule,
1921
}

‎packages/eslint-plugin-query/src/rules/no-unstable-deps/no-unstable-deps.rule.ts

+128
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,128 @@
1+
import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/utils'
2+
import { getDocsUrl } from '../../utils/get-docs-url'
3+
import { detectTanstackQueryImports } from '../../utils/detect-react-query-imports'
4+
import type { TSESTree } from '@typescript-eslint/utils'
5+
import type { ExtraRuleDocs } from '../../types'
6+
7+
export const name = 'no-unstable-deps'
8+
9+
export const reactHookNames = ['useEffect', 'useCallback', 'useMemo']
10+
export const useQueryHookNames = [
11+
'useQuery',
12+
'useSuspenseQuery',
13+
'useQueries',
14+
'useSuspenseQueries',
15+
'useInfiniteQuery',
16+
'useSuspenseInfiniteQuery',
17+
]
18+
const allHookNames = ['useMutation', ...useQueryHookNames]
19+
const createRule = ESLintUtils.RuleCreator<ExtraRuleDocs>(getDocsUrl)
20+
21+
export const rule = createRule({
22+
name,
23+
meta: {
24+
type: 'problem',
25+
docs: {
26+
description:
27+
'Disallow putting the result of useMutation directly in a React hook dependency array',
28+
recommended: 'error',
29+
},
30+
messages: {
31+
noUnstableDeps: `The result of {{queryHook}} is not referentially stable, so don't pass it directly into the dependencies array of {{reactHook}}. Instead, destructure the return value of {{queryHook}} and pass the destructured values into the dependency array of {{reactHook}}.`,
32+
},
33+
schema: [],
34+
},
35+
defaultOptions: [],
36+
37+
create: detectTanstackQueryImports((context) => {
38+
const trackedVariables: Record<string, string> = {}
39+
const hookAliasMap: Record<string, string> = {}
40+
41+
function getReactHook(node: TSESTree.CallExpression): string | undefined {
42+
if (node.callee.type === 'Identifier') {
43+
const calleeName = node.callee.name
44+
// Check if the identifier is a known React hook or an alias
45+
if (reactHookNames.includes(calleeName) || calleeName in hookAliasMap) {
46+
return calleeName
47+
}
48+
} else if (
49+
node.callee.type === 'MemberExpression' &&
50+
node.callee.object.type === 'Identifier' &&
51+
node.callee.object.name === 'React' &&
52+
node.callee.property.type === 'Identifier' &&
53+
reactHookNames.includes(node.callee.property.name)
54+
) {
55+
// Member expression case: `React.useCallback`
56+
return node.callee.property.name
57+
}
58+
return undefined
59+
}
60+
61+
function collectVariableNames(
62+
pattern: TSESTree.BindingName,
63+
queryHook: string,
64+
) {
65+
if (pattern.type === AST_NODE_TYPES.Identifier) {
66+
trackedVariables[pattern.name] = queryHook
67+
}
68+
}
69+
70+
return {
71+
ImportDeclaration(node: TSESTree.ImportDeclaration) {
72+
if (
73+
node.specifiers.length > 0 &&
74+
node.importKind === 'value' &&
75+
node.source.value === 'React'
76+
) {
77+
node.specifiers.forEach((specifier) => {
78+
if (
79+
specifier.type === AST_NODE_TYPES.ImportSpecifier &&
80+
reactHookNames.includes(specifier.imported.name)
81+
) {
82+
// Track alias or direct import
83+
hookAliasMap[specifier.local.name] = specifier.imported.name
84+
}
85+
})
86+
}
87+
},
88+
89+
VariableDeclarator(node) {
90+
if (
91+
node.init !== null &&
92+
node.init.type === AST_NODE_TYPES.CallExpression &&
93+
node.init.callee.type === AST_NODE_TYPES.Identifier &&
94+
allHookNames.includes(node.init.callee.name)
95+
) {
96+
collectVariableNames(node.id, node.init.callee.name)
97+
}
98+
},
99+
CallExpression: (node) => {
100+
const reactHook = getReactHook(node)
101+
if (
102+
reactHook !== undefined &&
103+
node.arguments.length > 1 &&
104+
node.arguments[1]?.type === AST_NODE_TYPES.ArrayExpression
105+
) {
106+
const depsArray = node.arguments[1].elements
107+
depsArray.forEach((dep) => {
108+
if (
109+
dep !== null &&
110+
dep.type === AST_NODE_TYPES.Identifier &&
111+
trackedVariables[dep.name] !== undefined
112+
) {
113+
const queryHook = trackedVariables[dep.name]
114+
context.report({
115+
node: dep,
116+
messageId: 'noUnstableDeps',
117+
data: {
118+
queryHook,
119+
reactHook,
120+
},
121+
})
122+
}
123+
})
124+
}
125+
},
126+
}
127+
}),
128+
})

0 commit comments

Comments
 (0)
Please sign in to comment.