Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(rule): add no-await-sync-events rule #240

Merged
merged 6 commits into from
Oct 29, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@
[![Tweet][tweet-badge]][tweet-url]

<!-- ALL-CONTRIBUTORS-BADGE:START - Do not remove or modify this section -->

[![All Contributors](https://img.shields.io/badge/all_contributors-34-orange.svg?style=flat-square)](#contributors-)

<!-- ALL-CONTRIBUTORS-BADGE:END -->

## Installation
Expand Down Expand Up @@ -137,6 +139,7 @@ To enable this configuration use the `extends` property in your
| [await-async-utils](docs/rules/await-async-utils.md) | Enforce async utils to be awaited properly | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
| [await-fire-event](docs/rules/await-fire-event.md) | Enforce async fire event methods to be awaited | ![vue-badge][] | |
| [consistent-data-testid](docs/rules/consistent-data-testid.md) | Ensure `data-testid` values match a provided regex. | | |
| [no-await-sync-events](docs/rules/no-await-sync-events.md) | Disallow unnecessary `await` for sync events | ![recommended-badge][] ![angular-badge][] ![react-badge][] | |
Belco90 marked this conversation as resolved.
Show resolved Hide resolved
| [no-await-sync-query](docs/rules/no-await-sync-query.md) | Disallow unnecessary `await` for sync queries | ![recommended-badge][] ![angular-badge][] ![react-badge][] ![vue-badge][] | |
| [no-debug](docs/rules/no-debug.md) | Disallow the use of `debug` | ![angular-badge][] ![react-badge][] ![vue-badge][] | |
| [no-dom-import](docs/rules/no-dom-import.md) | Disallow importing from DOM Testing Library | ![angular-badge][] ![react-badge][] ![vue-badge][] | ![fixable-badge][] |
Expand Down Expand Up @@ -226,6 +229,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d

<!-- markdownlint-enable -->
<!-- prettier-ignore-end -->

<!-- ALL-CONTRIBUTORS-LIST:END -->

This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome!
16 changes: 15 additions & 1 deletion docs/rules/no-await-sync-events.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ Ensure that sync events are not awaited unnecessarily.

Functions in the event object provided by Testing Library, including
fireEvent and userEvent, do NOT return Promise, with an exception of
`userEvent.type`. Some examples are:
`userEvent.type`, which delays the promise resolve only if [`delay`
option](https://github.com/testing-library/user-event#typeelement-text-options) is specified.
Some examples are:

- `fireEvent.click`
- `fireEvent.select`
Expand All @@ -29,6 +31,12 @@ const bar = () => {
await userEvent.tab();
// ...
};

const baz = () => {
// ...
await userEvent.type(textInput, 'abc');
// ...
};
```

Examples of **correct** code for this rule:
Expand All @@ -45,6 +53,12 @@ const bar = () => {
userEvent.tab();
// ...
};

const baz = () => {
// ...
await userEvent.type(textInput, 'abc', {delay: 1000});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add an extra correct example for userEvent.type without await and delay please?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated

// ...
};
```

## Notes
Expand Down
25 changes: 17 additions & 8 deletions lib/rules/no-await-sync-events.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils';
import { getDocsUrl, ASYNC_EVENTS } from '../utils';

import { getDocsUrl, SYNC_EVENTS } from '../utils';
import { isObjectExpression, isProperty, isIdentifier } from '../node-utils';
export const RULE_NAME = 'no-await-sync-events';
export type MessageIds = 'noAwaitSyncEvents';
type Options = [];

const ASYNC_EVENTS_REGEXP = new RegExp(`^(${ASYNC_EVENTS.join('|')})$`);
const SYNC_EVENTS_REGEXP = new RegExp(`^(${SYNC_EVENTS.join('|')})$`);
export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({
name: RULE_NAME,
meta: {
Expand All @@ -25,17 +25,26 @@ export default ESLintUtils.RuleCreator(getDocsUrl)<Options, MessageIds>({

create(context) {
// userEvent.type() is an exception, which returns a
// Promise, even it resolves immediately.
// for the sake of semantically correct, w/ or w/o await
// are both OK
// Promise. But it is only necessary to wait when delay
// option is specified. So this rule has a special exception
// for the case await userEvent.type(element, 'abc', {delay: 1234})
return {
[`AwaitExpression > CallExpression > MemberExpression > Identifier[name=${ASYNC_EVENTS_REGEXP}]`](
[`AwaitExpression > CallExpression > MemberExpression > Identifier[name=${SYNC_EVENTS_REGEXP}]`](
node: TSESTree.Identifier
) {
const memberExpression = node.parent as TSESTree.MemberExpression;
const methodNode = memberExpression.property as TSESTree.Identifier;
const callExpression = memberExpression.parent as TSESTree.CallExpression;
const withDelay = callExpression.arguments.length >= 3 &&
isObjectExpression(callExpression.arguments[2]) &&
callExpression.arguments[2].properties.some(
property =>
isProperty(property) &&
isIdentifier(property.key) &&
property.key.name === 'delay'
);

if (!(node.name === 'userEvent' && methodNode.name === 'type')) {
if (!(node.name === 'userEvent' && methodNode.name === 'type' && withDelay)) {
context.report({
node: methodNode,
messageId: 'noAwaitSyncEvents',
Expand Down
4 changes: 2 additions & 2 deletions lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ const ASYNC_UTILS = [
'waitForDomChange',
];

const ASYNC_EVENTS = [
const SYNC_EVENTS = [
'fireEvent',
'userEvent',
];
Expand All @@ -83,7 +83,7 @@ export {
ASYNC_QUERIES_COMBINATIONS,
ALL_QUERIES_COMBINATIONS,
ASYNC_UTILS,
ASYNC_EVENTS,
SYNC_EVENTS,
TESTING_FRAMEWORK_SETUP_HOOKS,
LIBRARY_MODULES,
PRESENCE_MATCHERS,
Expand Down
30 changes: 19 additions & 11 deletions tests/lib/rules/no-await-sync-events.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createRuleTester } from '../test-utils';
import rule, { RULE_NAME } from '../../../lib/rules/no-await-sync-events';
import { ASYNC_EVENTS } from '../../../lib/utils';
import { SYNC_EVENTS } from '../../../lib/utils';

const ruleTester = createRuleTester();

Expand Down Expand Up @@ -103,7 +103,7 @@ const userEventFunctions = [
'unhover',
];
let eventFunctions: string[] = [];
ASYNC_EVENTS.forEach(event => {
SYNC_EVENTS.forEach(event => {
switch (event) {
case 'fireEvent':
eventFunctions = eventFunctions.concat(fireEventFunctions.map((f: string): string => `${event}.${f}`));
Expand Down Expand Up @@ -134,7 +134,7 @@ ruleTester.run(RULE_NAME, rule, {
},
{
code: `() => {
await userEvent.type('foo')
await userEvent.type('foo', 'bar', {delay: 1234})
}
`,
},
Expand All @@ -143,15 +143,23 @@ ruleTester.run(RULE_NAME, rule, {
invalid: [
// sync events with await operator are not valid
...eventFunctions.map(func => ({
code: `() => {
await ${func}('foo')
}
code: `
import { fireEvent } from '@testing-library/framework';
import userEvent from '@testing-library/user-event';
test('should report sync event awaited', async() => {
await ${func}('foo');
});
`,
errors: [
{
messageId: 'noAwaitSyncEvents',
},
],
errors: [{ line: 5, messageId: 'noAwaitSyncEvents' },],
})),
{
code: `
import userEvent from '@testing-library/user-event';
test('should report sync event awaited', async() => {
await userEvent.type('foo', 'bar', {hello: 1234});
});
`,
errors: [{ line: 4, messageId: 'noAwaitSyncEvents' },],
}
],
});