Skip to content

Commit deab1c4

Browse files
committed
fix(await-async-query): work around false-positives in chained findBy queries
1 parent b1eca80 commit deab1c4

File tree

2 files changed

+155
-3
lines changed

2 files changed

+155
-3
lines changed

lib/rules/await-async-query.ts

+129-3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ import {
88
getInnermostReturningFunction,
99
getVariableReferences,
1010
isPromiseHandled,
11+
isMemberExpression,
12+
isCallExpression,
1113
} from '../node-utils';
1214

1315
export const RULE_NAME = 'await-async-query';
@@ -39,13 +41,128 @@ export default createTestingLibraryRule<Options, MessageIds>({
3941
defaultOptions: [],
4042

4143
create(context, _, helpers) {
42-
const functionWrappersNames: string[] = [];
44+
const functionWrappersNamesSync: string[] = [];
45+
const functionWrappersNamesAsync: string[] = [];
46+
47+
function detectSyncQueryWrapper(node: TSESTree.Identifier) {
48+
const innerFunction = getInnermostReturningFunction(context, node);
49+
if (innerFunction) {
50+
functionWrappersNamesSync.push(getFunctionName(innerFunction));
51+
}
52+
}
4353

4454
function detectAsyncQueryWrapper(node: TSESTree.Identifier) {
4555
const innerFunction = getInnermostReturningFunction(context, node);
4656
if (innerFunction) {
47-
functionWrappersNames.push(getFunctionName(innerFunction));
57+
functionWrappersNamesAsync.push(getFunctionName(innerFunction));
58+
}
59+
}
60+
61+
function resolveVariable(
62+
node: TSESTree.Node,
63+
scope: ReturnType<typeof context['getScope']> | null
64+
): TSESTree.Node | null {
65+
if (scope == null) {
66+
return null;
67+
}
68+
69+
if (node.type === 'Identifier') {
70+
const variable = scope.variables.find(({ name }) => name === node.name);
71+
72+
// variable not found in this scope, so recursively check parent scope(s) for definition
73+
if (variable == null) {
74+
return resolveVariable(node, scope.upper);
75+
}
76+
77+
if (variable.defs.length === 0) {
78+
return null;
79+
}
80+
81+
const result = variable.defs[variable.defs.length - 1].node;
82+
83+
if (!ASTUtils.isVariableDeclarator(result)) {
84+
return null;
85+
}
86+
87+
return result.init;
88+
}
89+
90+
return node;
91+
}
92+
93+
// true in cases like:
94+
// - getByText('foo').findByType(SomeType)
95+
// - (await findByText('foo')).findByType(SomeType)
96+
// - const variable = await findByText('foo'); variable.findByType(SomeType)
97+
// - function helper() { return screen.getByText('foo'); }; helper().findByType(SomeType)
98+
function hasQueryResultInChain(node: TSESTree.Node): boolean {
99+
if (ASTUtils.isIdentifier(node)) {
100+
return false;
101+
}
102+
103+
if (ASTUtils.isAwaitExpression(node)) {
104+
// great, we have an inline await, so let's check if it's a query
105+
const identifierNode = getDeepestIdentifierNode(node);
106+
107+
if (!identifierNode) {
108+
return false;
109+
}
110+
111+
if (
112+
helpers.isAsyncQuery(identifierNode) &&
113+
isPromiseHandled(identifierNode)
114+
) {
115+
return true;
116+
}
117+
118+
if (
119+
functionWrappersNamesAsync.includes(identifierNode.name) &&
120+
isPromiseHandled(identifierNode)
121+
) {
122+
return true;
123+
}
124+
125+
return false;
126+
}
127+
128+
if (isMemberExpression(node)) {
129+
// check inline sync query (e.g. foo.getByText(...) checks `getByText`)
130+
if (
131+
ASTUtils.isIdentifier(node.property) &&
132+
helpers.isSyncQuery(node.property)
133+
) {
134+
return true;
135+
}
136+
137+
// check sync query reference (e.g. foo.getByText(...) checks `foo` is defined elsewhere)
138+
if (ASTUtils.isIdentifier(node.object)) {
139+
const definition = resolveVariable(node.object, context.getScope());
140+
141+
if (definition == null) {
142+
return false;
143+
}
144+
145+
return hasQueryResultInChain(definition);
146+
}
147+
148+
// check sync query reference (e.g. foo().getByText(...) checks `foo` is defined elsewhere)
149+
if (isCallExpression(node.object)) {
150+
if (
151+
ASTUtils.isIdentifier(node.object.callee) &&
152+
functionWrappersNamesSync.includes(node.object.callee.name)
153+
) {
154+
return true;
155+
}
156+
}
157+
158+
return hasQueryResultInChain(node.object);
159+
}
160+
161+
if (isCallExpression(node)) {
162+
return hasQueryResultInChain(node.callee);
48163
}
164+
165+
return false;
49166
}
50167

51168
return {
@@ -56,6 +173,10 @@ export default createTestingLibraryRule<Options, MessageIds>({
56173
return;
57174
}
58175

176+
if (helpers.isSyncQuery(identifierNode)) {
177+
detectSyncQueryWrapper(identifierNode);
178+
}
179+
59180
if (helpers.isAsyncQuery(identifierNode)) {
60181
// detect async query used within wrapper function for later analysis
61182
detectAsyncQueryWrapper(identifierNode);
@@ -69,6 +190,11 @@ export default createTestingLibraryRule<Options, MessageIds>({
69190
return;
70191
}
71192

193+
// check chained usage for an instance of sync query, which means this might be a false positive from react-test-renderer
194+
if (hasQueryResultInChain(closestCallExpressionNode)) {
195+
return;
196+
}
197+
72198
const references = getVariableReferences(
73199
context,
74200
closestCallExpressionNode.parent
@@ -104,7 +230,7 @@ export default createTestingLibraryRule<Options, MessageIds>({
104230
}
105231
}
106232
} else if (
107-
functionWrappersNames.includes(identifierNode.name) &&
233+
functionWrappersNamesAsync.includes(identifierNode.name) &&
108234
!isPromiseHandled(identifierNode)
109235
) {
110236
// check async queries used within a wrapper previously detected

tests/lib/rules/await-async-query.test.ts

+26
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,32 @@ ruleTester.run(RULE_NAME, rule, {
345345
// valid async query usage without any function defined
346346
// so there is no innermost function scope found
347347
`const element = await findByRole('button')`,
348+
349+
// react-test-renderer provided `findBy*` queries should be ignored
350+
// https://github.com/testing-library/eslint-plugin-testing-library/issues/673
351+
{
352+
code: `// issue #673
353+
test('this is a valid case', async () => {
354+
const screen = renderContainer()
355+
356+
const syncCall = screen.getByRole('button')
357+
const asyncCall = await screen.findByRole('button')
358+
359+
const syncCall2 = () => { return screen.getByRole('button') }
360+
const asyncCall2 = async () => screen.findByRole('button')
361+
362+
const thing1 = await screen.findByText('foo')
363+
const thing2 = screen.getByText('foo').findByProps({ testID: 'bar' })
364+
const thing3 = (await screen.thing.findByText('foo')).findByProps({ testID: 'bar' })
365+
const thing3a = syncCall.findByProps({ testID: 'bar' })
366+
const thing3b = asyncCall.findByProps({ testID: 'bar' })
367+
const thing3a2 = syncCall2().findByProps({ testID: 'bar' })
368+
const thing3b2 = (await asyncCall2()).findByProps({ testID: 'bar' })
369+
const thing4 = screen.getAllByText('foo')[0].findByProps({ testID: 'bar' })
370+
const thing5 = screen.getAllByText('foo').filter(value => true)[0].findByProps({ testID: 'bar' })
371+
})
372+
`,
373+
},
348374
],
349375

350376
invalid: [

0 commit comments

Comments
 (0)