From 80a49ffd3f51001d0d3ac3e714ef289deb6acc11 Mon Sep 17 00:00:00 2001 From: Eliya Cohen <co.eliya2@gmail.com> Date: Sat, 23 Nov 2024 21:38:42 +0200 Subject: [PATCH] fix(eslint-plugin-query): improve external reference relevance (#8334) * fix: handle external variable in queryFn without failure Adjusted tests to ensure that the rule does not fail when a queryFn inside queryOptions contains a reference to an external variable. Also updated other tests to wrap query calls in a function component for consistency. fix #8326 * ci: apply automated fixes --------- Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> --- .../src/__tests__/exhaustive-deps.test.ts | 137 ++++++++++++------ .../exhaustive-deps/exhaustive-deps.utils.ts | 5 +- .../src/utils/ast-utils.ts | 8 +- 3 files changed, 102 insertions(+), 48 deletions(-) diff --git a/packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts b/packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts index 482fc9dd5c..c7611775b5 100644 --- a/packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts +++ b/packages/eslint-plugin-query/src/__tests__/exhaustive-deps.test.ts @@ -460,6 +460,19 @@ ruleTester.run('exhaustive-deps', rule, { }); `, }, + { + name: 'should not fail when queryFn inside queryOptions contains a reference to an external variable', + code: normalizeIndent` + const EXTERNAL = 1; + + export const queries = { + foo: queryOptions({ + queryKey: ['foo'], + queryFn: () => Promise.resolve(EXTERNAL), + }), + }; + `, + }, ], invalid: [ { @@ -492,8 +505,10 @@ ruleTester.run('exhaustive-deps', rule, { { name: 'should fail when no deps are passed (react)', code: normalizeIndent` - const id = 1; - useQuery({ queryKey: ["entity"], queryFn: () => api.getEntity(id) }); + function Component() { + const id = 1; + useQuery({ queryKey: ["entity"], queryFn: () => api.getEntity(id) }); + } `, errors: [ { @@ -504,8 +519,10 @@ ruleTester.run('exhaustive-deps', rule, { messageId: 'fixTo', data: { result: '["entity", id]' }, output: normalizeIndent` - const id = 1; - useQuery({ queryKey: ["entity", id], queryFn: () => api.getEntity(id) }); + function Component() { + const id = 1; + useQuery({ queryKey: ["entity", id], queryFn: () => api.getEntity(id) }); + } `, }, ], @@ -515,8 +532,10 @@ ruleTester.run('exhaustive-deps', rule, { { name: 'should fail when no deps are passed (solid)', code: normalizeIndent` - const id = 1; - createQuery({ queryKey: ["entity"], queryFn: () => api.getEntity(id) }); + function Component() { + const id = 1; + createQuery({ queryKey: ["entity"], queryFn: () => api.getEntity(id) }); + } `, errors: [ { @@ -527,8 +546,10 @@ ruleTester.run('exhaustive-deps', rule, { messageId: 'fixTo', data: { result: '["entity", id]' }, output: normalizeIndent` - const id = 1; - createQuery({ queryKey: ["entity", id], queryFn: () => api.getEntity(id) }); + function Component() { + const id = 1; + createQuery({ queryKey: ["entity", id], queryFn: () => api.getEntity(id) }); + } `, }, ], @@ -538,8 +559,10 @@ ruleTester.run('exhaustive-deps', rule, { { name: 'should fail when deps are passed incorrectly', code: normalizeIndent` - const id = 1; - useQuery({ queryKey: ["entity/\${id}"], queryFn: () => api.getEntity(id) }); + function Component() { + const id = 1; + useQuery({ queryKey: ["entity/\${id}"], queryFn: () => api.getEntity(id) }); + } `, errors: [ { @@ -550,8 +573,10 @@ ruleTester.run('exhaustive-deps', rule, { messageId: 'fixTo', data: { result: '["entity/${id}", id]' }, output: normalizeIndent` - const id = 1; - useQuery({ queryKey: ["entity/\${id}", id], queryFn: () => api.getEntity(id) }); + function Component() { + const id = 1; + useQuery({ queryKey: ["entity/\${id}", id], queryFn: () => api.getEntity(id) }); + } `, }, ], @@ -561,9 +586,11 @@ ruleTester.run('exhaustive-deps', rule, { { name: 'should pass missing dep while key has a template literal', code: normalizeIndent` - const a = 1; - const b = 2; - useQuery({ queryKey: [\`entity/\${a}\`], queryFn: () => api.getEntity(a, b) }); + function Component() { + const a = 1; + const b = 2; + useQuery({ queryKey: [\`entity/\${a}\`], queryFn: () => api.getEntity(a, b) }); + } `, errors: [ { @@ -574,9 +601,11 @@ ruleTester.run('exhaustive-deps', rule, { messageId: 'fixTo', data: { result: '[`entity/${a}`, b]' }, output: normalizeIndent` - const a = 1; - const b = 2; - useQuery({ queryKey: [\`entity/\${a}\`, b], queryFn: () => api.getEntity(a, b) }); + function Component() { + const a = 1; + const b = 2; + useQuery({ queryKey: [\`entity/\${a}\`, b], queryFn: () => api.getEntity(a, b) }); + } `, }, ], @@ -586,14 +615,16 @@ ruleTester.run('exhaustive-deps', rule, { { name: 'should fail when dep exists inside setter and missing in queryKey', code: normalizeIndent` - const [id] = React.useState(1); - useQuery({ + function Component() { + const [id] = React.useState(1); + useQuery({ queryKey: ["entity"], queryFn: () => { - const { data } = axios.get(\`.../\${id}\`); - return data; + const { data } = axios.get(\`.../\${id}\`); + return data; } - }); + }); + } `, errors: [ { @@ -604,14 +635,16 @@ ruleTester.run('exhaustive-deps', rule, { messageId: 'fixTo', data: { result: '["entity", id]' }, output: normalizeIndent` - const [id] = React.useState(1); - useQuery({ + function Component() { + const [id] = React.useState(1); + useQuery({ queryKey: ["entity", id], queryFn: () => { - const { data } = axios.get(\`.../\${id}\`); - return data; + const { data } = axios.get(\`.../\${id}\`); + return data; } - }); + }); + } `, }, ], @@ -713,9 +746,11 @@ ruleTester.run('exhaustive-deps', rule, { { name: 'should fail when a queryKey is a reference of an array expression with a missing dep', code: normalizeIndent` - const x = 5; - const queryKey = ['foo'] - useQuery({ queryKey, queryFn: () => x }) + function Component() { + const x = 5; + const queryKey = ['foo'] + useQuery({ queryKey, queryFn: () => x }) + } `, errors: [ { @@ -728,9 +763,11 @@ ruleTester.run('exhaustive-deps', rule, { result: "['foo', x]", }, output: normalizeIndent` - const x = 5; - const queryKey = ['foo', x] - useQuery({ queryKey, queryFn: () => x }) + function Component() { + const x = 5; + const queryKey = ['foo', x] + useQuery({ queryKey, queryFn: () => x }) + } `, }, ], @@ -758,12 +795,14 @@ ruleTester.run('exhaustive-deps', rule, { { name: 'should fail if queryFn is using multiple object props when only one of them is in the queryKey', code: normalizeIndent` - const state = { foo: 'foo', bar: 'bar' } + function Component() { + const state = { foo: 'foo', bar: 'bar' } - useQuery({ + useQuery({ queryKey: ['state', state.foo], queryFn: () => Promise.resolve({ foo: state.foo, bar: state.bar }) - }) + }) + } `, errors: [ { @@ -771,12 +810,14 @@ ruleTester.run('exhaustive-deps', rule, { { messageId: 'fixTo', output: normalizeIndent` - const state = { foo: 'foo', bar: 'bar' } + function Component() { + const state = { foo: 'foo', bar: 'bar' } - useQuery({ + useQuery({ queryKey: ['state', state.foo, state.bar], queryFn: () => Promise.resolve({ foo: state.foo, bar: state.bar }) - }) + }) + } `, }, ], @@ -788,14 +829,16 @@ ruleTester.run('exhaustive-deps', rule, { { name: 'should fail if queryFn is invalid while using FunctionExpression syntax', code: normalizeIndent` - const id = 1; + function Component() { + const id = 1; - useQuery({ + useQuery({ queryKey: [], queryFn() { Promise.resolve(id) } - }) + }); + } `, errors: [ { @@ -803,14 +846,16 @@ ruleTester.run('exhaustive-deps', rule, { { messageId: 'fixTo', output: normalizeIndent` - const id = 1; + function Component() { + const id = 1; - useQuery({ + useQuery({ queryKey: [id], queryFn() { Promise.resolve(id) } - }) + }); + } `, }, ], diff --git a/packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts b/packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts index b87952c75c..5c45d1d0e8 100644 --- a/packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts +++ b/packages/eslint-plugin-query/src/rules/exhaustive-deps/exhaustive-deps.utils.ts @@ -12,8 +12,11 @@ export const ExhaustiveDepsUtils = { const { sourceCode, reference, scopeManager, node } = params const component = ASTUtils.getFunctionAncestor(sourceCode, node) + if (component === undefined) { + return false + } + if ( - component !== undefined && !ASTUtils.isDeclaredInNode({ scopeManager, reference, diff --git a/packages/eslint-plugin-query/src/utils/ast-utils.ts b/packages/eslint-plugin-query/src/utils/ast-utils.ts index b5e5dcf0d4..df489bdc18 100644 --- a/packages/eslint-plugin-query/src/utils/ast-utils.ts +++ b/packages/eslint-plugin-query/src/utils/ast-utils.ts @@ -241,7 +241,13 @@ export const ASTUtils = { node: TSESTree.Node, ) { for (const ancestor of sourceCode.getAncestors(node)) { - if (ancestor.type === AST_NODE_TYPES.FunctionDeclaration) { + if ( + ASTUtils.isNodeOfOneOf(ancestor, [ + AST_NODE_TYPES.FunctionDeclaration, + AST_NODE_TYPES.FunctionExpression, + AST_NODE_TYPES.ArrowFunctionExpression, + ]) + ) { return ancestor }