From 80173278bd161f79bb21a95c0c774ea8c342a378 Mon Sep 17 00:00:00 2001 From: jhuang Date: Thu, 22 Oct 2020 01:13:11 -0700 Subject: [PATCH 1/6] feat(rule): add no-await-sync-events rule --- docs/rules/no-await-sync-events.md | 53 +++++++ lib/index.ts | 2 + lib/rules/no-await-sync-events.ts | 50 ++++++ lib/utils.ts | 6 + tests/lib/rules/no-await-sync-events.test.ts | 157 +++++++++++++++++++ 5 files changed, 268 insertions(+) create mode 100644 docs/rules/no-await-sync-events.md create mode 100644 lib/rules/no-await-sync-events.ts create mode 100644 tests/lib/rules/no-await-sync-events.test.ts diff --git a/docs/rules/no-await-sync-events.md b/docs/rules/no-await-sync-events.md new file mode 100644 index 00000000..2c993c26 --- /dev/null +++ b/docs/rules/no-await-sync-events.md @@ -0,0 +1,53 @@ +# Disallow unnecessary `await` for sync events (no-await-sync-events) + +Ensure that sync events are not awaited unnecessarily. + +## Rule Details + +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: + +- `fireEvent.click` +- `fireEvent.select` +- `userEvent.tab` +- `userEvent.hover` + +This rule aims to prevent users from waiting for those function calls. + +Examples of **incorrect** code for this rule: + +```js +const foo = async () => { + // ... + await fireEvent.click(button); + // ... +}; + +const bar = () => { + // ... + await userEvent.tab(); + // ... +}; +``` + +Examples of **correct** code for this rule: + +```js +const foo = () => { + // ... + fireEvent.click(button); + // ... +}; + +const bar = () => { + // ... + userEvent.tab(); + // ... +}; +``` + +## Notes + +There is another rule `await-fire-event`, which is only in Vue Testing +Library. Please do not confuse with this rule. diff --git a/lib/index.ts b/lib/index.ts index 2257bb2c..cb35f058 100644 --- a/lib/index.ts +++ b/lib/index.ts @@ -2,6 +2,7 @@ import awaitAsyncQuery from './rules/await-async-query'; import awaitAsyncUtils from './rules/await-async-utils'; import awaitFireEvent from './rules/await-fire-event'; import consistentDataTestid from './rules/consistent-data-testid'; +import noAwaitSyncEvents from './rules/no-await-sync-events'; import noAwaitSyncQuery from './rules/no-await-sync-query'; import noDebug from './rules/no-debug'; import noDomImport from './rules/no-dom-import'; @@ -20,6 +21,7 @@ const rules = { 'await-async-utils': awaitAsyncUtils, 'await-fire-event': awaitFireEvent, 'consistent-data-testid': consistentDataTestid, + 'no-await-sync-events': noAwaitSyncEvents, 'no-await-sync-query': noAwaitSyncQuery, 'no-debug': noDebug, 'no-dom-import': noDomImport, diff --git a/lib/rules/no-await-sync-events.ts b/lib/rules/no-await-sync-events.ts new file mode 100644 index 00000000..6d8399ff --- /dev/null +++ b/lib/rules/no-await-sync-events.ts @@ -0,0 +1,50 @@ +import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-utils'; +import { getDocsUrl, ASYNC_EVENTS } from '../utils'; + +export const RULE_NAME = 'no-await-sync-events'; +export type MessageIds = 'noAwaitSyncEvents'; +type Options = []; + +const ASYNC_EVENTS_REGEXP = new RegExp(`^(${ASYNC_EVENTS.join('|')})$`); +export default ESLintUtils.RuleCreator(getDocsUrl)({ + name: RULE_NAME, + meta: { + type: 'problem', + docs: { + description: 'Disallow unnecessary `await` for sync events', + category: 'Best Practices', + recommended: 'error', + }, + messages: { + noAwaitSyncEvents: '`{{ name }}` does not need `await` operator', + }, + fixable: null, + schema: [], + }, + defaultOptions: [], + + 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 + return { + [`AwaitExpression > CallExpression > MemberExpression > Identifier[name=${ASYNC_EVENTS_REGEXP}]`]( + node: TSESTree.Identifier + ) { + const memberExpression = node.parent as TSESTree.MemberExpression; + const methodNode = memberExpression.property as TSESTree.Identifier; + + if (!(node.name === 'userEvent' && methodNode.name === 'type')) { + context.report({ + node: methodNode, + messageId: 'noAwaitSyncEvents', + data: { + name: `${node.name}.${methodNode.name}`, + }, + }); + } + }, + }; + }, +}); diff --git a/lib/utils.ts b/lib/utils.ts index 8ea7512c..f647cdce 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -63,6 +63,11 @@ const ASYNC_UTILS = [ 'waitForDomChange', ]; +const ASYNC_EVENTS = [ + 'fireEvent', + 'userEvent', +]; + const TESTING_FRAMEWORK_SETUP_HOOKS = ['beforeEach', 'beforeAll']; const PRESENCE_MATCHERS = ['toBeInTheDocument', 'toBeTruthy', 'toBeDefined']; @@ -78,6 +83,7 @@ export { ASYNC_QUERIES_COMBINATIONS, ALL_QUERIES_COMBINATIONS, ASYNC_UTILS, + ASYNC_EVENTS, TESTING_FRAMEWORK_SETUP_HOOKS, LIBRARY_MODULES, PRESENCE_MATCHERS, diff --git a/tests/lib/rules/no-await-sync-events.test.ts b/tests/lib/rules/no-await-sync-events.test.ts new file mode 100644 index 00000000..370f4392 --- /dev/null +++ b/tests/lib/rules/no-await-sync-events.test.ts @@ -0,0 +1,157 @@ +import { createRuleTester } from '../test-utils'; +import rule, { RULE_NAME } from '../../../lib/rules/no-await-sync-events'; +import { ASYNC_EVENTS } from '../../../lib/utils'; + +const ruleTester = createRuleTester(); + +const fireEventFunctions = [ + 'copy', + 'cut', + 'paste', + 'compositionEnd', + 'compositionStart', + 'compositionUpdate', + 'keyDown', + 'keyPress', + 'keyUp', + 'focus', + 'blur', + 'focusIn', + 'focusOut', + 'change', + 'input', + 'invalid', + 'submit', + 'reset', + 'click', + 'contextMenu', + 'dblClick', + 'drag', + 'dragEnd', + 'dragEnter', + 'dragExit', + 'dragLeave', + 'dragOver', + 'dragStart', + 'drop', + 'mouseDown', + 'mouseEnter', + 'mouseLeave', + 'mouseMove', + 'mouseOut', + 'mouseOver', + 'mouseUp', + 'popState', + 'select', + 'touchCancel', + 'touchEnd', + 'touchMove', + 'touchStart', + 'scroll', + 'wheel', + 'abort', + 'canPlay', + 'canPlayThrough', + 'durationChange', + 'emptied', + 'encrypted', + 'ended', + 'loadedData', + 'loadedMetadata', + 'loadStart', + 'pause', + 'play', + 'playing', + 'progress', + 'rateChange', + 'seeked', + 'seeking', + 'stalled', + 'suspend', + 'timeUpdate', + 'volumeChange', + 'waiting', + 'load', + 'error', + 'animationStart', + 'animationEnd', + 'animationIteration', + 'transitionEnd', + 'doubleClick', + 'pointerOver', + 'pointerEnter', + 'pointerDown', + 'pointerMove', + 'pointerUp', + 'pointerCancel', + 'pointerOut', + 'pointerLeave', + 'gotPointerCapture', + 'lostPointerCapture', +]; +const userEventFunctions = [ + 'clear', + 'click', + 'dblClick', + 'selectOptions', + 'deselectOptions', + 'upload', + // 'type', + 'tab', + 'paste', + 'hover', + 'unhover', +]; +let eventFunctions: string[] = []; +ASYNC_EVENTS.forEach(event => { + switch (event) { + case 'fireEvent': + eventFunctions = eventFunctions.concat(fireEventFunctions.map((f: string): string => `${event}.${f}`)); + break; + case 'userEvent': + eventFunctions = eventFunctions.concat(userEventFunctions.map((f: string): string => `${event}.${f}`)); + break; + default: + eventFunctions.push(`${event}.anyFunc`); + } +}); + +ruleTester.run(RULE_NAME, rule, { + valid: [ + // sync events without await are valid + // userEvent.type() is an exception + ...eventFunctions.map(func => ({ + code: `() => { + ${func}('foo') + } + `, + })), + { + code: `() => { + userEvent.type('foo') + } + `, + }, + { + code: `() => { + await userEvent.type('foo') + } + `, + }, + ], + + invalid: [ + // sync events with await operator are not valid + ...eventFunctions.map(func => ({ + code: `() => { + await ${func}('foo') + } + `, + errors: [ + { + messageId: 'noAwaitSyncEvents', + }, + ], + })), + ], +}); From ce84207efac909b78ffd36e1541e662613ecb3e9 Mon Sep 17 00:00:00 2001 From: jhuang Date: Sun, 25 Oct 2020 21:46:24 -0700 Subject: [PATCH 2/6] feat(rule): address feedback --- README.md | 4 +++ lib/rules/no-await-sync-events.ts | 25 ++++++++++------ lib/utils.ts | 4 +-- tests/lib/rules/no-await-sync-events.test.ts | 30 +++++++++++++------- 4 files changed, 42 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 6b2ec53a..571703cf 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,9 @@ [![Tweet][tweet-badge]][tweet-url] + [![All Contributors](https://img.shields.io/badge/all_contributors-34-orange.svg?style=flat-square)](#contributors-) + ## Installation @@ -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][] | | | [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][] | @@ -226,6 +229,7 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d + This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome! diff --git a/lib/rules/no-await-sync-events.ts b/lib/rules/no-await-sync-events.ts index 6d8399ff..2076a59c 100644 --- a/lib/rules/no-await-sync-events.ts +++ b/lib/rules/no-await-sync-events.ts @@ -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)({ name: RULE_NAME, meta: { @@ -25,17 +25,26 @@ export default ESLintUtils.RuleCreator(getDocsUrl)({ 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', diff --git a/lib/utils.ts b/lib/utils.ts index f647cdce..f6f5b323 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -63,7 +63,7 @@ const ASYNC_UTILS = [ 'waitForDomChange', ]; -const ASYNC_EVENTS = [ +const SYNC_EVENTS = [ 'fireEvent', 'userEvent', ]; @@ -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, diff --git a/tests/lib/rules/no-await-sync-events.test.ts b/tests/lib/rules/no-await-sync-events.test.ts index 370f4392..c221b03c 100644 --- a/tests/lib/rules/no-await-sync-events.test.ts +++ b/tests/lib/rules/no-await-sync-events.test.ts @@ -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(); @@ -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}`)); @@ -134,7 +134,7 @@ ruleTester.run(RULE_NAME, rule, { }, { code: `() => { - await userEvent.type('foo') + await userEvent.type('foo', 'bar', {delay: 1234}) } `, }, @@ -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' },], + } ], }); From 780ac256c3636ddb5a3cd2fa8e8b7cadfbf9a1fa Mon Sep 17 00:00:00 2001 From: jhuang Date: Sun, 25 Oct 2020 21:54:14 -0700 Subject: [PATCH 3/6] feat(rule): update doc --- docs/rules/no-await-sync-events.md | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/docs/rules/no-await-sync-events.md b/docs/rules/no-await-sync-events.md index 2c993c26..80744627 100644 --- a/docs/rules/no-await-sync-events.md +++ b/docs/rules/no-await-sync-events.md @@ -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` @@ -29,6 +31,12 @@ const bar = () => { await userEvent.tab(); // ... }; + +const baz = () => { + // ... + await userEvent.type(textInput, 'abc'); + // ... +}; ``` Examples of **correct** code for this rule: @@ -45,6 +53,12 @@ const bar = () => { userEvent.tab(); // ... }; + +const baz = () => { + // ... + await userEvent.type(textInput, 'abc', {delay: 1000}); + // ... +}; ``` ## Notes From 295ba379b70b15a3ed3cb5d76e8a54ea97771d89 Mon Sep 17 00:00:00 2001 From: Jian Huang Date: Mon, 26 Oct 2020 15:55:03 -0700 Subject: [PATCH 4/6] Update no-await-sync-events.md --- docs/rules/no-await-sync-events.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/rules/no-await-sync-events.md b/docs/rules/no-await-sync-events.md index 80744627..9eb61548 100644 --- a/docs/rules/no-await-sync-events.md +++ b/docs/rules/no-await-sync-events.md @@ -55,8 +55,9 @@ const bar = () => { }; const baz = () => { - // ... + // await userEvent.type only with delay option await userEvent.type(textInput, 'abc', {delay: 1000}); + userEvent.type(textInput, '123'); // ... }; ``` From 47759a6b9ec0480066893f9883831c180e06872a Mon Sep 17 00:00:00 2001 From: Jian Huang Date: Mon, 26 Oct 2020 17:01:31 -0700 Subject: [PATCH 5/6] feat(rule): clean doc --- README.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/README.md b/README.md index 571703cf..2870517e 100644 --- a/README.md +++ b/README.md @@ -23,9 +23,7 @@ [![Tweet][tweet-badge]][tweet-url] - [![All Contributors](https://img.shields.io/badge/all_contributors-34-orange.svg?style=flat-square)](#contributors-) - ## Installation @@ -229,7 +227,6 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d - This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome! From d3c389754f4a3dd15580335fca7b8a6aeac88d24 Mon Sep 17 00:00:00 2001 From: Jian Huang Date: Tue, 27 Oct 2020 21:06:08 -0700 Subject: [PATCH 6/6] feat(rule): clean doc --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 2870517e..7b5fa7ba 100644 --- a/README.md +++ b/README.md @@ -137,7 +137,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][] | | +| [no-await-sync-events](docs/rules/no-await-sync-events.md) | Disallow unnecessary `await` for sync events | | | | [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][] |