feat(rule): add no-await-sync-events rule #240
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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)<Options, MessageIds>({ | ||
| 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}`, | ||
| }, | ||
| }); | ||
| } | ||
| }, | ||
| }; | ||
| }, | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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: [ | ||
|
There was a problem hiding this comment. You need to include more tests for checking There is an ongoing refactor for v4 of the plugin which actually will check that for all rules out of the box, so I don't know if you prefer to wait for that version to be released. There was a problem hiding this comment. @Belco90 There was a problem hiding this comment. The former, so we don't report const declared by the user with the same name. As mentioned, the internal refactor for v4 will pass some helpers to all rules to check this generically, so I'm not sure if you prefer to wait for that. There was a problem hiding this comment. Thanks for the clarification. |
||
| // 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' },], | ||
| } | ||
| ], | ||
| }); | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
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.typewithout await and delay please?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated