From cf59c2e21022b5ead58dafc865d953ccc3c71a37 Mon Sep 17 00:00:00 2001 From: jochongs Date: Mon, 26 May 2025 13:17:14 +0900 Subject: [PATCH] feat(eslint-plugin): refactor and add mutation-property-order rule --- .../mutation-property-order.rule.test.ts | 218 ++++++++++++++++++ ...st.ts => sort-data-by-order.utils.test.ts} | 2 +- packages/eslint-plugin-query/src/index.ts | 2 + packages/eslint-plugin-query/src/rules.ts | 2 + .../constants.ts | 2 + .../infinite-query-property-order.rule.ts | 129 +++-------- .../mutation-property-order/constants.ts | 9 + .../mutation-property-order.rule.ts | 31 +++ .../src/utils/create-property-order-rule.ts | 103 +++++++++ .../sort-data-by-order.ts} | 0 10 files changed, 396 insertions(+), 102 deletions(-) create mode 100644 packages/eslint-plugin-query/src/__tests__/mutation-property-order.rule.test.ts rename packages/eslint-plugin-query/src/__tests__/{infinite-query-property-order.utils.test.ts => sort-data-by-order.utils.test.ts} (94%) create mode 100644 packages/eslint-plugin-query/src/rules/mutation-property-order/constants.ts create mode 100644 packages/eslint-plugin-query/src/rules/mutation-property-order/mutation-property-order.rule.ts create mode 100644 packages/eslint-plugin-query/src/utils/create-property-order-rule.ts rename packages/eslint-plugin-query/src/{rules/infinite-query-property-order/infinite-query-property-order.utils.ts => utils/sort-data-by-order.ts} (100%) diff --git a/packages/eslint-plugin-query/src/__tests__/mutation-property-order.rule.test.ts b/packages/eslint-plugin-query/src/__tests__/mutation-property-order.rule.test.ts new file mode 100644 index 0000000000..34839c684c --- /dev/null +++ b/packages/eslint-plugin-query/src/__tests__/mutation-property-order.rule.test.ts @@ -0,0 +1,218 @@ +import { RuleTester } from '@typescript-eslint/rule-tester' +import combinate from 'combinate' + +import { + checkedProperties, + mutationFunctions, +} from '../rules/mutation-property-order/constants' +import { + name, + rule, +} from '../rules/mutation-property-order/mutation-property-order.rule' +import { + generateInterleavedCombinations, + generatePartialCombinations, + generatePermutations, + normalizeIndent, +} from './test-utils' +import type { MutationFunctions } from '../rules/mutation-property-order/constants' + +const ruleTester = new RuleTester() + +type CheckedProperties = (typeof checkedProperties)[number] +const orderIndependentProps = [ + 'gcTime', + '...objectExpressionSpread', + '...callExpressionSpread', + '...memberCallExpressionSpread', +] as const +type OrderIndependentProps = (typeof orderIndependentProps)[number] + +interface TestCase { + mutationFunction: MutationFunctions + properties: Array +} + +const validTestMatrix = combinate({ + mutationFunction: [...mutationFunctions], + properties: generatePartialCombinations(checkedProperties, 2), +}) + +export function generateInvalidPermutations( + arr: ReadonlyArray, +): Array<{ + invalid: Array + valid: Array +}> { + const combinations = generatePartialCombinations(arr, 2) + const allPermutations: Array<{ + invalid: Array + valid: Array + }> = [] + + for (const combination of combinations) { + const permutations = generatePermutations(combination) + // skip the first permutation as it matches the original combination + const invalidPermutations = permutations.slice(1) + + if (combination.includes('onError') && combination.includes('onSettled')) { + if (combination.indexOf('onError') < combination.indexOf('onSettled')) { + // since we ignore the relative order of 'onError' and 'onSettled', we skip this combination (but keep the other one where `onSettled` is before `onError`) + + continue + } + } + + allPermutations.push( + ...invalidPermutations + .map((p) => { + // ignore the relative order of 'onError' and 'onSettled' + const correctedValid = [...combination].sort((a, b) => { + if ( + (a === 'onSettled' && b === 'onError') || + (a === 'onError' && b === 'onSettled') + ) { + return p.indexOf(a) - p.indexOf(b) + } + return checkedProperties.indexOf(a) - checkedProperties.indexOf(b) + }) + return { invalid: p, valid: correctedValid } + }) + .filter( + ({ invalid }) => + // if `onError` and `onSettled` are next to each other and `onMutate` is not present, we skip this invalid permutation + Math.abs( + invalid.indexOf('onSettled') - invalid.indexOf('onError'), + ) !== 1, + ), + ) + } + + return allPermutations +} + +const invalidPermutations = generateInvalidPermutations(checkedProperties) + +type Interleaved = CheckedProperties | OrderIndependentProps +const interleavedInvalidPermutations: Array<{ + invalid: Array + valid: Array +}> = [] +for (const invalidPermutation of invalidPermutations) { + const invalid = generateInterleavedCombinations( + invalidPermutation.invalid, + orderIndependentProps, + ) + const valid = generateInterleavedCombinations( + invalidPermutation.valid, + orderIndependentProps, + ) + + for (let i = 0; i < invalid.length; i++) { + interleavedInvalidPermutations.push({ + invalid: invalid[i]!, + valid: valid[i]!, + }) + } +} + +const invalidTestMatrix = combinate({ + mutationFunction: [...mutationFunctions], + properties: interleavedInvalidPermutations, +}) + +const callExpressionSpread = normalizeIndent` + ...mutationOptions({ + onSuccess: () => {}, + retry: 3, + })` + +function getCode({ mutationFunction: mutationFunction, properties }: TestCase) { + function getPropertyCode( + property: CheckedProperties | OrderIndependentProps, + ) { + switch (property) { + case '...objectExpressionSpread': + return `...objectExpressionSpread` + case '...callExpressionSpread': + return callExpressionSpread + case '...memberCallExpressionSpread': + return '...myOptions.mutationOptions()' + case 'gcTime': + return 'gcTime: 5 * 60 * 1000' + case 'onMutate': + return 'onMutate: (data) => {\n return { foo: data }\n}' + case 'onError': + return 'onError: (error, variables, context) => {\n console.log("error:", error, "context:", context)\n}' + case 'onSettled': + return 'onSettled: (data, error, variables, context) => {\n console.log("settled", context)\n}' + } + } + return ` + import { ${mutationFunction} } from '@tanstack/react-query' + + ${mutationFunction}({ + ${properties.map(getPropertyCode).join(',\n ')} + }) + ` +} + +const validTestCases = validTestMatrix.map( + ({ mutationFunction, properties }) => ({ + name: `should pass when order is correct for ${mutationFunction} with order: ${properties.join(', ')}`, + code: getCode({ + mutationFunction: mutationFunction, + properties, + }), + }), +) + +const invalidTestCases = invalidTestMatrix.map( + ({ mutationFunction, properties }) => ({ + name: `incorrect property order id detected for ${mutationFunction} with invalid order: ${properties.invalid.join(', ')}, valid order ${properties.valid.join(', ')}`, + code: getCode({ + mutationFunction: mutationFunction, + properties: properties.invalid, + }), + errors: [{ messageId: 'invalidOrder' }], + output: getCode({ + mutationFunction: mutationFunction, + properties: properties.valid, + }), + }), +) + +ruleTester.run(name, rule, { + valid: validTestCases, + invalid: invalidTestCases, +}) + +const regressionTestCases = { + valid: [ + { + name: 'should pass with call expression spread in useMutation', + code: normalizeIndent` + import { useMutation } from '@tanstack/react-query' + + const { mutate } = useMutation({ + ...mutationOptions({ + retry: 3, + onSuccess: () => console.log('success'), + }), + onMutate: (data) => { + return { foo: data } + }, + onError: (error, variables, context) => { + console.log(error, context) + }, + onSettled: (data, error, variables, context) => { + console.log('settled', context) + }, + }) + `, + }, + ], + invalid: [], +} + +ruleTester.run(name, rule, regressionTestCases) diff --git a/packages/eslint-plugin-query/src/__tests__/infinite-query-property-order.utils.test.ts b/packages/eslint-plugin-query/src/__tests__/sort-data-by-order.utils.test.ts similarity index 94% rename from packages/eslint-plugin-query/src/__tests__/infinite-query-property-order.utils.test.ts rename to packages/eslint-plugin-query/src/__tests__/sort-data-by-order.utils.test.ts index 6b505f7065..577252b7c6 100644 --- a/packages/eslint-plugin-query/src/__tests__/infinite-query-property-order.utils.test.ts +++ b/packages/eslint-plugin-query/src/__tests__/sort-data-by-order.utils.test.ts @@ -1,5 +1,5 @@ import { describe, expect, test } from 'vitest' -import { sortDataByOrder } from '../rules/infinite-query-property-order/infinite-query-property-order.utils' +import { sortDataByOrder } from '../utils/sort-data-by-order' describe('create-route-property-order utils', () => { describe('sortDataByOrder', () => { diff --git a/packages/eslint-plugin-query/src/index.ts b/packages/eslint-plugin-query/src/index.ts index 7a70f9e9ff..46dd3d7dba 100644 --- a/packages/eslint-plugin-query/src/index.ts +++ b/packages/eslint-plugin-query/src/index.ts @@ -31,6 +31,7 @@ Object.assign(plugin.configs, { '@tanstack/query/no-unstable-deps': 'error', '@tanstack/query/infinite-query-property-order': 'error', '@tanstack/query/no-void-query-fn': 'error', + '@tanstack/query/mutation-property-order': 'error', }, }, 'flat/recommended': [ @@ -46,6 +47,7 @@ Object.assign(plugin.configs, { '@tanstack/query/no-unstable-deps': 'error', '@tanstack/query/infinite-query-property-order': 'error', '@tanstack/query/no-void-query-fn': 'error', + '@tanstack/query/mutation-property-order': 'error', }, }, ], diff --git a/packages/eslint-plugin-query/src/rules.ts b/packages/eslint-plugin-query/src/rules.ts index 5163deb3de..d527768ec1 100644 --- a/packages/eslint-plugin-query/src/rules.ts +++ b/packages/eslint-plugin-query/src/rules.ts @@ -4,6 +4,7 @@ import * as noRestDestructuring from './rules/no-rest-destructuring/no-rest-dest import * as noUnstableDeps from './rules/no-unstable-deps/no-unstable-deps.rule' import * as infiniteQueryPropertyOrder from './rules/infinite-query-property-order/infinite-query-property-order.rule' import * as noVoidQueryFn from './rules/no-void-query-fn/no-void-query-fn.rule' +import * as mutationPropertyOrder from './rules/mutation-property-order/mutation-property-order.rule' import type { ESLintUtils } from '@typescript-eslint/utils' import type { ExtraRuleDocs } from './types' @@ -22,4 +23,5 @@ export const rules: Record< [noUnstableDeps.name]: noUnstableDeps.rule, [infiniteQueryPropertyOrder.name]: infiniteQueryPropertyOrder.rule, [noVoidQueryFn.name]: noVoidQueryFn.rule, + [mutationPropertyOrder.name]: mutationPropertyOrder.rule, } diff --git a/packages/eslint-plugin-query/src/rules/infinite-query-property-order/constants.ts b/packages/eslint-plugin-query/src/rules/infinite-query-property-order/constants.ts index f3b5a1f911..ec1f3ff287 100644 --- a/packages/eslint-plugin-query/src/rules/infinite-query-property-order/constants.ts +++ b/packages/eslint-plugin-query/src/rules/infinite-query-property-order/constants.ts @@ -12,6 +12,8 @@ export const checkedProperties = [ 'getNextPageParam', ] as const +export type InfiniteQueryProperties = (typeof checkedProperties)[number] + export const sortRules = [ [['queryFn'], ['getPreviousPageParam', 'getNextPageParam']], ] as const diff --git a/packages/eslint-plugin-query/src/rules/infinite-query-property-order/infinite-query-property-order.rule.ts b/packages/eslint-plugin-query/src/rules/infinite-query-property-order/infinite-query-property-order.rule.ts index d145030cc6..7568fef9a4 100644 --- a/packages/eslint-plugin-query/src/rules/infinite-query-property-order/infinite-query-property-order.rule.ts +++ b/packages/eslint-plugin-query/src/rules/infinite-query-property-order/infinite-query-property-order.rule.ts @@ -1,107 +1,34 @@ -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 { sortDataByOrder } from './infinite-query-property-order.utils' +import { createPropertyOrderRule } from '../../utils/create-property-order-rule' import { infiniteQueryFunctions, sortRules } from './constants' -import type { InfiniteQueryFunctions } from './constants' -import type { ExtraRuleDocs } from '../../types' - -const createRule = ESLintUtils.RuleCreator(getDocsUrl) - -const infiniteQueryFunctionsSet = new Set(infiniteQueryFunctions) -function isInfiniteQueryFunction(node: any): node is InfiniteQueryFunctions { - return infiniteQueryFunctionsSet.has(node) -} +import type { + InfiniteQueryFunctions, + InfiniteQueryProperties, +} from './constants' export const name = 'infinite-query-property-order' -export const rule = createRule({ - name, - meta: { - type: 'problem', - docs: { - description: - 'Ensure correct order of inference sensitive properties for infinite queries', - recommended: 'error', - }, - messages: { - invalidOrder: 'Invalid order of properties for `{{function}}`.', +export const rule = createPropertyOrderRule< + InfiniteQueryFunctions, + InfiniteQueryProperties +>( + { + name, + meta: { + type: 'problem', + docs: { + description: + 'Ensure correct order of inference sensitive properties for infinite queries', + recommended: 'error', + }, + messages: { + invalidOrder: 'Invalid order of properties for `{{function}}`.', + }, + schema: [], + hasSuggestions: true, + fixable: 'code', }, - schema: [], - hasSuggestions: true, - fixable: 'code', + defaultOptions: [], }, - defaultOptions: [], - - create: detectTanstackQueryImports((context) => { - return { - CallExpression(node) { - if (node.callee.type !== AST_NODE_TYPES.Identifier) { - return - } - const infiniteQueryFunction = node.callee.name - if (!isInfiniteQueryFunction(infiniteQueryFunction)) { - return - } - const argument = node.arguments[0] - if (argument === undefined || argument.type !== 'ObjectExpression') { - return - } - - const allProperties = argument.properties - - // no need to sort if there is at max 1 property - if (allProperties.length < 2) { - return - } - - const properties = allProperties.flatMap((p, index) => { - if ( - p.type === AST_NODE_TYPES.Property && - p.key.type === AST_NODE_TYPES.Identifier - ) { - return { name: p.key.name, property: p } - } else return { name: `_property_${index}`, property: p } - }) - - const sortedProperties = sortDataByOrder(properties, sortRules, 'name') - if (sortedProperties === null) { - return - } - context.report({ - node: argument, - data: { function: node.callee.name }, - messageId: 'invalidOrder', - fix(fixer) { - const sourceCode = context.sourceCode - - const reorderedText = sortedProperties.reduce( - (sourceText, specifier, index) => { - let textBetweenProperties = '' - if (index < allProperties.length - 1) { - textBetweenProperties = sourceCode - .getText() - .slice( - allProperties[index]!.range[1], - allProperties[index + 1]!.range[0], - ) - } - return ( - sourceText + - sourceCode.getText(specifier.property) + - textBetweenProperties - ) - }, - '', - ) - return fixer.replaceTextRange( - [allProperties[0]!.range[0], allProperties.at(-1)!.range[1]], - reorderedText, - ) - }, - }) - }, - } - }), -}) + infiniteQueryFunctions, + sortRules, +) diff --git a/packages/eslint-plugin-query/src/rules/mutation-property-order/constants.ts b/packages/eslint-plugin-query/src/rules/mutation-property-order/constants.ts new file mode 100644 index 0000000000..04c809a694 --- /dev/null +++ b/packages/eslint-plugin-query/src/rules/mutation-property-order/constants.ts @@ -0,0 +1,9 @@ +export const mutationFunctions = ['useMutation'] as const + +export type MutationFunctions = (typeof mutationFunctions)[number] + +export const checkedProperties = ['onMutate', 'onError', 'onSettled'] as const + +export type MutationProperties = (typeof checkedProperties)[number] + +export const sortRules = [[['onMutate'], ['onError', 'onSettled']]] as const diff --git a/packages/eslint-plugin-query/src/rules/mutation-property-order/mutation-property-order.rule.ts b/packages/eslint-plugin-query/src/rules/mutation-property-order/mutation-property-order.rule.ts new file mode 100644 index 0000000000..b4857e12fe --- /dev/null +++ b/packages/eslint-plugin-query/src/rules/mutation-property-order/mutation-property-order.rule.ts @@ -0,0 +1,31 @@ +import { createPropertyOrderRule } from '../../utils/create-property-order-rule' +import { mutationFunctions, sortRules } from './constants' +import type { MutationFunctions, MutationProperties } from './constants' + +export const name = 'mutation-property-order' + +export const rule = createPropertyOrderRule< + MutationFunctions, + MutationProperties +>( + { + name, + meta: { + type: 'problem', + docs: { + description: + 'Ensure correct order of inference-sensitive properties in useMutation()', + recommended: 'error', + }, + messages: { + invalidOrder: 'Invalid order of properties for `{{function}}`.', + }, + schema: [], + hasSuggestions: true, + fixable: 'code', + }, + defaultOptions: [], + }, + mutationFunctions, + sortRules, +) diff --git a/packages/eslint-plugin-query/src/utils/create-property-order-rule.ts b/packages/eslint-plugin-query/src/utils/create-property-order-rule.ts new file mode 100644 index 0000000000..f10f05ac65 --- /dev/null +++ b/packages/eslint-plugin-query/src/utils/create-property-order-rule.ts @@ -0,0 +1,103 @@ +import { AST_NODE_TYPES, ESLintUtils } from '@typescript-eslint/utils' + +import { getDocsUrl } from './get-docs-url' +import { detectTanstackQueryImports } from './detect-react-query-imports' +import { sortDataByOrder } from './sort-data-by-order' +import type { ExtraRuleDocs } from '../types' + +const createRule = ESLintUtils.RuleCreator(getDocsUrl) + +export function createPropertyOrderRule< + TFunc extends string, + TProp extends string, +>( + options: Omit[0], 'create'>, + targetFunctions: ReadonlyArray | Array, + orderRules: ReadonlyArray< + Readonly<[ReadonlyArray, ReadonlyArray]> + >, +) { + const targetFunctionSet = new Set(targetFunctions) + function isTargetFunction(node: any): node is TFunc { + return targetFunctionSet.has(node) + } + + return createRule({ + ...options, + create: detectTanstackQueryImports((context) => { + return { + CallExpression(node) { + if (node.callee.type !== AST_NODE_TYPES.Identifier) { + return + } + const functions = node.callee.name + if (!isTargetFunction(functions)) { + return + } + const argument = node.arguments[0] + if (argument === undefined || argument.type !== 'ObjectExpression') { + return + } + + const allProperties = argument.properties + + // no need to sort if there is at max 1 property + if (allProperties.length < 2) { + return + } + + const properties = allProperties.flatMap((p, index) => { + if ( + p.type === AST_NODE_TYPES.Property && + p.key.type === AST_NODE_TYPES.Identifier + ) { + return { name: p.key.name, property: p } + } else return { name: `_property_${index}`, property: p } + }) + + const sortedProperties = sortDataByOrder( + properties, + orderRules, + 'name', + ) + if (sortedProperties === null) { + return + } + + context.report({ + node: argument, + data: { function: node.callee.name }, + messageId: 'invalidOrder', + fix(fixer) { + const sourceCode = context.sourceCode + + const reorderedText = sortedProperties.reduce( + (sourceText, specifier, index) => { + let textBetweenProperties = '' + if (index < allProperties.length - 1) { + textBetweenProperties = sourceCode + .getText() + .slice( + allProperties[index]!.range[1], + allProperties[index + 1]!.range[0], + ) + } + return ( + sourceText + + sourceCode.getText(specifier.property) + + textBetweenProperties + ) + }, + '', + ) + return fixer.replaceTextRange( + [allProperties[0]!.range[0], allProperties.at(-1)!.range[1]], + reorderedText, + ) + }, + }) + }, + } + }), + }) +} diff --git a/packages/eslint-plugin-query/src/rules/infinite-query-property-order/infinite-query-property-order.utils.ts b/packages/eslint-plugin-query/src/utils/sort-data-by-order.ts similarity index 100% rename from packages/eslint-plugin-query/src/rules/infinite-query-property-order/infinite-query-property-order.utils.ts rename to packages/eslint-plugin-query/src/utils/sort-data-by-order.ts