-
Notifications
You must be signed in to change notification settings - Fork 136
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
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 <jhuang0@walmartlabs.com>
- Loading branch information
Showing
6 changed files
with
301 additions
and
0 deletions.
There are no files selected for viewing
This file contains 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 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 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 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 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 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: [ | ||
// 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' },], | ||
} | ||
], | ||
}); |