Skip to content

Commit aca5271

Browse files
committed
fix: consider promise.all() in await-async-utils
1 parent b72ef47 commit aca5271

File tree

5 files changed

+102
-5
lines changed

5 files changed

+102
-5
lines changed

docs/rules/await-async-utils.md

+6
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,12 @@ test('something correctly', async () => {
5959
// return the promise within a function is correct too!
6060
const makeCustomWait = () =>
6161
waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere'));
62+
63+
// using Promise.all combining the methods
64+
await Promise.all([
65+
waitFor(() => getByLabelText('email')),
66+
waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere')),
67+
]);
6268
});
6369
```
6470

jest.config.js

+7
Original file line numberDiff line numberDiff line change
@@ -10,5 +10,12 @@ module.exports = {
1010
lines: 100,
1111
statements: 100,
1212
},
13+
// TODO drop this custom threshold in v4
14+
"./lib/node-utils.ts": {
15+
branches: 90,
16+
functions: 90,
17+
lines: 90,
18+
statements: 90,
19+
}
1320
},
1421
};

lib/node-utils.ts

+12
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@ export function isImportSpecifier(
3030
return node && node.type === AST_NODE_TYPES.ImportSpecifier;
3131
}
3232

33+
export function isImportNamespaceSpecifier(
34+
node: TSESTree.Node
35+
): node is TSESTree.ImportNamespaceSpecifier {
36+
return node?.type === AST_NODE_TYPES.ImportNamespaceSpecifier
37+
}
38+
3339
export function isImportDefaultSpecifier(
3440
node: TSESTree.Node
3541
): node is TSESTree.ImportDefaultSpecifier {
@@ -143,6 +149,12 @@ export function isReturnStatement(
143149
return node && node.type === AST_NODE_TYPES.ReturnStatement;
144150
}
145151

152+
export function isArrayExpression(
153+
node: TSESTree.Node
154+
): node is TSESTree.ArrayExpression {
155+
return node?.type === AST_NODE_TYPES.ArrayExpression
156+
}
157+
146158
export function isAwaited(node: TSESTree.Node) {
147159
return (
148160
isAwaitExpression(node) ||

lib/rules/await-async-utils.ts

+22-4
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,12 @@ import {
55
isAwaited,
66
isPromiseResolved,
77
getVariableReferences,
8+
isMemberExpression,
9+
isImportSpecifier,
10+
isImportNamespaceSpecifier,
11+
isCallExpression,
12+
isArrayExpression,
13+
isIdentifier
814
} from '../node-utils';
915

1016
export const RULE_NAME = 'await-async-utils';
@@ -13,6 +19,17 @@ type Options = [];
1319

1420
const ASYNC_UTILS_REGEXP = new RegExp(`^(${ASYNC_UTILS.join('|')})$`);
1521

22+
// verifies the CallExpression is Promise.all()
23+
function isPromiseAll(node: TSESTree.CallExpression) {
24+
return isMemberExpression(node.callee) && isIdentifier(node.callee.object) && node.callee.object.name === 'Promise' && isIdentifier(node.callee.property) && node.callee.property.name === 'all'
25+
}
26+
27+
// verifies the node is part of an array used in a CallExpression
28+
function isInPromiseAll(node: TSESTree.Node) {
29+
const parent = node.parent
30+
return isCallExpression(parent) && isArrayExpression(parent.parent) && isCallExpression(parent.parent.parent) && isPromiseAll(parent.parent.parent)
31+
}
32+
1633
export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
1734
name: RULE_NAME,
1835
meta: {
@@ -45,11 +62,11 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
4562

4663
if (!LIBRARY_MODULES.includes(parent.source.value.toString())) return;
4764

48-
if (node.type === 'ImportSpecifier') {
65+
if (isImportSpecifier(node)) {
4966
importedAsyncUtils.push(node.imported.name);
5067
}
5168

52-
if (node.type === 'ImportNamespaceSpecifier') {
69+
if (isImportNamespaceSpecifier(node)) {
5370
importedAsyncUtils.push(node.local.name);
5471
}
5572
},
@@ -72,7 +89,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
7289
},
7390
'Program:exit'() {
7491
const testingLibraryUtilUsage = asyncUtilsUsage.filter(usage => {
75-
if (usage.node.type === 'MemberExpression') {
92+
if (isMemberExpression(usage.node)) {
7693
const object = usage.node.object as TSESTree.Identifier;
7794

7895
return importedAsyncUtils.includes(object.name);
@@ -88,7 +105,8 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
88105
references &&
89106
references.length === 0 &&
90107
!isAwaited(node.parent.parent) &&
91-
!isPromiseResolved(node)
108+
!isPromiseResolved(node) &&
109+
!isInPromiseAll(node)
92110
) {
93111
context.report({
94112
node,

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

+55-1
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,50 @@ ruleTester.run(RULE_NAME, rule, {
120120
});
121121
`,
122122
})),
123-
123+
...ASYNC_UTILS.map(asyncUtil => ({
124+
code: `
125+
import { ${asyncUtil} } from '@testing-library/dom';
126+
test('${asyncUtil} util used in with Promise.all() does not trigger an error', async () => {
127+
await Promise.all([
128+
${asyncUtil}(callback1),
129+
${asyncUtil}(callback2),
130+
]);
131+
});
132+
`,
133+
})),
134+
...ASYNC_UTILS.map(asyncUtil => ({
135+
code: `
136+
import { ${asyncUtil} } from '@testing-library/dom';
137+
test('${asyncUtil} util used in with Promise.all() with an await does not trigger an error', async () => {
138+
await Promise.all([
139+
await ${asyncUtil}(callback1),
140+
await ${asyncUtil}(callback2),
141+
]);
142+
});
143+
`,
144+
})),
145+
...ASYNC_UTILS.map(asyncUtil => ({
146+
code: `
147+
import { ${asyncUtil} } from '@testing-library/dom';
148+
test('${asyncUtil} util used in with Promise.all() with ".then" does not trigger an error', async () => {
149+
Promise.all([
150+
${asyncUtil}(callback1),
151+
${asyncUtil}(callback2),
152+
]).then(() => console.log('foo'));
153+
});
154+
`,
155+
})),
156+
{
157+
code: `
158+
import { waitFor, waitForElementToBeRemoved } from '@testing-library/dom';
159+
test('combining different async methods with Promise.all does not throw an error', async () => {
160+
await Promise.all([
161+
waitFor(() => getByLabelText('email')),
162+
waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere')),
163+
])
164+
});
165+
`
166+
},
124167
{
125168
code: `
126169
import { waitForElementToBeRemoved } from '@testing-library/dom';
@@ -139,6 +182,17 @@ ruleTester.run(RULE_NAME, rule, {
139182
});
140183
`,
141184
},
185+
{
186+
code: `
187+
test('using unrelated promises with Promise.all do not throw an error', async () => {
188+
await Promise.all([
189+
someMethod(),
190+
promise1,
191+
await foo().then(() => baz())
192+
])
193+
})
194+
`
195+
}
142196
],
143197
invalid: [
144198
...ASYNC_UTILS.map(asyncUtil => ({

0 commit comments

Comments
 (0)