From a93ea4104a586c491342d34d276006462ea1ac0a Mon Sep 17 00:00:00 2001 From: Manuel Schiller Date: Sun, 18 Aug 2024 14:24:55 +0200 Subject: [PATCH 1/5] feat(eslint-plugin-query): add rule that disallows putting the result of useMutation directly in a React hook dependency array --- .../src/__tests__/no-mutation-in-deps.test.ts | 64 ++++++++++ packages/eslint-plugin-query/src/rules.ts | 2 + .../no-mutation-in-deps.rule.ts | 109 ++++++++++++++++++ 3 files changed, 175 insertions(+) create mode 100644 packages/eslint-plugin-query/src/__tests__/no-mutation-in-deps.test.ts create mode 100644 packages/eslint-plugin-query/src/rules/no-mutation-in-deps/no-mutation-in-deps.rule.ts diff --git a/packages/eslint-plugin-query/src/__tests__/no-mutation-in-deps.test.ts b/packages/eslint-plugin-query/src/__tests__/no-mutation-in-deps.test.ts new file mode 100644 index 0000000000..74086d0532 --- /dev/null +++ b/packages/eslint-plugin-query/src/__tests__/no-mutation-in-deps.test.ts @@ -0,0 +1,64 @@ +import { RuleTester } from '@typescript-eslint/rule-tester' +import { + reactHookNames, + rule, +} from '../rules/no-mutation-in-deps/no-mutation-in-deps.rule' + +const ruleTester = new RuleTester({ + parser: '@typescript-eslint/parser', + settings: {}, +}) + +const baseTestCases = { + valid: (importStatement: string, hookInvocation: string) => ({ + name: `should pass when destructured mutate is passed to useCallback as dependency - ${importStatement} - ${hookInvocation}`, + code: ` + ${importStatement} + import { useMutation } from "@tanstack/react-query"; + + function Component() { + const { mutate } = useMutation({ mutationFn: (value: string) => value }); + const callback = ${hookInvocation}(() => { mutate('hello') }, [mutate]); + return; + } + `, + }), + invalid: (importStatement: string, hookInvocation: string) => ({ + name: `result of useMutation is passed to useCallback as dependency - ${importStatement} - ${hookInvocation}`, + code: ` + ${importStatement} + import { useMutation } from "@tanstack/react-query"; + + function Component() { + const mutation = useMutation({ mutationFn: (value: string) => value }); + const callback = ${hookInvocation}(() => { mutation.mutate('hello') }, [mutation]); + return; + } + `, + errors: [{ messageId: 'mutationInDeps' }], + }), +} + +const testCases = (hook: string) => [ + { + importStatement: 'import * as React from "React";', + hookInvocation: `React.${hook}`, + }, + { + importStatement: `import { ${hook} } from "React";`, + hookInvocation: hook, + }, + { + importStatement: `import { ${hook} as useAlias } from "React";`, + hookInvocation: 'useAlias', + }, +] + +reactHookNames.forEach((hookName) => { + testCases(hookName).forEach(({ importStatement, hookInvocation }) => { + ruleTester.run('no-mutation-in-deps', rule, { + valid: [baseTestCases.valid(importStatement, hookInvocation)], + invalid: [baseTestCases.invalid(importStatement, hookInvocation)], + }) + }) +}) diff --git a/packages/eslint-plugin-query/src/rules.ts b/packages/eslint-plugin-query/src/rules.ts index fe7ac8c215..26cce9177a 100644 --- a/packages/eslint-plugin-query/src/rules.ts +++ b/packages/eslint-plugin-query/src/rules.ts @@ -1,6 +1,7 @@ import * as exhaustiveDeps from './rules/exhaustive-deps/exhaustive-deps.rule' import * as stableQueryClient from './rules/stable-query-client/stable-query-client.rule' import * as noRestDestructuring from './rules/no-rest-destructuring/no-rest-destructuring.rule' +import * as noMutationInDeps from './rules/no-mutation-in-deps/no-mutation-in-deps.rule' import type { ESLintUtils } from '@typescript-eslint/utils' import type { ExtraRuleDocs } from './types' @@ -16,4 +17,5 @@ export const rules: Record< [exhaustiveDeps.name]: exhaustiveDeps.rule, [stableQueryClient.name]: stableQueryClient.rule, [noRestDestructuring.name]: noRestDestructuring.rule, + [noMutationInDeps.name]: noMutationInDeps.rule, } diff --git a/packages/eslint-plugin-query/src/rules/no-mutation-in-deps/no-mutation-in-deps.rule.ts b/packages/eslint-plugin-query/src/rules/no-mutation-in-deps/no-mutation-in-deps.rule.ts new file mode 100644 index 0000000000..7022037624 --- /dev/null +++ b/packages/eslint-plugin-query/src/rules/no-mutation-in-deps/no-mutation-in-deps.rule.ts @@ -0,0 +1,109 @@ +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-mutation-in-deps' + +export const reactHookNames = ['useEffect', 'useCallback', 'useMemo'] + +const createRule = ESLintUtils.RuleCreator(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: { + mutationInDeps: `The result of useMutation is not referentially stable, so don't pass it directly into the dependencies array of a hook like useEffect, useMemo, or useCallback. Instead, destructure the return value of useMutation and pass the destructured values into the dependency array.`, + }, + schema: [], + }, + defaultOptions: [], + + create: detectTanstackQueryImports((context) => { + const trackedVariables = new Set() + const hookAliasMap: Record = {} + + function isReactHook(node: TSESTree.CallExpression): boolean { + if (node.callee.type === 'Identifier') { + const calleeName = node.callee.name + // Check if the identifier is a known React hook or an alias + return reactHookNames.includes(calleeName) || calleeName in hookAliasMap + } 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 true + } + return false + } + + function collectVariableNames(pattern: TSESTree.BindingName) { + if (pattern.type === AST_NODE_TYPES.Identifier) { + trackedVariables.add(pattern.name) + } + } + + 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 && + node.init.callee.name === 'useMutation' + ) { + collectVariableNames(node.id) + } + }, + CallExpression: (node) => { + if ( + isReactHook(node) && + 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.has(dep.name) + ) { + context.report({ + node: dep, + messageId: 'mutationInDeps', + }) + } + }) + } + }, + } + }), +}) From 4f93227d25c9184ef8e07ea1b78a44c8e2f61ee5 Mon Sep 17 00:00:00 2001 From: Manuel Schiller Date: Sun, 18 Aug 2024 14:32:35 +0200 Subject: [PATCH 2/5] add doc --- docs/config.json | 4 +++ docs/eslint/eslint-plugin-query.md | 1 + docs/eslint/no-mutation-in-deps.md | 46 ++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+) create mode 100644 docs/eslint/no-mutation-in-deps.md diff --git a/docs/config.json b/docs/config.json index 24239e85e6..40143ae603 100644 --- a/docs/config.json +++ b/docs/config.json @@ -792,6 +792,10 @@ { "label": "No Rest Destructuring", "to": "eslint/no-rest-destructuring" + }, + { + "label": "No Mutation in Deps", + "to": "eslint/no-mutation-in-deps" } ] }, diff --git a/docs/eslint/eslint-plugin-query.md b/docs/eslint/eslint-plugin-query.md index 4cad83beb9..f1d819aee6 100644 --- a/docs/eslint/eslint-plugin-query.md +++ b/docs/eslint/eslint-plugin-query.md @@ -96,3 +96,4 @@ Alternatively, add `@tanstack/eslint-plugin-query` to the plugins section, and c - [@tanstack/query/exhaustive-deps](../exhaustive-deps) - [@tanstack/query/no-rest-destructuring](../no-rest-destructuring) - [@tanstack/query/stable-query-client](../stable-query-client) +- [@tanstack/query/no-mutation-in-deps](../no-mutation-in-deps.md) diff --git a/docs/eslint/no-mutation-in-deps.md b/docs/eslint/no-mutation-in-deps.md new file mode 100644 index 0000000000..cc8baf09c5 --- /dev/null +++ b/docs/eslint/no-mutation-in-deps.md @@ -0,0 +1,46 @@ +--- +id: no-mutation-in-deps +title: Disallow putting the result of `useMutation` directly in a React hook dependency array +--- + +The object returned from `useMutation` is **not** referentially stable, so it should **not** be put directly into the dependency array of a React hook (e.g. `useEffect`, `useMemo`, `useCallback`). +Instead, destructure the return value of useMutation and pass the destructured values into the dependency array. + +## Rule Details + +Examples of **incorrect** code for this rule: + +```tsx +/* eslint "@tanstack/query/no-mutation-in-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-mutation-in-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 From b74f5cebd40c2440a982c31c46c515036a2f3b97 Mon Sep 17 00:00:00 2001 From: Manuel Schiller Date: Sun, 18 Aug 2024 18:33:36 +0200 Subject: [PATCH 3/5] add rule to recommended rules --- packages/eslint-plugin-query/src/index.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/eslint-plugin-query/src/index.ts b/packages/eslint-plugin-query/src/index.ts index 595e44ea2d..236bf24516 100644 --- a/packages/eslint-plugin-query/src/index.ts +++ b/packages/eslint-plugin-query/src/index.ts @@ -28,6 +28,7 @@ Object.assign(plugin.configs, { '@tanstack/query/exhaustive-deps': 'error', '@tanstack/query/no-rest-destructuring': 'warn', '@tanstack/query/stable-query-client': 'error', + '@tanstack/query/no-mutation-in-deps': 'error', }, }, 'flat/recommended': [ @@ -39,6 +40,7 @@ Object.assign(plugin.configs, { '@tanstack/query/exhaustive-deps': 'error', '@tanstack/query/no-rest-destructuring': 'warn', '@tanstack/query/stable-query-client': 'error', + '@tanstack/query/no-mutation-in-deps': 'error', }, }, ], From 8065447a4dfbc386e25545071aeb9bfe7e7afcef Mon Sep 17 00:00:00 2001 From: Manuel Schiller Date: Mon, 19 Aug 2024 18:33:31 +0200 Subject: [PATCH 4/5] generalize to all affected query hooks --- docs/config.json | 4 +- docs/eslint/eslint-plugin-query.md | 2 +- docs/eslint/no-mutation-in-deps.md | 46 ------- .../no-unstable-query-mutation-in-deps.md | 56 ++++++++ .../src/__tests__/no-mutation-in-deps.test.ts | 64 --------- ...no-unstable-query-mutation-in-deps.test.ts | 128 ++++++++++++++++++ packages/eslint-plugin-query/src/index.ts | 4 +- packages/eslint-plugin-query/src/rules.ts | 4 +- ...o-unstable-query-mutation-in-deps.rule.ts} | 49 +++++-- 9 files changed, 225 insertions(+), 132 deletions(-) delete mode 100644 docs/eslint/no-mutation-in-deps.md create mode 100644 docs/eslint/no-unstable-query-mutation-in-deps.md delete mode 100644 packages/eslint-plugin-query/src/__tests__/no-mutation-in-deps.test.ts create mode 100644 packages/eslint-plugin-query/src/__tests__/no-unstable-query-mutation-in-deps.test.ts rename packages/eslint-plugin-query/src/rules/{no-mutation-in-deps/no-mutation-in-deps.rule.ts => no-unstable-query-mutation-in-deps/no-unstable-query-mutation-in-deps.rule.ts} (64%) diff --git a/docs/config.json b/docs/config.json index 40143ae603..87a770bf91 100644 --- a/docs/config.json +++ b/docs/config.json @@ -794,8 +794,8 @@ "to": "eslint/no-rest-destructuring" }, { - "label": "No Mutation in Deps", - "to": "eslint/no-mutation-in-deps" + "label": "No Unstable Query/Mutation in Deps", + "to": "eslint/no-unstable-query-mutation-in-deps" } ] }, diff --git a/docs/eslint/eslint-plugin-query.md b/docs/eslint/eslint-plugin-query.md index f1d819aee6..8cee2eff50 100644 --- a/docs/eslint/eslint-plugin-query.md +++ b/docs/eslint/eslint-plugin-query.md @@ -96,4 +96,4 @@ Alternatively, add `@tanstack/eslint-plugin-query` to the plugins section, and c - [@tanstack/query/exhaustive-deps](../exhaustive-deps) - [@tanstack/query/no-rest-destructuring](../no-rest-destructuring) - [@tanstack/query/stable-query-client](../stable-query-client) -- [@tanstack/query/no-mutation-in-deps](../no-mutation-in-deps.md) +- [@tanstack/query/no-unstable-query-mutation-in-deps](../no-unstable-query-mutation-in-deps.md) diff --git a/docs/eslint/no-mutation-in-deps.md b/docs/eslint/no-mutation-in-deps.md deleted file mode 100644 index cc8baf09c5..0000000000 --- a/docs/eslint/no-mutation-in-deps.md +++ /dev/null @@ -1,46 +0,0 @@ ---- -id: no-mutation-in-deps -title: Disallow putting the result of `useMutation` directly in a React hook dependency array ---- - -The object returned from `useMutation` is **not** referentially stable, so it should **not** be put directly into the dependency array of a React hook (e.g. `useEffect`, `useMemo`, `useCallback`). -Instead, destructure the return value of useMutation and pass the destructured values into the dependency array. - -## Rule Details - -Examples of **incorrect** code for this rule: - -```tsx -/* eslint "@tanstack/query/no-mutation-in-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-mutation-in-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 diff --git a/docs/eslint/no-unstable-query-mutation-in-deps.md b/docs/eslint/no-unstable-query-mutation-in-deps.md new file mode 100644 index 0000000000..d5ccae1ca7 --- /dev/null +++ b/docs/eslint/no-unstable-query-mutation-in-deps.md @@ -0,0 +1,56 @@ +--- +id: no-unstable-query-mutation-in-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-query-mutation-in-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-query-mutation-in-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 diff --git a/packages/eslint-plugin-query/src/__tests__/no-mutation-in-deps.test.ts b/packages/eslint-plugin-query/src/__tests__/no-mutation-in-deps.test.ts deleted file mode 100644 index 74086d0532..0000000000 --- a/packages/eslint-plugin-query/src/__tests__/no-mutation-in-deps.test.ts +++ /dev/null @@ -1,64 +0,0 @@ -import { RuleTester } from '@typescript-eslint/rule-tester' -import { - reactHookNames, - rule, -} from '../rules/no-mutation-in-deps/no-mutation-in-deps.rule' - -const ruleTester = new RuleTester({ - parser: '@typescript-eslint/parser', - settings: {}, -}) - -const baseTestCases = { - valid: (importStatement: string, hookInvocation: string) => ({ - name: `should pass when destructured mutate is passed to useCallback as dependency - ${importStatement} - ${hookInvocation}`, - code: ` - ${importStatement} - import { useMutation } from "@tanstack/react-query"; - - function Component() { - const { mutate } = useMutation({ mutationFn: (value: string) => value }); - const callback = ${hookInvocation}(() => { mutate('hello') }, [mutate]); - return; - } - `, - }), - invalid: (importStatement: string, hookInvocation: string) => ({ - name: `result of useMutation is passed to useCallback as dependency - ${importStatement} - ${hookInvocation}`, - code: ` - ${importStatement} - import { useMutation } from "@tanstack/react-query"; - - function Component() { - const mutation = useMutation({ mutationFn: (value: string) => value }); - const callback = ${hookInvocation}(() => { mutation.mutate('hello') }, [mutation]); - return; - } - `, - errors: [{ messageId: 'mutationInDeps' }], - }), -} - -const testCases = (hook: string) => [ - { - importStatement: 'import * as React from "React";', - hookInvocation: `React.${hook}`, - }, - { - importStatement: `import { ${hook} } from "React";`, - hookInvocation: hook, - }, - { - importStatement: `import { ${hook} as useAlias } from "React";`, - hookInvocation: 'useAlias', - }, -] - -reactHookNames.forEach((hookName) => { - testCases(hookName).forEach(({ importStatement, hookInvocation }) => { - ruleTester.run('no-mutation-in-deps', rule, { - valid: [baseTestCases.valid(importStatement, hookInvocation)], - invalid: [baseTestCases.invalid(importStatement, hookInvocation)], - }) - }) -}) diff --git a/packages/eslint-plugin-query/src/__tests__/no-unstable-query-mutation-in-deps.test.ts b/packages/eslint-plugin-query/src/__tests__/no-unstable-query-mutation-in-deps.test.ts new file mode 100644 index 0000000000..82b183abb2 --- /dev/null +++ b/packages/eslint-plugin-query/src/__tests__/no-unstable-query-mutation-in-deps.test.ts @@ -0,0 +1,128 @@ +import { RuleTester } from '@typescript-eslint/rule-tester' +import { + reactHookNames, + rule, + useQueryHookNames, +} from '../rules/no-unstable-query-mutation-in-deps/no-unstable-query-mutation-in-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: 'noUnstableQueryMutationInDeps', + 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: 'noUnstableQueryMutationInDeps', + 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-query-mutation-in-deps', rule, { + valid: baseTestCases.valid({ + reactHookImport, + reactHookInvocation, + reactHookAlias, + }), + invalid: baseTestCases.invalid({ + reactHookImport, + reactHookInvocation, + reactHookAlias, + }), + }) + }, + ) +}) diff --git a/packages/eslint-plugin-query/src/index.ts b/packages/eslint-plugin-query/src/index.ts index 236bf24516..0770e2ceca 100644 --- a/packages/eslint-plugin-query/src/index.ts +++ b/packages/eslint-plugin-query/src/index.ts @@ -28,7 +28,7 @@ Object.assign(plugin.configs, { '@tanstack/query/exhaustive-deps': 'error', '@tanstack/query/no-rest-destructuring': 'warn', '@tanstack/query/stable-query-client': 'error', - '@tanstack/query/no-mutation-in-deps': 'error', + '@tanstack/query/no-unstable-query-mutation-in-deps': 'error', }, }, 'flat/recommended': [ @@ -40,7 +40,7 @@ Object.assign(plugin.configs, { '@tanstack/query/exhaustive-deps': 'error', '@tanstack/query/no-rest-destructuring': 'warn', '@tanstack/query/stable-query-client': 'error', - '@tanstack/query/no-mutation-in-deps': 'error', + '@tanstack/query/no-unstable-query-mutation-in-deps': 'error', }, }, ], diff --git a/packages/eslint-plugin-query/src/rules.ts b/packages/eslint-plugin-query/src/rules.ts index 26cce9177a..279760c65f 100644 --- a/packages/eslint-plugin-query/src/rules.ts +++ b/packages/eslint-plugin-query/src/rules.ts @@ -1,7 +1,7 @@ import * as exhaustiveDeps from './rules/exhaustive-deps/exhaustive-deps.rule' import * as stableQueryClient from './rules/stable-query-client/stable-query-client.rule' import * as noRestDestructuring from './rules/no-rest-destructuring/no-rest-destructuring.rule' -import * as noMutationInDeps from './rules/no-mutation-in-deps/no-mutation-in-deps.rule' +import * as noUnstableQueryMutationInDeps from './rules/no-unstable-query-mutation-in-deps/no-unstable-query-mutation-in-deps.rule' import type { ESLintUtils } from '@typescript-eslint/utils' import type { ExtraRuleDocs } from './types' @@ -17,5 +17,5 @@ export const rules: Record< [exhaustiveDeps.name]: exhaustiveDeps.rule, [stableQueryClient.name]: stableQueryClient.rule, [noRestDestructuring.name]: noRestDestructuring.rule, - [noMutationInDeps.name]: noMutationInDeps.rule, + [noUnstableQueryMutationInDeps.name]: noUnstableQueryMutationInDeps.rule, } diff --git a/packages/eslint-plugin-query/src/rules/no-mutation-in-deps/no-mutation-in-deps.rule.ts b/packages/eslint-plugin-query/src/rules/no-unstable-query-mutation-in-deps/no-unstable-query-mutation-in-deps.rule.ts similarity index 64% rename from packages/eslint-plugin-query/src/rules/no-mutation-in-deps/no-mutation-in-deps.rule.ts rename to packages/eslint-plugin-query/src/rules/no-unstable-query-mutation-in-deps/no-unstable-query-mutation-in-deps.rule.ts index 7022037624..de36ff0cf2 100644 --- a/packages/eslint-plugin-query/src/rules/no-mutation-in-deps/no-mutation-in-deps.rule.ts +++ b/packages/eslint-plugin-query/src/rules/no-unstable-query-mutation-in-deps/no-unstable-query-mutation-in-deps.rule.ts @@ -4,10 +4,18 @@ import { detectTanstackQueryImports } from '../../utils/detect-react-query-impor import type { TSESTree } from '@typescript-eslint/utils' import type { ExtraRuleDocs } from '../../types' -export const name = 'no-mutation-in-deps' +export const name = 'no-unstable-query-mutation-in-deps' export const reactHookNames = ['useEffect', 'useCallback', 'useMemo'] - +export const useQueryHookNames = [ + 'useQuery', + 'useSuspenseQuery', + 'useQueries', + 'useSuspenseQueries', + 'useInfiniteQuery', + 'useSuspenseInfiniteQuery', +] +const allHookNames = ['useMutation', ...useQueryHookNames] const createRule = ESLintUtils.RuleCreator(getDocsUrl) export const rule = createRule({ @@ -20,21 +28,23 @@ export const rule = createRule({ recommended: 'error', }, messages: { - mutationInDeps: `The result of useMutation is not referentially stable, so don't pass it directly into the dependencies array of a hook like useEffect, useMemo, or useCallback. Instead, destructure the return value of useMutation and pass the destructured values into the dependency array.`, + noUnstableQueryMutationInDeps: `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 = new Set() + const trackedVariables: Record = {} const hookAliasMap: Record = {} - function isReactHook(node: TSESTree.CallExpression): boolean { + 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 - return reactHookNames.includes(calleeName) || calleeName in hookAliasMap + if (reactHookNames.includes(calleeName) || calleeName in hookAliasMap) { + return calleeName + } } else if ( node.callee.type === 'MemberExpression' && node.callee.object.type === 'Identifier' && @@ -43,14 +53,17 @@ export const rule = createRule({ reactHookNames.includes(node.callee.property.name) ) { // Member expression case: `React.useCallback` - return true + return node.callee.property.name } - return false + return undefined } - function collectVariableNames(pattern: TSESTree.BindingName) { + function collectVariableNames( + pattern: TSESTree.BindingName, + queryHook: string, + ) { if (pattern.type === AST_NODE_TYPES.Identifier) { - trackedVariables.add(pattern.name) + trackedVariables[pattern.name] = queryHook } } @@ -78,14 +91,15 @@ export const rule = createRule({ node.init !== null && node.init.type === AST_NODE_TYPES.CallExpression && node.init.callee.type === AST_NODE_TYPES.Identifier && - node.init.callee.name === 'useMutation' + allHookNames.includes(node.init.callee.name) ) { - collectVariableNames(node.id) + collectVariableNames(node.id, node.init.callee.name) } }, CallExpression: (node) => { + const reactHook = getReactHook(node) if ( - isReactHook(node) && + reactHook !== undefined && node.arguments.length > 1 && node.arguments[1]?.type === AST_NODE_TYPES.ArrayExpression ) { @@ -94,11 +108,16 @@ export const rule = createRule({ if ( dep !== null && dep.type === AST_NODE_TYPES.Identifier && - trackedVariables.has(dep.name) + trackedVariables[dep.name] !== undefined ) { + const queryHook = trackedVariables[dep.name] context.report({ node: dep, - messageId: 'mutationInDeps', + messageId: 'noUnstableQueryMutationInDeps', + data: { + queryHook, + reactHook, + }, }) } }) From 5191cec05959dbcda3d76103f891cb08df21b8a2 Mon Sep 17 00:00:00 2001 From: Manuel Schiller Date: Mon, 19 Aug 2024 20:46:15 +0200 Subject: [PATCH 5/5] rename rule to "no-unstable-deps" --- docs/config.json | 4 ++-- docs/eslint/eslint-plugin-query.md | 2 +- ...ery-mutation-in-deps.md => no-unstable-deps.md} | 6 +++--- ...on-in-deps.test.ts => no-unstable-deps.test.ts} | 14 +++++++++----- packages/eslint-plugin-query/src/index.ts | 4 ++-- packages/eslint-plugin-query/src/rules.ts | 4 ++-- .../no-unstable-deps.rule.ts} | 6 +++--- 7 files changed, 22 insertions(+), 18 deletions(-) rename docs/eslint/{no-unstable-query-mutation-in-deps.md => no-unstable-deps.md} (87%) rename packages/eslint-plugin-query/src/__tests__/{no-unstable-query-mutation-in-deps.test.ts => no-unstable-deps.test.ts} (91%) rename packages/eslint-plugin-query/src/rules/{no-unstable-query-mutation-in-deps/no-unstable-query-mutation-in-deps.rule.ts => no-unstable-deps/no-unstable-deps.rule.ts} (90%) diff --git a/docs/config.json b/docs/config.json index 87a770bf91..89eb47fd2a 100644 --- a/docs/config.json +++ b/docs/config.json @@ -794,8 +794,8 @@ "to": "eslint/no-rest-destructuring" }, { - "label": "No Unstable Query/Mutation in Deps", - "to": "eslint/no-unstable-query-mutation-in-deps" + "label": "No Unstable Deps", + "to": "eslint/no-unstable-deps" } ] }, diff --git a/docs/eslint/eslint-plugin-query.md b/docs/eslint/eslint-plugin-query.md index 8cee2eff50..d8f6839c94 100644 --- a/docs/eslint/eslint-plugin-query.md +++ b/docs/eslint/eslint-plugin-query.md @@ -96,4 +96,4 @@ Alternatively, add `@tanstack/eslint-plugin-query` to the plugins section, and c - [@tanstack/query/exhaustive-deps](../exhaustive-deps) - [@tanstack/query/no-rest-destructuring](../no-rest-destructuring) - [@tanstack/query/stable-query-client](../stable-query-client) -- [@tanstack/query/no-unstable-query-mutation-in-deps](../no-unstable-query-mutation-in-deps.md) +- [@tanstack/query/no-unstable-deps](../no-unstable-deps.md) diff --git a/docs/eslint/no-unstable-query-mutation-in-deps.md b/docs/eslint/no-unstable-deps.md similarity index 87% rename from docs/eslint/no-unstable-query-mutation-in-deps.md rename to docs/eslint/no-unstable-deps.md index d5ccae1ca7..529f82def6 100644 --- a/docs/eslint/no-unstable-query-mutation-in-deps.md +++ b/docs/eslint/no-unstable-deps.md @@ -1,5 +1,5 @@ --- -id: no-unstable-query-mutation-in-deps +id: no-unstable-deps title: Disallow putting the result of query hooks directly in a React hook dependency array --- @@ -21,7 +21,7 @@ Instead, destructure the return value of the query hook and pass the destructure Examples of **incorrect** code for this rule: ```tsx -/* eslint "@tanstack/query/no-unstable-query-mutation-in-deps": "warn" */ +/* eslint "@tanstack/query/no-unstable-deps": "warn" */ import { useCallback } from 'React' import { useMutation } from '@tanstack/react-query' @@ -37,7 +37,7 @@ function Component() { Examples of **correct** code for this rule: ```tsx -/* eslint "@tanstack/query/no-unstable-query-mutation-in-deps": "warn" */ +/* eslint "@tanstack/query/no-unstable-deps": "warn" */ import { useCallback } from 'React' import { useMutation } from '@tanstack/react-query' diff --git a/packages/eslint-plugin-query/src/__tests__/no-unstable-query-mutation-in-deps.test.ts b/packages/eslint-plugin-query/src/__tests__/no-unstable-deps.test.ts similarity index 91% rename from packages/eslint-plugin-query/src/__tests__/no-unstable-query-mutation-in-deps.test.ts rename to packages/eslint-plugin-query/src/__tests__/no-unstable-deps.test.ts index 82b183abb2..31b0fc8ef7 100644 --- a/packages/eslint-plugin-query/src/__tests__/no-unstable-query-mutation-in-deps.test.ts +++ b/packages/eslint-plugin-query/src/__tests__/no-unstable-deps.test.ts @@ -3,7 +3,7 @@ import { reactHookNames, rule, useQueryHookNames, -} from '../rules/no-unstable-query-mutation-in-deps/no-unstable-query-mutation-in-deps.rule' +} from '../rules/no-unstable-deps/no-unstable-deps.rule' const ruleTester = new RuleTester({ parser: '@typescript-eslint/parser', @@ -46,7 +46,11 @@ const baseTestCases = { `, })), ), - invalid: ({ reactHookImport, reactHookInvocation, reactHookAlias }: TestCase) => + invalid: ({ + reactHookImport, + reactHookInvocation, + reactHookAlias, + }: TestCase) => [ { name: `result of useMutation is passed to ${reactHookInvocation} as dependency `, @@ -62,7 +66,7 @@ const baseTestCases = { `, errors: [ { - messageId: 'noUnstableQueryMutationInDeps', + messageId: 'noUnstableDeps', data: { reactHook: reactHookAlias, queryHook: 'useMutation' }, }, ], @@ -82,7 +86,7 @@ const baseTestCases = { `, errors: [ { - messageId: 'noUnstableQueryMutationInDeps', + messageId: 'noUnstableDeps', data: { reactHook: reactHookAlias, queryHook }, }, ], @@ -111,7 +115,7 @@ const testCases = (reactHookName: string) => [ reactHookNames.forEach((reactHookName) => { testCases(reactHookName).forEach( ({ reactHookInvocation, reactHookAlias, reactHookImport }) => { - ruleTester.run('no-unstable-query-mutation-in-deps', rule, { + ruleTester.run('no-unstable-deps', rule, { valid: baseTestCases.valid({ reactHookImport, reactHookInvocation, diff --git a/packages/eslint-plugin-query/src/index.ts b/packages/eslint-plugin-query/src/index.ts index 0770e2ceca..f70ccf7a88 100644 --- a/packages/eslint-plugin-query/src/index.ts +++ b/packages/eslint-plugin-query/src/index.ts @@ -28,7 +28,7 @@ Object.assign(plugin.configs, { '@tanstack/query/exhaustive-deps': 'error', '@tanstack/query/no-rest-destructuring': 'warn', '@tanstack/query/stable-query-client': 'error', - '@tanstack/query/no-unstable-query-mutation-in-deps': 'error', + '@tanstack/query/no-unstable-deps': 'error', }, }, 'flat/recommended': [ @@ -40,7 +40,7 @@ Object.assign(plugin.configs, { '@tanstack/query/exhaustive-deps': 'error', '@tanstack/query/no-rest-destructuring': 'warn', '@tanstack/query/stable-query-client': 'error', - '@tanstack/query/no-unstable-query-mutation-in-deps': 'error', + '@tanstack/query/no-unstable-deps': 'error', }, }, ], diff --git a/packages/eslint-plugin-query/src/rules.ts b/packages/eslint-plugin-query/src/rules.ts index 279760c65f..9bc18ae9ea 100644 --- a/packages/eslint-plugin-query/src/rules.ts +++ b/packages/eslint-plugin-query/src/rules.ts @@ -1,7 +1,7 @@ import * as exhaustiveDeps from './rules/exhaustive-deps/exhaustive-deps.rule' import * as stableQueryClient from './rules/stable-query-client/stable-query-client.rule' import * as noRestDestructuring from './rules/no-rest-destructuring/no-rest-destructuring.rule' -import * as noUnstableQueryMutationInDeps from './rules/no-unstable-query-mutation-in-deps/no-unstable-query-mutation-in-deps.rule' +import * as noUnstableDeps from './rules/no-unstable-deps/no-unstable-deps.rule' import type { ESLintUtils } from '@typescript-eslint/utils' import type { ExtraRuleDocs } from './types' @@ -17,5 +17,5 @@ export const rules: Record< [exhaustiveDeps.name]: exhaustiveDeps.rule, [stableQueryClient.name]: stableQueryClient.rule, [noRestDestructuring.name]: noRestDestructuring.rule, - [noUnstableQueryMutationInDeps.name]: noUnstableQueryMutationInDeps.rule, + [noUnstableDeps.name]: noUnstableDeps.rule, } diff --git a/packages/eslint-plugin-query/src/rules/no-unstable-query-mutation-in-deps/no-unstable-query-mutation-in-deps.rule.ts b/packages/eslint-plugin-query/src/rules/no-unstable-deps/no-unstable-deps.rule.ts similarity index 90% rename from packages/eslint-plugin-query/src/rules/no-unstable-query-mutation-in-deps/no-unstable-query-mutation-in-deps.rule.ts rename to packages/eslint-plugin-query/src/rules/no-unstable-deps/no-unstable-deps.rule.ts index de36ff0cf2..d82738d158 100644 --- a/packages/eslint-plugin-query/src/rules/no-unstable-query-mutation-in-deps/no-unstable-query-mutation-in-deps.rule.ts +++ b/packages/eslint-plugin-query/src/rules/no-unstable-deps/no-unstable-deps.rule.ts @@ -4,7 +4,7 @@ import { detectTanstackQueryImports } from '../../utils/detect-react-query-impor import type { TSESTree } from '@typescript-eslint/utils' import type { ExtraRuleDocs } from '../../types' -export const name = 'no-unstable-query-mutation-in-deps' +export const name = 'no-unstable-deps' export const reactHookNames = ['useEffect', 'useCallback', 'useMemo'] export const useQueryHookNames = [ @@ -28,7 +28,7 @@ export const rule = createRule({ recommended: 'error', }, messages: { - noUnstableQueryMutationInDeps: `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}}.`, + 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: [], }, @@ -113,7 +113,7 @@ export const rule = createRule({ const queryHook = trackedVariables[dep.name] context.report({ node: dep, - messageId: 'noUnstableQueryMutationInDeps', + messageId: 'noUnstableDeps', data: { queryHook, reactHook,