Skip to content

Commit 9d44911

Browse files
thebinaryfelixthebinaryfelix
authored
Merge branch 'v4' into no-container
2 parents 5aa3a80 + a4cc8d8 commit 9d44911

11 files changed

+264
-8
lines changed

‎.all-contributorsrc

+2-1
Original file line numberDiff line numberDiff line change
@@ -271,7 +271,8 @@
271271
"profile": "https://nickmccurdy.com/",
272272
"contributions": [
273273
"ideas",
274-
"code"
274+
"code",
275+
"review"
275276
]
276277
},
277278
{

‎README.md

+2-1
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,7 @@ To enable this configuration use the `extends` property in your
144144
| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
145145
| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
146146
| [no-manual-cleanup](docs/rules/no-manual-cleanup.md) | Disallow the use of `cleanup` | | |
147+
| [no-promise-in-fire-event](docs/rules/no-promise-in-fire-event.md) | Disallow the use of promises passed to a `fireEvent` method | | |
147148
| [no-wait-for-empty-callback](docs/rules/no-wait-for-empty-callback.md) | Disallow empty callbacks for `waitFor` and `waitForElementToBeRemoved` | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
148149
| [prefer-explicit-assert](docs/rules/prefer-explicit-assert.md) | Suggest using explicit assertions rather than just `getBy*` queries | | |
149150
| [prefer-find-by](docs/rules/prefer-find-by.md) | Suggest using `findBy*` methods instead of the `waitFor` + `getBy` queries | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
@@ -210,7 +211,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
210211
<td align="center"><a href="https://xxxl.digital/"><img src="https://avatars2.githubusercontent.com/u/42043025?v=4" width="100px;" alt=""/><br /><sub><b>Miguel Erja González</b></sub></a><br /><a href="https://github.com/testing-library/eslint-plugin-testing-library/issues?q=author%3Amiguelerja" title="Bug reports">🐛</a></td>
211212
<td align="center"><a href="http://pustovalov.dev"><img src="https://avatars2.githubusercontent.com/u/1568885?v=4" width="100px;" alt=""/><br /><sub><b>Pavel Pustovalov</b></sub></a><br /><a href="https://github.com/testing-library/eslint-plugin-testing-library/issues?q=author%3Apustovalov" title="Bug reports">🐛</a></td>
212213
<td align="center"><a href="https://github.com/jrparish"><img src="https://avatars3.githubusercontent.com/u/5173987?v=4" width="100px;" alt=""/><br /><sub><b>Jacob Parish</b></sub></a><br /><a href="https://github.com/testing-library/eslint-plugin-testing-library/issues?q=author%3Ajrparish" title="Bug reports">🐛</a> <a href="https://github.com/testing-library/eslint-plugin-testing-library/commits?author=jrparish" title="Code">💻</a> <a href="https://github.com/testing-library/eslint-plugin-testing-library/commits?author=jrparish" title="Tests">⚠️</a></td>
213-
<td align="center"><a href="https://nickmccurdy.com/"><img src="https://avatars0.githubusercontent.com/u/927220?v=4" width="100px;" alt=""/><br /><sub><b>Nick McCurdy</b></sub></a><br /><a href="#ideas-nickmccurdy" title="Ideas, Planning, & Feedback">🤔</a> <a href="https://github.com/testing-library/eslint-plugin-testing-library/commits?author=nickmccurdy" title="Code">💻</a></td>
214+
<td align="center"><a href="https://nickmccurdy.com/"><img src="https://avatars0.githubusercontent.com/u/927220?v=4" width="100px;" alt=""/><br /><sub><b>Nick McCurdy</b></sub></a><br /><a href="#ideas-nickmccurdy" title="Ideas, Planning, & Feedback">🤔</a> <a href="https://github.com/testing-library/eslint-plugin-testing-library/commits?author=nickmccurdy" title="Code">💻</a> <a href="https://github.com/testing-library/eslint-plugin-testing-library/pulls?q=is%3Apr+reviewed-by%3Anickmccurdy" title="Reviewed Pull Requests">👀</a></td>
214215
<td align="center"><a href="https://stefancameron.com/"><img src="https://avatars3.githubusercontent.com/u/2855350?v=4" width="100px;" alt=""/><br /><sub><b>Stefan Cameron</b></sub></a><br /><a href="https://github.com/testing-library/eslint-plugin-testing-library/issues?q=author%3Astefcameron" title="Bug reports">🐛</a></td>
215216
</tr>
216217
</table>

‎docs/rules/no-promise-in-fire-event.md

+35
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
# Disallow the use of promises passed to a `fireEvent` method (no-promise-in-fire-event)
2+
3+
The `fireEvent` method expects that a DOM element is passed.
4+
5+
Examples of **incorrect** code for this rule:
6+
7+
```js
8+
import { screen, fireEvent } from '@testing-library/react';
9+
10+
// usage of findBy queries
11+
fireEvent.click(screen.findByRole('button'));
12+
13+
// usage of promises
14+
fireEvent.click(new Promise(jest.fn())
15+
```
16+
17+
Examples of **correct** code for this rule:
18+
19+
```js
20+
import { screen, fireEvent } from '@testing-library/react';
21+
22+
// use getBy queries
23+
fireEvent.click(screen.getByRole('button'));
24+
25+
// use awaited findBy queries
26+
fireEvent.click(await screen.findByRole('button'));
27+
28+
// this won't give a linting error, but it will throw a runtime error
29+
const promise = new Promise();
30+
fireEvent.click(promise)`,
31+
```
32+
33+
## Further Reading
34+
35+
- [A Github Issue explaining the problem](https://github.com/testing-library/dom-testing-library/issues/609)

‎docs/rules/prefer-screen-queries.md

+6
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ const utils = render(<Foo />);
5454
utils.rerender(<Foo />);
5555
utils.unmount();
5656
utils.asFragment();
57+
58+
// the same functions, but called from destructuring
59+
const { rerender, unmount, asFragment } = render(<Foo />);
60+
rerender(<Foo />);
61+
asFragment();
62+
unmount();
5763
```
5864

5965
## Further Reading

‎lib/index.ts

+3
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import noDebug from './rules/no-debug';
88
import noDomImport from './rules/no-dom-import';
99
import noManualCleanup from './rules/no-manual-cleanup';
1010
import noWaitForEmptyCallback from './rules/no-wait-for-empty-callback';
11+
import noPromiseInFireEvent from './rules/no-promise-in-fire-event';
1112
import preferExplicitAssert from './rules/prefer-explicit-assert';
1213
import preferPresenceQueries from './rules/prefer-presence-queries';
1314
import preferScreenQueries from './rules/prefer-screen-queries';
@@ -24,6 +25,7 @@ const rules = {
2425
'no-debug': noDebug,
2526
'no-dom-import': noDomImport,
2627
'no-manual-cleanup': noManualCleanup,
28+
'no-promise-in-fire-event': noPromiseInFireEvent,
2729
'no-wait-for-empty-callback': noWaitForEmptyCallback,
2830
'prefer-explicit-assert': preferExplicitAssert,
2931
'prefer-find-by': preferFindBy,
@@ -36,6 +38,7 @@ const recommendedRules = {
3638
'testing-library/await-async-query': 'error',
3739
'testing-library/await-async-utils': 'error',
3840
'testing-library/no-await-sync-query': 'error',
41+
'testing-library/no-promise-in-fire-event': 'error',
3942
'testing-library/no-wait-for-empty-callback': 'error',
4043
'testing-library/prefer-find-by': 'error',
4144
'testing-library/prefer-screen-queries': 'error',

‎lib/node-utils.ts

+11-3
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,12 @@ export function isAwaitExpression(
1212
return node && node.type === 'AwaitExpression';
1313
}
1414

15+
export function isNewExpression(
16+
node: TSESTree.Node
17+
): node is TSESTree.NewExpression {
18+
return node && node.type === 'NewExpression';
19+
}
20+
1521
export function isIdentifier(node: TSESTree.Node): node is TSESTree.Identifier {
1622
return node && node.type === 'Identifier';
1723
}
@@ -103,8 +109,10 @@ export function hasThenProperty(node: TSESTree.Node) {
103109
);
104110
}
105111

106-
export function isArrowFunctionExpression(node: TSESTree.Node): node is TSESTree.ArrowFunctionExpression {
107-
return node && node.type === 'ArrowFunctionExpression'
112+
export function isArrowFunctionExpression(
113+
node: TSESTree.Node
114+
): node is TSESTree.ArrowFunctionExpression {
115+
return node && node.type === 'ArrowFunctionExpression';
108116
}
109117

110118
function isRenderFunction(
@@ -138,4 +146,4 @@ export function isRenderVariableDeclarator(
138146
}
139147

140148
return false;
141-
}
149+
}

‎lib/rules/no-promise-in-fire-event.ts

+74
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import { TSESTree, ESLintUtils } from '@typescript-eslint/experimental-utils';
2+
import { getDocsUrl, ASYNC_QUERIES_VARIANTS } from '../utils';
3+
import {
4+
isNewExpression,
5+
isIdentifier,
6+
isImportSpecifier,
7+
isCallExpression,
8+
} from '../node-utils';
9+
10+
export const RULE_NAME = 'no-promise-in-fire-event';
11+
export type MessageIds = 'noPromiseInFireEvent';
12+
type Options = [];
13+
14+
export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
15+
name: RULE_NAME,
16+
meta: {
17+
type: 'problem',
18+
docs: {
19+
description:
20+
'Disallow the use of promises passed to a `fireEvent` method',
21+
category: 'Best Practices',
22+
recommended: false,
23+
},
24+
messages: {
25+
noPromiseInFireEvent:
26+
"A promise shouldn't be passed to a `fireEvent` method, instead pass the DOM element",
27+
},
28+
fixable: 'code',
29+
schema: [],
30+
},
31+
defaultOptions: [],
32+
33+
create(context) {
34+
return {
35+
'ImportDeclaration[source.value=/testing-library/]'(
36+
node: TSESTree.ImportDeclaration
37+
) {
38+
const fireEventImportNode = node.specifiers.find(
39+
specifier =>
40+
isImportSpecifier(specifier) &&
41+
specifier.imported &&
42+
'fireEvent' === specifier.imported.name
43+
) as TSESTree.ImportSpecifier;
44+
45+
const { references } = context.getDeclaredVariables(
46+
fireEventImportNode
47+
)[0];
48+
49+
for (const reference of references) {
50+
const referenceNode = reference.identifier;
51+
const callExpression = referenceNode.parent
52+
.parent as TSESTree.CallExpression;
53+
const [element] = callExpression.arguments as TSESTree.Node[];
54+
if (isCallExpression(element) || isNewExpression(element)) {
55+
const methodName = isIdentifier(element.callee)
56+
? element.callee.name
57+
: ((element.callee as TSESTree.MemberExpression)
58+
.property as TSESTree.Identifier).name;
59+
60+
if (
61+
ASYNC_QUERIES_VARIANTS.some(q => methodName.startsWith(q)) ||
62+
methodName === 'Promise'
63+
) {
64+
context.report({
65+
node: element,
66+
messageId: 'noPromiseInFireEvent',
67+
});
68+
}
69+
}
70+
}
71+
},
72+
};
73+
},
74+
});

‎tests/__snapshots__/index.test.ts.snap

+4
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ Object {
1515
"error",
1616
"angular",
1717
],
18+
"testing-library/no-promise-in-fire-event": "error",
1819
"testing-library/no-wait-for-empty-callback": "error",
1920
"testing-library/prefer-find-by": "error",
2021
"testing-library/prefer-screen-queries": "error",
@@ -37,6 +38,7 @@ Object {
3738
"error",
3839
"react",
3940
],
41+
"testing-library/no-promise-in-fire-event": "error",
4042
"testing-library/no-wait-for-empty-callback": "error",
4143
"testing-library/prefer-find-by": "error",
4244
"testing-library/prefer-screen-queries": "error",
@@ -53,6 +55,7 @@ Object {
5355
"testing-library/await-async-query": "error",
5456
"testing-library/await-async-utils": "error",
5557
"testing-library/no-await-sync-query": "error",
58+
"testing-library/no-promise-in-fire-event": "error",
5659
"testing-library/no-wait-for-empty-callback": "error",
5760
"testing-library/prefer-find-by": "error",
5861
"testing-library/prefer-screen-queries": "error",
@@ -76,6 +79,7 @@ Object {
7679
"error",
7780
"vue",
7881
],
82+
"testing-library/no-promise-in-fire-event": "error",
7983
"testing-library/no-wait-for-empty-callback": "error",
8084
"testing-library/prefer-find-by": "error",
8185
"testing-library/prefer-screen-queries": "error",

‎tests/lib/rules/no-promise-in-fire-event.test.ts

+94
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
import { createRuleTester } from '../test-utils';
2+
import rule, { RULE_NAME } from '../../../lib/rules/no-promise-in-fire-event';
3+
4+
const ruleTester = createRuleTester();
5+
6+
ruleTester.run(RULE_NAME, rule, {
7+
valid: [
8+
{
9+
code: `
10+
import {fireEvent} from '@testing-library/foo';
11+
12+
fireEvent.click(screen.getByRole('button'))
13+
`,
14+
},
15+
{
16+
code: `
17+
import {fireEvent} from '@testing-library/foo';
18+
19+
fireEvent.click(queryByRole('button'))`,
20+
},
21+
{
22+
code: `
23+
import {fireEvent} from '@testing-library/foo';
24+
25+
fireEvent.click(someRef)`,
26+
},
27+
{
28+
code: `fireEvent.click(findByText('submit'))`,
29+
},
30+
{
31+
code: `
32+
import {fireEvent} from '@testing-library/foo';
33+
34+
const promise = new Promise();
35+
fireEvent.click(promise)`,
36+
},
37+
{
38+
code: `
39+
import {fireEvent} from '@testing-library/foo';
40+
41+
fireEvent.click(await screen.findByRole('button'))
42+
`,
43+
},
44+
{
45+
code: `fireEvent.click(Promise())`,
46+
},
47+
],
48+
invalid: [
49+
{
50+
code: `
51+
import {fireEvent} from '@testing-library/foo';
52+
53+
fireEvent.click(screen.findByRole('button'))`,
54+
errors: [
55+
{
56+
messageId: 'noPromiseInFireEvent',
57+
},
58+
],
59+
},
60+
{
61+
code: `
62+
import {fireEvent} from '@testing-library/foo';
63+
64+
fireEvent.click(findByText('submit'))`,
65+
errors: [
66+
{
67+
messageId: 'noPromiseInFireEvent',
68+
},
69+
],
70+
},
71+
{
72+
code: `
73+
import {fireEvent} from '@testing-library/foo';
74+
75+
fireEvent.click(Promise('foo'))`,
76+
errors: [
77+
{
78+
messageId: 'noPromiseInFireEvent',
79+
},
80+
],
81+
},
82+
{
83+
code: `
84+
import {fireEvent} from '@testing-library/foo';
85+
86+
fireEvent.click(new Promise('foo'))`,
87+
errors: [
88+
{
89+
messageId: 'noPromiseInFireEvent',
90+
},
91+
],
92+
},
93+
],
94+
});

‎tests/lib/rules/prefer-screen-queries.test.ts

+21-3
Original file line numberDiff line numberDiff line change
@@ -47,21 +47,39 @@ ruleTester.run(RULE_NAME, rule, {
4747
},
4848
{
4949
code: `
50-
const utils = render(baz);
51-
screen.rerender();
50+
const { rerender } = render(baz);
51+
rerender();
5252
`
5353
},
5454
{
5555
code: `
5656
const utils = render(baz);
57-
utils.unmount();
57+
utils.rerender();
5858
`
5959
},
6060
{
6161
code: `
6262
const utils = render(baz);
6363
utils.asFragment();
6464
`
65+
},
66+
{
67+
code: `
68+
const { asFragment } = render(baz);
69+
asFragment();
70+
`
71+
},
72+
{
73+
code: `
74+
const { unmount } = render(baz);
75+
unmount();
76+
`
77+
},
78+
{
79+
code: `
80+
const utils = render(baz);
81+
utils.unmount();
82+
`
6583
}
6684
],
6785

‎tests/lib/rules/prefer-wait-for.test.ts

+12
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,18 @@ ruleTester.run(RULE_NAME, rule, {
6060
cy.wait();
6161
`,
6262
},
63+
{
64+
// https://github.com/testing-library/eslint-plugin-testing-library/issues/145
65+
code: `
66+
async function wait(): Promise<any> {
67+
// doesn't matter
68+
}
69+
70+
function callsWait(): void {
71+
await wait();
72+
}
73+
`,
74+
},
6375
],
6476

6577
invalid: [

0 commit comments

Comments
 (0)