From 1f57b15b60a25a70b910b79f8bacff283c718d34 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Sat, 9 Apr 2022 18:19:01 +0200 Subject: [PATCH 1/4] feat(no-await-sync-events): add eventModules to rule options --- lib/rules/no-await-sync-events.ts | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/lib/rules/no-await-sync-events.ts b/lib/rules/no-await-sync-events.ts index ea7ce157..69e44046 100644 --- a/lib/rules/no-await-sync-events.ts +++ b/lib/rules/no-await-sync-events.ts @@ -9,11 +9,14 @@ import { isProperty, } from '../node-utils'; +const USER_EVENT_ASYNC_EXCEPTIONS: string[] = ['type', 'keyboard']; +const VALID_EVENT_MODULES = ['fire-event', 'user-event'] as const; + export const RULE_NAME = 'no-await-sync-events'; export type MessageIds = 'noAwaitSyncEvents'; -type Options = []; - -const USER_EVENT_ASYNC_EXCEPTIONS: string[] = ['type', 'keyboard']; +type Options = [ + { eventModules?: readonly typeof VALID_EVENT_MODULES[number][] } +]; export default createTestingLibraryRule({ name: RULE_NAME, @@ -32,9 +35,19 @@ export default createTestingLibraryRule({ noAwaitSyncEvents: '`{{ name }}` is sync and does not need `await` operator', }, - schema: [], + schema: [ + { + type: 'object', + properties: { + eventModules: { + enum: VALID_EVENT_MODULES, + }, + }, + additionalProperties: false, + }, + ], }, - defaultOptions: [], + defaultOptions: [{ eventModules: VALID_EVENT_MODULES }], create(context, _, helpers) { // userEvent.type() and userEvent.keyboard() are exceptions, which returns a From 2d8fe9552b8fd8cfa529d955729f85d50a1f61e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Sun, 10 Apr 2022 23:12:48 +0200 Subject: [PATCH 2/4] feat(no-await-sync-events): report only when event module enabled --- README.md | 2 + lib/rules/no-await-sync-events.ts | 21 ++++-- tests/lib/rules/no-await-sync-events.test.ts | 71 ++++++++++++++++++++ 3 files changed, 90 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index d9ea34cc..0b42e932 100644 --- a/README.md +++ b/README.md @@ -316,6 +316,8 @@ You can avoid this by: If you think the error you are getting is not related to this at all, please [fill a new issue](https://github.com/testing-library/eslint-plugin-testing-library/issues/new/choose) with as many details as possible. +## TODO: user-event v14 is async but the plugin complains about it shouldn't be awaited + ## Other documentation - [Semantic Versioning Policy](/docs/semantic-versioning-policy.md) diff --git a/lib/rules/no-await-sync-events.ts b/lib/rules/no-await-sync-events.ts index 69e44046..1adad982 100644 --- a/lib/rules/no-await-sync-events.ts +++ b/lib/rules/no-await-sync-events.ts @@ -49,7 +49,9 @@ export default createTestingLibraryRule({ }, defaultOptions: [{ eventModules: VALID_EVENT_MODULES }], - create(context, _, helpers) { + create(context, [options], helpers) { + const { eventModules = VALID_EVENT_MODULES } = options; + // userEvent.type() and userEvent.keyboard() are exceptions, which returns a // Promise. But it is only necessary to wait when delay option other than 0 // is specified. So this rule has a special exception for the case await: @@ -63,14 +65,25 @@ export default createTestingLibraryRule({ return; } - const isSimulateEventMethod = - helpers.isUserEventMethod(simulateEventFunctionIdentifier) || - helpers.isFireEventMethod(simulateEventFunctionIdentifier); + const isUserEventMethod = helpers.isUserEventMethod( + simulateEventFunctionIdentifier + ); + const isFireEventMethod = helpers.isFireEventMethod( + simulateEventFunctionIdentifier + ); + const isSimulateEventMethod = isUserEventMethod || isFireEventMethod; if (!isSimulateEventMethod) { return; } + if (isFireEventMethod && !eventModules.includes('fire-event')) { + return; + } + if (isUserEventMethod && !eventModules.includes('user-event')) { + return; + } + const lastArg = node.arguments[node.arguments.length - 1]; const hasDelay = diff --git a/tests/lib/rules/no-await-sync-events.test.ts b/tests/lib/rules/no-await-sync-events.test.ts index e63c130b..6389b1d7 100644 --- a/tests/lib/rules/no-await-sync-events.test.ts +++ b/tests/lib/rules/no-await-sync-events.test.ts @@ -166,6 +166,30 @@ ruleTester.run(RULE_NAME, rule, { }); `, }, + + // valid tests for fire-event when only user-event set in eventModules + ...FIRE_EVENT_FUNCTIONS.map((func) => ({ + name: `await fireEvent.${func} - "fire-event" disabled in eventModules option`, + code: ` + import { fireEvent } from '@testing-library/framework'; + test('should not report fireEvent.${func} sync event awaited', async() => { + await fireEvent.${func}('foo'); + }); + `, + options: [{ eventModules: 'user-event' }], + })), + + // valid tests for user-event when only fire-event set in eventModules + ...USER_EVENT_SYNC_FUNCTIONS.map((func) => ({ + name: `await userEvent.${func} - "user-event" disabled in eventModules option`, + code: ` + import userEvent from '@testing-library/user-event'; + test('should not report userEvent.${func} sync event awaited', async() => { + await userEvent.${func}('foo'); + }); + `, + options: [{ eventModules: 'fire-event' }], + })), ], invalid: [ @@ -210,6 +234,53 @@ ruleTester.run(RULE_NAME, rule, { } as const) ), + // sync fireEvent methods with await operator are not valid + // when only fire-event set in eventModules + ...FIRE_EVENT_FUNCTIONS.map( + (func) => + ({ + name: `await fireEvent.${func} - "fire-event" set in eventModules option`, + code: ` + import { fireEvent } from '@testing-library/framework'; + test('should report fireEvent.${func} sync event awaited', async() => { + await fireEvent.${func}('foo'); + }); + `, + options: [{ eventModules: 'fire-event' }], + errors: [ + { + line: 4, + column: 17, + messageId: 'noAwaitSyncEvents', + data: { name: `fireEvent.${func}` }, + }, + ], + } as const) + ), + // sync userEvent sync methods with await operator are not valid + // when only fire-event set in eventModules + ...USER_EVENT_SYNC_FUNCTIONS.map( + (func) => + ({ + name: `await userEvent.${func} - "user-event" set in eventModules option`, + code: ` + import userEvent from '@testing-library/user-event'; + test('should report userEvent.${func} sync event awaited', async() => { + await userEvent.${func}('foo'); + }); + `, + options: [{ eventModules: 'user-event' }], + errors: [ + { + line: 4, + column: 17, + messageId: 'noAwaitSyncEvents', + data: { name: `userEvent.${func}` }, + }, + ], + } as const) + ), + { code: ` import userEvent from '@testing-library/user-event'; From 3c543a2e0261ec13b81a17cd5b5fc13743f0ac95 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Sun, 10 Apr 2022 23:34:14 +0200 Subject: [PATCH 3/4] docs(no-await-sync-events): add options section --- README.md | 2 -- docs/rules/no-await-sync-events.md | 45 ++++++++++++++++++++++++++---- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/README.md b/README.md index 0b42e932..d9ea34cc 100644 --- a/README.md +++ b/README.md @@ -316,8 +316,6 @@ You can avoid this by: If you think the error you are getting is not related to this at all, please [fill a new issue](https://github.com/testing-library/eslint-plugin-testing-library/issues/new/choose) with as many details as possible. -## TODO: user-event v14 is async but the plugin complains about it shouldn't be awaited - ## Other documentation - [Semantic Versioning Policy](/docs/semantic-versioning-policy.md) diff --git a/docs/rules/no-await-sync-events.md b/docs/rules/no-await-sync-events.md index 58206373..141c3632 100644 --- a/docs/rules/no-await-sync-events.md +++ b/docs/rules/no-await-sync-events.md @@ -4,7 +4,7 @@ Ensure that sync simulated events are not awaited unnecessarily. ## Rule Details -Methods for simulating events in Testing Library ecosystem -`fireEvent` and `userEvent`- +Methods for simulating events in Testing Library ecosystem -`fireEvent` and `userEvent` prior to v14 - do NOT return any Promise, with an exception of `userEvent.type` and `userEvent.keyboard`, which delays the promise resolve only if [`delay` option](https://github.com/testing-library/user-event#typeelement-text-options) is specified. @@ -13,8 +13,8 @@ Some examples of simulating events not returning any Promise are: - `fireEvent.click` - `fireEvent.select` -- `userEvent.tab` -- `userEvent.hover` +- `userEvent.tab` (prior to `user-event` v14) +- `userEvent.hover` (prior to `user-event` v14) This rule aims to prevent users from waiting for those function calls. @@ -29,12 +29,14 @@ const foo = async () => { const bar = async () => { // ... + // userEvent prior to v14 await userEvent.tab(); // ... }; const baz = async () => { // ... + // userEvent prior to v14 await userEvent.type(textInput, 'abc'); await userEvent.keyboard('abc'); // ... @@ -66,9 +68,42 @@ const baz = async () => { userEvent.keyboard('123'); // ... }; + +const qux = async () => { + // userEvent v14 + await userEvent.tab(); + await userEvent.click(button); + await userEvent.type(textInput, 'abc'); + await userEvent.keyboard('abc'); + // ... +}; +``` + +## Options + +This rule provides the following options: + +- `eventModules`: array of strings. The possibilities are: `"fire-event"` and `"user-event"`. Defaults to `["fire-event", "user-event"]` + +### `eventModules` + +This option gives you more granular control of which event modules you want to report, so you can choose to only report methods from either `fire-event`, `user-event` or both. + +Example: + +```json +{ + "testing-library/no-await-sync-events": [ + "error", + { + "eventModules": ["fire-event", "user-event"] + } + ] +} ``` ## Notes -There is another rule `await-fire-event`, which is only in Vue Testing -Library. Please do not confuse with this rule. +- Since `user-event` v14 all its methods are async, so you should disable reporting them by setting the `eventModules` to just `"fire-event"` so `user-event` methods are not reported. +- There is another rule `await-fire-event`, which is only in Vue Testing + Library. Please do not confuse with this rule. From 5e288f35a894f08a5ff2b997fa66ce39c419a672 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mario=20Beltr=C3=A1n=20Alarc=C3=B3n?= Date: Mon, 11 Apr 2022 09:57:56 +0100 Subject: [PATCH 4/4] test: remove "name" property --- tests/lib/rules/no-await-sync-events.test.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/tests/lib/rules/no-await-sync-events.test.ts b/tests/lib/rules/no-await-sync-events.test.ts index 6389b1d7..bf9ebe43 100644 --- a/tests/lib/rules/no-await-sync-events.test.ts +++ b/tests/lib/rules/no-await-sync-events.test.ts @@ -169,7 +169,6 @@ ruleTester.run(RULE_NAME, rule, { // valid tests for fire-event when only user-event set in eventModules ...FIRE_EVENT_FUNCTIONS.map((func) => ({ - name: `await fireEvent.${func} - "fire-event" disabled in eventModules option`, code: ` import { fireEvent } from '@testing-library/framework'; test('should not report fireEvent.${func} sync event awaited', async() => { @@ -181,7 +180,6 @@ ruleTester.run(RULE_NAME, rule, { // valid tests for user-event when only fire-event set in eventModules ...USER_EVENT_SYNC_FUNCTIONS.map((func) => ({ - name: `await userEvent.${func} - "user-event" disabled in eventModules option`, code: ` import userEvent from '@testing-library/user-event'; test('should not report userEvent.${func} sync event awaited', async() => { @@ -239,7 +237,6 @@ ruleTester.run(RULE_NAME, rule, { ...FIRE_EVENT_FUNCTIONS.map( (func) => ({ - name: `await fireEvent.${func} - "fire-event" set in eventModules option`, code: ` import { fireEvent } from '@testing-library/framework'; test('should report fireEvent.${func} sync event awaited', async() => { @@ -262,7 +259,6 @@ ruleTester.run(RULE_NAME, rule, { ...USER_EVENT_SYNC_FUNCTIONS.map( (func) => ({ - name: `await userEvent.${func} - "user-event" set in eventModules option`, code: ` import userEvent from '@testing-library/user-event'; test('should report userEvent.${func} sync event awaited', async() => {