Skip to content

Commit 5e7b9a3

Browse files
puglyfeCharley
andauthored
fix: catch handles promise (#1059)
Closes #1058 Co-authored-by: Charley <[email protected]>
1 parent ab45896 commit 5e7b9a3

File tree

6 files changed

+55
-9
lines changed

6 files changed

+55
-9
lines changed

docs/rules/await-async-queries.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ problems in the tests. The promise will be considered as handled when:
2121

2222
- using the `await` operator
2323
- wrapped within `Promise.all` or `Promise.allSettled` methods
24-
- chaining the `then` method
24+
- chaining the `then` or `catch` method
2525
- chaining `resolves` or `rejects` from jest
2626
- chaining `toResolve()` or `toReject()` from [jest-extended](https://github.com/jest-community/jest-extended#promise)
2727
- chaining jasmine [async matchers](https://jasmine.github.io/api/edge/async-matchers.html)

docs/rules/await-async-utils.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ problems in the tests. The promise will be considered as handled when:
1919

2020
- using the `await` operator
2121
- wrapped within `Promise.all` or `Promise.allSettled` methods
22-
- chaining the `then` method
22+
- chaining the `then` or `catch` method
2323
- chaining `resolves` or `rejects` from jest
2424
- chaining `toResolve()` or `toReject()` from [jest-extended](https://github.com/jest-community/jest-extended#promise)
2525
- chaining jasmine [async matchers](https://jasmine.github.io/api/edge/async-matchers.html)

lib/node-utils/index.ts

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -151,16 +151,24 @@ export function hasThenProperty(node: TSESTree.Node): boolean {
151151
);
152152
}
153153

154-
export function hasChainedThen(node: TSESTree.Node): boolean {
154+
export function hasPromiseHandlerProperty(node: TSESTree.Node): boolean {
155+
return (
156+
isMemberExpression(node) &&
157+
ASTUtils.isIdentifier(node.property) &&
158+
['then', 'catch'].includes(node.property.name)
159+
);
160+
}
161+
162+
export function hasChainedPromiseHandler(node: TSESTree.Node): boolean {
155163
const parent = node.parent;
156164

157-
// wait(...).then(...)
165+
// wait(...).then(...) or wait(...).catch(...)
158166
if (isCallExpression(parent) && parent.parent) {
159-
return hasThenProperty(parent.parent);
167+
return hasPromiseHandlerProperty(parent.parent);
160168
}
161169

162-
// promise.then(...)
163-
return !!parent && hasThenProperty(parent);
170+
// promise.then(...) or promise.catch(...)
171+
return !!parent && hasPromiseHandlerProperty(parent);
164172
}
165173

166174
export function isPromiseIdentifier(
@@ -214,7 +222,7 @@ export function isPromisesArrayResolved(node: TSESTree.Node): boolean {
214222
* - it belongs to the `await` expression
215223
* - it belongs to the `Promise.all` method
216224
* - it belongs to the `Promise.allSettled` method
217-
* - it's chained with the `then` method
225+
* - it's chained with the `then` or `catch` method
218226
* - it's returned from a function
219227
* - has `resolves` or `rejects` jest methods
220228
* - has `toResolve` or `toReject` jest-extended matchers
@@ -243,7 +251,7 @@ export function isPromiseHandled(nodeIdentifier: TSESTree.Identifier): boolean {
243251
)
244252
return true;
245253
if (hasClosestExpectHandlesPromise(node.parent)) return true;
246-
if (hasChainedThen(node)) return true;
254+
if (hasChainedPromiseHandler(node)) return true;
247255
if (isPromisesArrayResolved(node)) return true;
248256
});
249257
}

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,16 @@ ruleTester.run(RULE_NAME, rule, {
9696
fireEvent.${eventMethod}(getByLabelText('username')).then(() => { done() })
9797
})
9898
})
99+
`,
100+
options: [{ eventModule: 'fireEvent' }] as const,
101+
})),
102+
...FIRE_EVENT_ASYNC_FUNCTIONS.map((eventMethod) => ({
103+
code: `
104+
import { fireEvent } from '${testingFramework}'
105+
test('chain catch method to promise from event method is valid', async (done) => {
106+
fireEvent.${eventMethod}(getByLabelText('username'))
107+
.catch((error) => { done() })
108+
})
99109
`,
100110
options: [{ eventModule: 'fireEvent' }] as const,
101111
})),
@@ -330,6 +340,16 @@ ruleTester.run(RULE_NAME, rule, {
330340
...USER_EVENT_ASYNC_FUNCTIONS.map((eventMethod) => ({
331341
code: `
332342
import userEvent from '${testingFramework}'
343+
test('chain catch method to promise from event method is valid', async (done) => {
344+
userEvent.${eventMethod}(getByLabelText('username'))
345+
.catch((error) => { done() })
346+
})
347+
`,
348+
options: [{ eventModule: 'userEvent' }] as const,
349+
})),
350+
...USER_EVENT_ASYNC_FUNCTIONS.map((eventMethod) => ({
351+
code: `
352+
import userEvent from '${testingFramework}'
333353
test('chain then method to several promises from event methods is valid', async (done) => {
334354
userEvent.${eventMethod}(getByLabelText('username')).then(() => {
335355
userEvent.${eventMethod}(getByLabelText('username')).then(() => { done() })

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,15 @@ ruleTester.run(RULE_NAME, rule, {
160160
`
161161
),
162162

163+
// async queries are valid with catch
164+
...createTestCase(
165+
(query) => `
166+
${query}('foo').catch((error) => {
167+
expect(error.message).toMatch(/my error message/)
168+
})
169+
`
170+
),
171+
163172
// async queries are valid when wrapped within Promise.all + await expression
164173
...createTestCase(
165174
(query) => `

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,15 @@ ruleTester.run(RULE_NAME, rule, {
4848
doSomethingElse();
4949
${asyncUtil}(() => getByLabelText('email')).then(() => { console.log('done') });
5050
});
51+
`,
52+
})),
53+
...ASYNC_UTILS.map((asyncUtil) => ({
54+
code: `
55+
import { ${asyncUtil} } from '${testingFramework}';
56+
test('${asyncUtil} util directly chained with catch is valid', () => {
57+
doSomethingElse();
58+
${asyncUtil}(() => getByLabelText('email')).catch((error) => { console.log('done') });
59+
});
5160
`,
5261
})),
5362
...ASYNC_UTILS.map((asyncUtil) => ({

0 commit comments

Comments
 (0)