From 2cc22636726af524acb316aeb51d208fd8441a98 Mon Sep 17 00:00:00 2001 From: Jian Huang Date: Thu, 29 Oct 2020 07:35:47 -0700 Subject: [PATCH] feat: add no-await-sync-events rule (#240) * feat(rule): add no-await-sync-events rule * feat(rule): address feedback * feat(rule): update doc * Update no-await-sync-events.md * feat(rule): clean doc * feat(rule): clean doc Co-authored-by: jhuang --- README.md | 1 + docs/rules/no-await-sync-events.md | 68 ++++++++ lib/index.ts | 2 + lib/rules/no-await-sync-events.ts | 59 +++++++ lib/utils.ts | 6 + tests/lib/rules/no-await-sync-events.test.ts | 165 +++++++++++++++++++ 6 files changed, 301 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/README.md b/README.md index 6b2ec53a..7b5fa7ba 100644 --- a/README.md +++ b/README.md @@ -137,6 +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 | | | | [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][] | diff --git a/docs/rules/no-await-sync-events.md b/docs/rules/no-await-sync-events.md new file mode 100644 index 00000000..9eb61548 --- /dev/null +++ b/docs/rules/no-await-sync-events.md @@ -0,0 +1,68 @@ +# 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`, 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` +- `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(); + // ... +}; + +const baz = () => { + // ... + await userEvent.type(textInput, 'abc'); + // ... +}; +``` + +Examples of **correct** code for this rule: + +```js +const foo = () => { + // ... + fireEvent.click(button); + // ... +}; + +const bar = () => { + // ... + userEvent.tab(); + // ... +}; + +const baz = () => { + // await userEvent.type only with delay option + await userEvent.type(textInput, 'abc', {delay: 1000}); + userEvent.type(textInput, '123'); + // ... +}; +``` + +## 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..2076a59c --- /dev/null +++ b/lib/rules/no-await-sync-events.ts @@ -0,0 +1,59 @@ +import { ESLintUtils, TSESTree } from '@typescript-eslint/experimental-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 SYNC_EVENTS_REGEXP = new RegExp(`^(${SYNC_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. 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=${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' && withDelay)) { + context.report({ + node: methodNode, + messageId: 'noAwaitSyncEvents', + data: { + name: `${node.name}.${methodNode.name}`, + }, + }); + } + }, + }; + }, +}); diff --git a/lib/utils.ts b/lib/utils.ts index 8ea7512c..f6f5b323 100644 --- a/lib/utils.ts +++ b/lib/utils.ts @@ -63,6 +63,11 @@ const ASYNC_UTILS = [ 'waitForDomChange', ]; +const SYNC_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, + 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 new file mode 100644 index 00000000..c221b03c --- /dev/null +++ b/tests/lib/rules/no-await-sync-events.test.ts @@ -0,0 +1,165 @@ +import { createRuleTester } from '../test-utils'; +import rule, { RULE_NAME } from '../../../lib/rules/no-await-sync-events'; +import { SYNC_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[] = []; +SYNC_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', 'bar', {delay: 1234}) + } + `, + }, + ], + + invalid: [ + // sync events with await operator are not valid + ...eventFunctions.map(func => ({ + 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: [{ 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' },], + } + ], +});