Skip to content

Commit 84a8ff1

Browse files
Newbie012lachlancollins
Newbie012
and
lachlancollins
authored
fix: improve exhaustive dependencies detection (#10258)
* fix: improve exhaustive-deps to detect nested callback/control flow dependencies * fix: detect callee objects as dependencies in exhaustive-deps rule --------- Co-authored-by: Lachlan Collins <1667261+lachlancollins@users.noreply.github.com>
1 parent 6bcf9bf commit 84a8ff1

File tree

5 files changed

+520
-29
lines changed

5 files changed

+520
-29
lines changed

‎.changeset/perky-seas-flash.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@tanstack/eslint-plugin-query': patch
3+
---
4+
5+
Fix `exhaustive-deps` to detect dependencies used inside nested `queryFn` callbacks/control flow, and avoid false positives when those dependencies are already present in complex `queryKey` expressions.

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

Lines changed: 329 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -34,20 +34,6 @@ ruleTester.run('exhaustive-deps', rule, {
3434
name: 'should not pass api.entity.get',
3535
code: 'useQuery({ queryKey: ["entity", id], queryFn: () => api.entity.get(id) });',
3636
},
37-
{
38-
name: 'should not pass api when is being used for calling a function',
39-
code: `
40-
import useApi from './useApi'
41-
42-
const useFoo = () => {
43-
const api = useApi();
44-
return useQuery({
45-
queryKey: ['foo'],
46-
queryFn: () => api.fetchFoo(),
47-
})
48-
}
49-
`,
50-
},
5137
{
5238
name: 'should pass props.src',
5339
code: `
@@ -259,6 +245,22 @@ ruleTester.run('exhaustive-deps', rule, {
259245
}
260246
`,
261247
},
248+
{
249+
name: 'should pass when queryKey is a chained queryKeyFactory while having deps in nested calls',
250+
code: normalizeIndent`
251+
const fooQueryKeyFactory = {
252+
foo: (num: number) => ({
253+
detail: (flag: boolean) => ['foo', num, flag] as const,
254+
}),
255+
}
256+
257+
const useFoo = (num: number, flag: boolean) =>
258+
useQuery({
259+
queryKey: fooQueryKeyFactory.foo(num).detail(flag),
260+
queryFn: () => Promise.resolve({ num, flag }),
261+
})
262+
`,
263+
},
262264
{
263265
name: 'should not treat new Error as missing dependency',
264266
code: normalizeIndent`
@@ -423,6 +425,50 @@ ruleTester.run('exhaustive-deps', rule, {
423425
}
424426
`,
425427
},
428+
{
429+
name: 'should pass when queryKey uses a direct conditional expression',
430+
code: normalizeIndent`
431+
function Component(cond, a, b) {
432+
useQuery({
433+
queryKey: ['thing', cond ? a : b],
434+
queryFn: () => (cond ? a : b),
435+
})
436+
}
437+
`,
438+
},
439+
{
440+
name: 'should pass when queryKey uses a direct binary expression',
441+
code: normalizeIndent`
442+
function Component(a, b) {
443+
useQuery({
444+
queryKey: ['thing', a + b],
445+
queryFn: () => a + b,
446+
})
447+
}
448+
`,
449+
},
450+
{
451+
name: 'should pass when queryKey uses a nested type assertion',
452+
code: normalizeIndent`
453+
function Component(dep) {
454+
useQuery({
455+
queryKey: ['thing', dep as string],
456+
queryFn: () => dep,
457+
})
458+
}
459+
`,
460+
},
461+
{
462+
name: 'should pass when queryKey derives values inside a callback',
463+
code: normalizeIndent`
464+
function Component(ids, prefix) {
465+
useQuery({
466+
queryKey: ['thing', ids.map((id) => prefix + '-' + id)],
467+
queryFn: () => ({ ids, prefix }),
468+
})
469+
}
470+
`,
471+
},
426472
{
427473
name: 'instanceof value should not be in query key',
428474
code: `
@@ -587,8 +633,94 @@ ruleTester.run('exhaustive-deps', rule, {
587633
})
588634
`,
589635
},
636+
{
637+
name: 'should ignore callback locals in Vue file queryFn',
638+
filename: 'Component.vue',
639+
code: normalizeIndent`
640+
import { useQuery } from '@tanstack/vue-query'
641+
642+
const ids = [1, 2, 3]
643+
useQuery({
644+
queryKey: ['entities', ids],
645+
queryFn: () => ids.map((id) => fetchEntity(id)),
646+
})
647+
`,
648+
},
649+
{
650+
name: 'should pass when dep used in then/catch is listed in queryKey',
651+
code: normalizeIndent`
652+
function Component() {
653+
const id = 1
654+
useQuery({
655+
queryKey: ['foo', id],
656+
queryFn: () =>
657+
Promise.resolve(null)
658+
.then(() => id)
659+
.catch(() => id),
660+
})
661+
}
662+
`,
663+
},
664+
{
665+
name: 'should pass when dep used in try/catch/finally is listed in queryKey',
666+
code: normalizeIndent`
667+
function Component() {
668+
const id = 1
669+
useQuery({
670+
queryKey: ['foo', id],
671+
queryFn: () => {
672+
try {
673+
return fetch(id)
674+
} catch (error) {
675+
console.error(error)
676+
return id
677+
} finally {
678+
console.log('done')
679+
}
680+
},
681+
})
682+
}
683+
`,
684+
},
590685
],
591686
invalid: [
687+
{
688+
name: 'should fail when api from hook is used for calling a function',
689+
code: normalizeIndent`
690+
import useApi from './useApi'
691+
692+
const useFoo = () => {
693+
const api = useApi();
694+
return useQuery({
695+
queryKey: ['foo'],
696+
queryFn: () => api.fetchFoo(),
697+
})
698+
}
699+
`,
700+
errors: [
701+
{
702+
messageId: 'missingDeps',
703+
data: { deps: 'api' },
704+
suggestions: [
705+
{
706+
messageId: 'fixTo',
707+
data: { result: "['foo', api]" },
708+
output: normalizeIndent`
709+
import useApi from './useApi'
710+
711+
const useFoo = () => {
712+
const api = useApi();
713+
return useQuery({
714+
queryKey: ['foo', api],
715+
queryFn: () => api.fetchFoo(),
716+
})
717+
}
718+
`,
719+
},
720+
],
721+
},
722+
],
723+
},
592724
{
593725
name: 'should fail when deps are missing in query factory',
594726
code: normalizeIndent`
@@ -888,6 +1020,49 @@ ruleTester.run('exhaustive-deps', rule, {
8881020
},
8891021
],
8901022
},
1023+
{
1024+
name: 'should fail when alias of props used in queryFn is missing in queryKey',
1025+
code: normalizeIndent`
1026+
function Component(props) {
1027+
const entities = props.entities;
1028+
1029+
const q = useQuery({
1030+
queryKey: ['get-stuff'],
1031+
queryFn: () => {
1032+
return api.fetchStuff({
1033+
ids: entities.map((o) => o.id)
1034+
});
1035+
}
1036+
});
1037+
}
1038+
`,
1039+
errors: [
1040+
{
1041+
messageId: 'missingDeps',
1042+
data: { deps: 'entities' },
1043+
suggestions: [
1044+
{
1045+
messageId: 'fixTo',
1046+
data: { result: "['get-stuff', entities]" },
1047+
output: normalizeIndent`
1048+
function Component(props) {
1049+
const entities = props.entities;
1050+
1051+
const q = useQuery({
1052+
queryKey: ['get-stuff', entities],
1053+
queryFn: () => {
1054+
return api.fetchStuff({
1055+
ids: entities.map((o) => o.id)
1056+
});
1057+
}
1058+
});
1059+
}
1060+
`,
1061+
},
1062+
],
1063+
},
1064+
],
1065+
},
8911066
{
8921067
name: 'should fail when queryKey is a queryKeyFactory while having missing dep',
8931068
code: normalizeIndent`
@@ -906,6 +1081,28 @@ ruleTester.run('exhaustive-deps', rule, {
9061081
},
9071082
],
9081083
},
1084+
{
1085+
name: 'should fail when queryKey is a chained queryKeyFactory while having missing dep in earlier call',
1086+
code: normalizeIndent`
1087+
const fooQueryKeyFactory = {
1088+
foo: (num: number) => ({
1089+
detail: (flag: boolean) => ['foo', num, flag] as const,
1090+
}),
1091+
}
1092+
1093+
const useFoo = (num: number, flag: boolean) =>
1094+
useQuery({
1095+
queryKey: fooQueryKeyFactory.foo(1).detail(flag),
1096+
queryFn: () => Promise.resolve({ num, flag }),
1097+
})
1098+
`,
1099+
errors: [
1100+
{
1101+
messageId: 'missingDeps',
1102+
data: { deps: 'num' },
1103+
},
1104+
],
1105+
},
9091106
{
9101107
name: 'should fail if queryFn is using multiple object props when only one of them is in the queryKey',
9111108
code: normalizeIndent`
@@ -1084,5 +1281,123 @@ ruleTester.run('exhaustive-deps', rule, {
10841281
},
10851282
],
10861283
},
1284+
{
1285+
name: 'should fail when dep used in then/catch is missing in queryKey',
1286+
code: normalizeIndent`
1287+
function Component() {
1288+
const id = 1
1289+
useQuery({
1290+
queryKey: ['foo'],
1291+
queryFn: () =>
1292+
Promise.resolve(null)
1293+
.then(() => id)
1294+
.catch(() => id),
1295+
})
1296+
}
1297+
`,
1298+
errors: [
1299+
{
1300+
messageId: 'missingDeps',
1301+
data: { deps: 'id' },
1302+
suggestions: [
1303+
{
1304+
messageId: 'fixTo',
1305+
output: normalizeIndent`
1306+
function Component() {
1307+
const id = 1
1308+
useQuery({
1309+
queryKey: ['foo', id],
1310+
queryFn: () =>
1311+
Promise.resolve(null)
1312+
.then(() => id)
1313+
.catch(() => id),
1314+
})
1315+
}
1316+
`,
1317+
},
1318+
],
1319+
},
1320+
],
1321+
},
1322+
{
1323+
name: 'should fail when queryKey callback only references a shadowing local',
1324+
code: normalizeIndent`
1325+
function Component(id, ids) {
1326+
useQuery({
1327+
queryKey: ['thing', ids.map((id) => id)],
1328+
queryFn: () => id,
1329+
})
1330+
}
1331+
`,
1332+
errors: [
1333+
{
1334+
messageId: 'missingDeps',
1335+
data: { deps: 'id' },
1336+
suggestions: [
1337+
{
1338+
messageId: 'fixTo',
1339+
output: normalizeIndent`
1340+
function Component(id, ids) {
1341+
useQuery({
1342+
queryKey: ['thing', ids.map((id) => id), id],
1343+
queryFn: () => id,
1344+
})
1345+
}
1346+
`,
1347+
},
1348+
],
1349+
},
1350+
],
1351+
},
1352+
{
1353+
name: 'should fail when dep used in try/catch/finally is missing in queryKey',
1354+
code: normalizeIndent`
1355+
function Component() {
1356+
const id = 1
1357+
useQuery({
1358+
queryKey: ['foo'],
1359+
queryFn: () => {
1360+
try {
1361+
return fetch(id)
1362+
} catch (error) {
1363+
console.error(error)
1364+
return id
1365+
} finally {
1366+
console.log('done')
1367+
}
1368+
},
1369+
})
1370+
}
1371+
`,
1372+
errors: [
1373+
{
1374+
messageId: 'missingDeps',
1375+
data: { deps: 'id' },
1376+
suggestions: [
1377+
{
1378+
messageId: 'fixTo',
1379+
output: normalizeIndent`
1380+
function Component() {
1381+
const id = 1
1382+
useQuery({
1383+
queryKey: ['foo', id],
1384+
queryFn: () => {
1385+
try {
1386+
return fetch(id)
1387+
} catch (error) {
1388+
console.error(error)
1389+
return id
1390+
} finally {
1391+
console.log('done')
1392+
}
1393+
},
1394+
})
1395+
}
1396+
`,
1397+
},
1398+
],
1399+
},
1400+
],
1401+
},
10871402
],
10881403
})

0 commit comments

Comments
 (0)