fix(await-async-utils): consider promise.all (#236)

Closes #227
testing-library · Oct 12, 2020 · 6ce0140 · 6ce0140
1 parent b72ef47
commit 6ce0140
Show file tree

Hide file tree

Showing 5 changed files with 102 additions and 5 deletions.
diff --git a/docs/rules/await-async-utils.md b/docs/rules/await-async-utils.md
@@ -59,6 +59,12 @@ test('something correctly', async () => {
   // return the promise within a function is correct too!
   const makeCustomWait = () =>
     waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere'));
+
+  // using Promise.all combining the methods
+  await Promise.all([
+    waitFor(() => getByLabelText('email')),
+    waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere')),
+  ]);
 });
 ```
 

diff --git a/jest.config.js b/jest.config.js
@@ -10,5 +10,12 @@ module.exports = {
       lines: 100,
       statements: 100,
     },
+    // TODO drop this custom threshold in v4
+    "./lib/node-utils.ts": {
+      branches: 90,
+      functions: 90,
+      lines: 90,
+      statements: 90,
+    }
   },
 };
diff --git a/lib/node-utils.ts b/lib/node-utils.ts
@@ -30,6 +30,12 @@ export function isImportSpecifier(
   return node && node.type === AST_NODE_TYPES.ImportSpecifier;
 }
 
+export function isImportNamespaceSpecifier(
+  node: TSESTree.Node
+): node is TSESTree.ImportNamespaceSpecifier {
+  return node?.type === AST_NODE_TYPES.ImportNamespaceSpecifier
+}
+
 export function isImportDefaultSpecifier(
   node: TSESTree.Node
 ): node is TSESTree.ImportDefaultSpecifier {
@@ -143,6 +149,12 @@ export function isReturnStatement(
   return node && node.type === AST_NODE_TYPES.ReturnStatement;
 }
 
+export function isArrayExpression(
+  node: TSESTree.Node
+): node is TSESTree.ArrayExpression {
+  return node?.type === AST_NODE_TYPES.ArrayExpression
+}
+
 export function isAwaited(node: TSESTree.Node) {
   return (
     isAwaitExpression(node) ||

diff --git a/lib/rules/await-async-utils.ts b/lib/rules/await-async-utils.ts
@@ -5,6 +5,12 @@ import {
   isAwaited,
   isPromiseResolved,
   getVariableReferences,
+  isMemberExpression,
+  isImportSpecifier,
+  isImportNamespaceSpecifier,
+  isCallExpression,
+  isArrayExpression,
+  isIdentifier
 } from '../node-utils';
 
 export const RULE_NAME = 'await-async-utils';
@@ -13,6 +19,17 @@ type Options = [];
 
 const ASYNC_UTILS_REGEXP = new RegExp(`^(${ASYNC_UTILS.join('|')})$`);
 
+// verifies the CallExpression is Promise.all()
+function isPromiseAll(node: TSESTree.CallExpression) {
+  return isMemberExpression(node.callee) && isIdentifier(node.callee.object) && node.callee.object.name === 'Promise' && isIdentifier(node.callee.property) && node.callee.property.name === 'all'
+}
+
+// verifies the node is part of an array used in a CallExpression
+function isInPromiseAll(node: TSESTree.Node) {
+  const parent = node.parent
+  return isCallExpression(parent) && isArrayExpression(parent.parent) && isCallExpression(parent.parent.parent) && isPromiseAll(parent.parent.parent)
+}
+
 export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
   name: RULE_NAME,
   meta: {
@@ -45,11 +62,11 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
 
         if (!LIBRARY_MODULES.includes(parent.source.value.toString())) return;
 
-        if (node.type === 'ImportSpecifier') {
+        if (isImportSpecifier(node)) {
           importedAsyncUtils.push(node.imported.name);
         }
 
-        if (node.type === 'ImportNamespaceSpecifier') {
+        if (isImportNamespaceSpecifier(node)) {
           importedAsyncUtils.push(node.local.name);
         }
       },
@@ -72,7 +89,7 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
       },
       'Program:exit'() {
         const testingLibraryUtilUsage = asyncUtilsUsage.filter(usage => {
-          if (usage.node.type === 'MemberExpression') {
+          if (isMemberExpression(usage.node)) {
             const object = usage.node.object as TSESTree.Identifier;
 
             return importedAsyncUtils.includes(object.name);
@@ -88,7 +105,8 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
             references &&
             references.length === 0 &&
             !isAwaited(node.parent.parent) &&
-            !isPromiseResolved(node)
+            !isPromiseResolved(node) &&
+            !isInPromiseAll(node)
           ) {
             context.report({
               node,

diff --git a/tests/lib/rules/await-async-utils.test.ts b/tests/lib/rules/await-async-utils.test.ts
@@ -120,7 +120,50 @@ ruleTester.run(RULE_NAME, rule, {
         });
       `,
     })),
-
+    ...ASYNC_UTILS.map(asyncUtil => ({
+      code: `
+        import { ${asyncUtil} } from '@testing-library/dom';
+        test('${asyncUtil} util used in with Promise.all() does not trigger an error', async () => {
+          await Promise.all([
+            ${asyncUtil}(callback1),
+            ${asyncUtil}(callback2),
+          ]);
+        });
+      `,
+    })),
+    ...ASYNC_UTILS.map(asyncUtil => ({
+      code: `
+        import { ${asyncUtil} } from '@testing-library/dom';
+        test('${asyncUtil} util used in with Promise.all() with an await does not trigger an error', async () => {
+          await Promise.all([
+            await ${asyncUtil}(callback1),
+            await ${asyncUtil}(callback2),
+          ]);
+        });
+      `,
+    })),
+    ...ASYNC_UTILS.map(asyncUtil => ({
+      code: `
+        import { ${asyncUtil} } from '@testing-library/dom';
+        test('${asyncUtil} util used in with Promise.all() with ".then" does not trigger an error', async () => {
+          Promise.all([
+            ${asyncUtil}(callback1),
+            ${asyncUtil}(callback2),
+          ]).then(() => console.log('foo'));
+        });
+      `,
+    })),
+    {
+      code: `
+        import { waitFor, waitForElementToBeRemoved } from '@testing-library/dom';
+        test('combining different async methods with Promise.all does not throw an error', async () => {
+          await Promise.all([
+            waitFor(() => getByLabelText('email')),
+            waitForElementToBeRemoved(() => document.querySelector('div.getOuttaHere')),
+          ])
+        });
+      `
+    },
     {
       code: `
         import { waitForElementToBeRemoved } from '@testing-library/dom';
@@ -139,6 +182,17 @@ ruleTester.run(RULE_NAME, rule, {
         });
       `,
     },
+    {
+      code: `
+      test('using unrelated promises with Promise.all do not throw an error', async () => {
+        await Promise.all([
+          someMethod(),
+          promise1,
+          await foo().then(() => baz())
+        ])
+      })
+      `
+    }
   ],
   invalid: [
     ...ASYNC_UTILS.map(asyncUtil => ({