-
-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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"
- Loading branch information
1 parent
683c85e
commit f65ebe7
Showing
7 changed files
with
325 additions
and
0 deletions.
There are no files selected for viewing
This file contains 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
This file contains 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
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
--- | ||
id: no-unstable-deps | ||
title: Disallow putting the result of query hooks directly in a React hook dependency array | ||
--- | ||
|
||
The object returned from the following query hooks is **not** referentially stable: | ||
|
||
- `useQuery` | ||
- `useSuspenseQuery` | ||
- `useQueries` | ||
- `useSuspenseQueries` | ||
- `useInfiniteQuery` | ||
- `useSuspenseInfiniteQuery` | ||
- `useMutation` | ||
|
||
The object returned from those hooks should **not** be put directly into the dependency array of a React hook (e.g. `useEffect`, `useMemo`, `useCallback`). | ||
Instead, destructure the return value of the query hook and pass the destructured values into the dependency array of the React hook. | ||
|
||
## Rule Details | ||
|
||
Examples of **incorrect** code for this rule: | ||
|
||
```tsx | ||
/* eslint "@tanstack/query/no-unstable-deps": "warn" */ | ||
import { useCallback } from 'React' | ||
import { useMutation } from '@tanstack/react-query' | ||
|
||
function Component() { | ||
const mutation = useMutation({ mutationFn: (value: string) => value }) | ||
const callback = useCallback(() => { | ||
mutation.mutate('hello') | ||
}, [mutation]) | ||
return null | ||
} | ||
``` | ||
|
||
Examples of **correct** code for this rule: | ||
|
||
```tsx | ||
/* eslint "@tanstack/query/no-unstable-deps": "warn" */ | ||
import { useCallback } from 'React' | ||
import { useMutation } from '@tanstack/react-query' | ||
|
||
function Component() { | ||
const { mutate } = useMutation({ mutationFn: (value: string) => value }) | ||
const callback = useCallback(() => { | ||
mutate('hello') | ||
}, [mutate]) | ||
return null | ||
} | ||
``` | ||
|
||
## Attributes | ||
|
||
- [x] ✅ Recommended | ||
- [ ] 🔧 Fixable |
132 changes: 132 additions & 0 deletions
132
packages/eslint-plugin-query/src/__tests__/no-unstable-deps.test.ts
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,132 @@ | ||
import { RuleTester } from '@typescript-eslint/rule-tester' | ||
import { | ||
reactHookNames, | ||
rule, | ||
useQueryHookNames, | ||
} from '../rules/no-unstable-deps/no-unstable-deps.rule' | ||
|
||
const ruleTester = new RuleTester({ | ||
parser: '@typescript-eslint/parser', | ||
settings: {}, | ||
}) | ||
|
||
interface TestCase { | ||
reactHookImport: string | ||
reactHookInvocation: string | ||
reactHookAlias: string | ||
} | ||
const baseTestCases = { | ||
valid: ({ reactHookImport, reactHookInvocation, reactHookAlias }: TestCase) => | ||
[ | ||
{ | ||
name: `should pass when destructured mutate is passed to ${reactHookAlias} as dependency`, | ||
code: ` | ||
${reactHookImport} | ||
import { useMutation } from "@tanstack/react-query"; | ||
function Component() { | ||
const { mutate } = useMutation({ mutationFn: (value: string) => value }); | ||
const callback = ${reactHookInvocation}(() => { mutate('hello') }, [mutate]); | ||
return; | ||
} | ||
`, | ||
}, | ||
].concat( | ||
useQueryHookNames.map((queryHook) => ({ | ||
name: `should pass result of ${queryHook} is passed to ${reactHookInvocation} as dependency`, | ||
code: ` | ||
${reactHookImport} | ||
import { ${queryHook} } from "@tanstack/react-query"; | ||
function Component() { | ||
const { refetch } = ${queryHook}({ queryFn: (value: string) => value }); | ||
const callback = ${reactHookInvocation}(() => { query.refetch() }, [refetch]); | ||
return; | ||
} | ||
`, | ||
})), | ||
), | ||
invalid: ({ | ||
reactHookImport, | ||
reactHookInvocation, | ||
reactHookAlias, | ||
}: TestCase) => | ||
[ | ||
{ | ||
name: `result of useMutation is passed to ${reactHookInvocation} as dependency `, | ||
code: ` | ||
${reactHookImport} | ||
import { useMutation } from "@tanstack/react-query"; | ||
function Component() { | ||
const mutation = useMutation({ mutationFn: (value: string) => value }); | ||
const callback = ${reactHookInvocation}(() => { mutation.mutate('hello') }, [mutation]); | ||
return; | ||
} | ||
`, | ||
errors: [ | ||
{ | ||
messageId: 'noUnstableDeps', | ||
data: { reactHook: reactHookAlias, queryHook: 'useMutation' }, | ||
}, | ||
], | ||
}, | ||
].concat( | ||
useQueryHookNames.map((queryHook) => ({ | ||
name: `result of ${queryHook} is passed to ${reactHookInvocation} as dependency`, | ||
code: ` | ||
${reactHookImport} | ||
import { ${queryHook} } from "@tanstack/react-query"; | ||
function Component() { | ||
const query = ${queryHook}({ queryFn: (value: string) => value }); | ||
const callback = ${reactHookInvocation}(() => { query.refetch() }, [query]); | ||
return; | ||
} | ||
`, | ||
errors: [ | ||
{ | ||
messageId: 'noUnstableDeps', | ||
data: { reactHook: reactHookAlias, queryHook }, | ||
}, | ||
], | ||
})), | ||
), | ||
} | ||
|
||
const testCases = (reactHookName: string) => [ | ||
{ | ||
reactHookImport: 'import * as React from "React";', | ||
reactHookInvocation: `React.${reactHookName}`, | ||
reactHookAlias: reactHookName, | ||
}, | ||
{ | ||
reactHookImport: `import { ${reactHookName} } from "React";`, | ||
reactHookInvocation: reactHookName, | ||
reactHookAlias: reactHookName, | ||
}, | ||
{ | ||
reactHookImport: `import { ${reactHookName} as useAlias } from "React";`, | ||
reactHookInvocation: 'useAlias', | ||
reactHookAlias: 'useAlias', | ||
}, | ||
] | ||
|
||
reactHookNames.forEach((reactHookName) => { | ||
testCases(reactHookName).forEach( | ||
({ reactHookInvocation, reactHookAlias, reactHookImport }) => { | ||
ruleTester.run('no-unstable-deps', rule, { | ||
valid: baseTestCases.valid({ | ||
reactHookImport, | ||
reactHookInvocation, | ||
reactHookAlias, | ||
}), | ||
invalid: baseTestCases.invalid({ | ||
reactHookImport, | ||
reactHookInvocation, | ||
reactHookAlias, | ||
}), | ||
}) | ||
}, | ||
) | ||
}) |
This file contains 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
This file contains 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
128 changes: 128 additions & 0 deletions
128
packages/eslint-plugin-query/src/rules/no-unstable-deps/no-unstable-deps.rule.ts
This file contains 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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,128 @@ | ||
import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/utils' | ||
import { getDocsUrl } from '../../utils/get-docs-url' | ||
import { detectTanstackQueryImports } from '../../utils/detect-react-query-imports' | ||
import type { TSESTree } from '@typescript-eslint/utils' | ||
import type { ExtraRuleDocs } from '../../types' | ||
|
||
export const name = 'no-unstable-deps' | ||
|
||
export const reactHookNames = ['useEffect', 'useCallback', 'useMemo'] | ||
export const useQueryHookNames = [ | ||
'useQuery', | ||
'useSuspenseQuery', | ||
'useQueries', | ||
'useSuspenseQueries', | ||
'useInfiniteQuery', | ||
'useSuspenseInfiniteQuery', | ||
] | ||
const allHookNames = ['useMutation', ...useQueryHookNames] | ||
const createRule = ESLintUtils.RuleCreator<ExtraRuleDocs>(getDocsUrl) | ||
|
||
export const rule = createRule({ | ||
name, | ||
meta: { | ||
type: 'problem', | ||
docs: { | ||
description: | ||
'Disallow putting the result of useMutation directly in a React hook dependency array', | ||
recommended: 'error', | ||
}, | ||
messages: { | ||
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}}.`, | ||
}, | ||
schema: [], | ||
}, | ||
defaultOptions: [], | ||
|
||
create: detectTanstackQueryImports((context) => { | ||
const trackedVariables: Record<string, string> = {} | ||
const hookAliasMap: Record<string, string> = {} | ||
|
||
function getReactHook(node: TSESTree.CallExpression): string | undefined { | ||
if (node.callee.type === 'Identifier') { | ||
const calleeName = node.callee.name | ||
// Check if the identifier is a known React hook or an alias | ||
if (reactHookNames.includes(calleeName) || calleeName in hookAliasMap) { | ||
return calleeName | ||
} | ||
} else if ( | ||
node.callee.type === 'MemberExpression' && | ||
node.callee.object.type === 'Identifier' && | ||
node.callee.object.name === 'React' && | ||
node.callee.property.type === 'Identifier' && | ||
reactHookNames.includes(node.callee.property.name) | ||
) { | ||
// Member expression case: `React.useCallback` | ||
return node.callee.property.name | ||
} | ||
return undefined | ||
} | ||
|
||
function collectVariableNames( | ||
pattern: TSESTree.BindingName, | ||
queryHook: string, | ||
) { | ||
if (pattern.type === AST_NODE_TYPES.Identifier) { | ||
trackedVariables[pattern.name] = queryHook | ||
} | ||
} | ||
|
||
return { | ||
ImportDeclaration(node: TSESTree.ImportDeclaration) { | ||
if ( | ||
node.specifiers.length > 0 && | ||
node.importKind === 'value' && | ||
node.source.value === 'React' | ||
) { | ||
node.specifiers.forEach((specifier) => { | ||
if ( | ||
specifier.type === AST_NODE_TYPES.ImportSpecifier && | ||
reactHookNames.includes(specifier.imported.name) | ||
) { | ||
// Track alias or direct import | ||
hookAliasMap[specifier.local.name] = specifier.imported.name | ||
} | ||
}) | ||
} | ||
}, | ||
|
||
VariableDeclarator(node) { | ||
if ( | ||
node.init !== null && | ||
node.init.type === AST_NODE_TYPES.CallExpression && | ||
node.init.callee.type === AST_NODE_TYPES.Identifier && | ||
allHookNames.includes(node.init.callee.name) | ||
) { | ||
collectVariableNames(node.id, node.init.callee.name) | ||
} | ||
}, | ||
CallExpression: (node) => { | ||
const reactHook = getReactHook(node) | ||
if ( | ||
reactHook !== undefined && | ||
node.arguments.length > 1 && | ||
node.arguments[1]?.type === AST_NODE_TYPES.ArrayExpression | ||
) { | ||
const depsArray = node.arguments[1].elements | ||
depsArray.forEach((dep) => { | ||
if ( | ||
dep !== null && | ||
dep.type === AST_NODE_TYPES.Identifier && | ||
trackedVariables[dep.name] !== undefined | ||
) { | ||
const queryHook = trackedVariables[dep.name] | ||
context.report({ | ||
node: dep, | ||
messageId: 'noUnstableDeps', | ||
data: { | ||
queryHook, | ||
reactHook, | ||
}, | ||
}) | ||
} | ||
}) | ||
} | ||
}, | ||
} | ||
}), | ||
}) |