Skip to content

Commit cedb56b

Browse files
fix: async checks when calling queries member expressions (#114)
Closes #113 * refactor(await-async-query): remove redundant checks While running Program:exit(), nodes were tested again unnecessarily for isAwaited and isPromiseResolved * fix(await-async-query): check queries being called as member expressions First, abstract a isQueryUsage function to check if a node should be checked for this rule. Then, add new selector for queries being called as part of MemberExpression and check their parent against the rule. Finally, use the queryName prop to make sure the right error message is printed * docs(await-async-query): add valid and invalid examples using screen * fix(no-await-sync-query): prevent screen sync queries from using await First, abstract reportError function. Then, create selector for MemberExpression queries. * docs(no-await-sync-query): add valid and invalid examples using screen
1 parent 8c675dc commit cedb56b

File tree

6 files changed

+93
-28
lines changed

6 files changed

+93
-28
lines changed

Diff for: docs/rules/await-async-query.md

+12
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ const bar = () => {
3030
findByText('submit');
3131
// ...
3232
};
33+
34+
const baz = () => {
35+
// ...
36+
screen.findAllByPlaceholderText('name');
37+
// ...
38+
};
3339
```
3440

3541
Examples of **correct** code for this rule:
@@ -50,6 +56,12 @@ const bar = () => {
5056
});
5157
};
5258

59+
const baz = () => {
60+
// ...
61+
await screen.findAllByPlaceholderText('name');
62+
// ...
63+
};
64+
5365
// return the promise within a function is correct too!
5466
const findMyButton = () => findByText('my button');
5567

Diff for: docs/rules/no-await-sync-query.md

+10
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,11 @@ const bar = () => {
2929
// ...
3030
});
3131
};
32+
33+
const baz = () => {
34+
// ...
35+
const button = await screen.getByText('submit');
36+
};
3237
```
3338

3439
Examples of **correct** code for this rule:
@@ -45,6 +50,11 @@ const bar = () => {
4550
const button = getByText('submit');
4651
// ...
4752
};
53+
54+
const baz = () => {
55+
// ...
56+
const button = screen.getByText('submit');
57+
};
4858
```
4959

5060
## Further Reading

Diff for: lib/rules/await-async-query.js

+18-15
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,26 @@ module.exports = {
2828

2929
create: function(context) {
3030
const testingLibraryQueryUsage = [];
31+
const isQueryUsage = node =>
32+
!isAwaited(node.parent.parent) &&
33+
!isPromiseResolved(node) &&
34+
!hasClosestExpectResolvesRejects(node);
35+
3136
return {
3237
[`CallExpression > Identifier[name=${ASYNC_QUERIES_REGEXP}]`](node) {
33-
if (
34-
!isAwaited(node.parent.parent) &&
35-
!isPromiseResolved(node) &&
36-
!hasClosestExpectResolvesRejects(node)
37-
) {
38-
testingLibraryQueryUsage.push(node);
38+
if (isQueryUsage(node)) {
39+
testingLibraryQueryUsage.push({ node, queryName: node.name });
40+
}
41+
},
42+
[`MemberExpression > Identifier[name=${ASYNC_QUERIES_REGEXP}]`](node) {
43+
// Perform checks in parent MemberExpression insted of current identifier
44+
const parent = node.parent;
45+
if (isQueryUsage(parent)) {
46+
testingLibraryQueryUsage.push({ node: parent, queryName: node.name });
3947
}
4048
},
4149
'Program:exit'() {
42-
testingLibraryQueryUsage.forEach(node => {
50+
testingLibraryQueryUsage.forEach(({ node, queryName }) => {
4351
const variableDeclaratorParent = node.parent.parent;
4452

4553
const references =
@@ -49,17 +57,12 @@ module.exports = {
4957
.references.slice(1)) ||
5058
[];
5159

52-
if (
53-
references &&
54-
references.length === 0 &&
55-
!isAwaited(node.parent.parent) &&
56-
!isPromiseResolved(node)
57-
) {
60+
if (references && references.length === 0) {
5861
context.report({
5962
node,
6063
messageId: 'awaitAsyncQuery',
6164
data: {
62-
name: node.name,
65+
name: queryName,
6366
},
6467
});
6568
} else {
@@ -73,7 +76,7 @@ module.exports = {
7376
node,
7477
messageId: 'awaitAsyncQuery',
7578
data: {
76-
name: node.name,
79+
name: queryName,
7780
},
7881
});
7982

Diff for: lib/rules/no-await-sync-query.js

+10-11
Original file line numberDiff line numberDiff line change
@@ -21,18 +21,17 @@ module.exports = {
2121
},
2222

2323
create: function(context) {
24+
const reportError = node =>
25+
context.report({
26+
node,
27+
messageId: 'noAwaitSyncQuery',
28+
data: {
29+
name: node.name,
30+
},
31+
});
2432
return {
25-
[`AwaitExpression > CallExpression > Identifier[name=${SYNC_QUERIES_REGEXP}]`](
26-
node
27-
) {
28-
context.report({
29-
node,
30-
messageId: 'noAwaitSyncQuery',
31-
data: {
32-
name: node.name,
33-
},
34-
});
35-
},
33+
[`AwaitExpression > CallExpression > Identifier[name=${SYNC_QUERIES_REGEXP}]`]: reportError,
34+
[`AwaitExpression > CallExpression > MemberExpression > Identifier[name=${SYNC_QUERIES_REGEXP}]`]: reportError,
3635
};
3736
},
3837
};

Diff for: tests/lib/rules/await-async-query.js

+27
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,14 @@ ruleTester.run('await-async-query', rule, {
2525
`,
2626
})),
2727

28+
// async queries declaration are valid
29+
...ASYNC_QUERIES_COMBINATIONS.map(query => ({
30+
code: `async () => {
31+
await screen.${query}('foo')
32+
}
33+
`,
34+
})),
35+
2836
// async queries with await operator are valid
2937
...ASYNC_QUERIES_COMBINATIONS.map(query => ({
3038
code: `async () => {
@@ -151,6 +159,25 @@ ruleTester.run('await-async-query', rule, {
151159
},
152160
],
153161
})),
162+
...ASYNC_QUERIES_COMBINATIONS.map(query => ({
163+
code: `async () => {
164+
screen.${query}('foo')
165+
}
166+
`,
167+
errors: [
168+
{
169+
messageId: 'awaitAsyncQuery',
170+
},
171+
],
172+
})),
173+
...ASYNC_QUERIES_COMBINATIONS.map(query => ({
174+
code: `const foo = screen.${query}('foo')`,
175+
errors: [
176+
{
177+
messageId: 'awaitAsyncQuery',
178+
},
179+
],
180+
})),
154181
...ASYNC_QUERIES_COMBINATIONS.map(query => ({
155182
code: `async () => {
156183
const foo = ${query}('foo')

Diff for: tests/lib/rules/no-await-sync-query.js

+16-2
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,9 @@ ruleTester.run('no-await-sync-query', rule, {
4343
})),
4444
],
4545

46-
invalid:
46+
invalid: [
4747
// sync queries with await operator are not valid
48-
SYNC_QUERIES_COMBINATIONS.map(query => ({
48+
...SYNC_QUERIES_COMBINATIONS.map(query => ({
4949
code: `async () => {
5050
await ${query}('foo')
5151
}
@@ -56,4 +56,18 @@ ruleTester.run('no-await-sync-query', rule, {
5656
},
5757
],
5858
})),
59+
60+
// sync queries in screen with await operator are not valid
61+
...SYNC_QUERIES_COMBINATIONS.map(query => ({
62+
code: `async () => {
63+
await screen.${query}('foo')
64+
}
65+
`,
66+
errors: [
67+
{
68+
messageId: 'noAwaitSyncQuery',
69+
},
70+
],
71+
})),
72+
],
5973
});

0 commit comments

Comments
 (0)