Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

userEvent.hover returns a promise - no-await-sync-events vs no-floating-promises reasoning #804

Closed
mkosir opened this issue Aug 23, 2023 · 2 comments
Assignees
Labels
question Further information is requested

Comments

@mkosir
Copy link

mkosir commented Aug 23, 2023

Have you read the Troubleshooting section?

Yes

Plugin version

v6.0.0

ESLint version

v8.47.0

Node.js version

18.17.1

package manager and version

npm 9.6.7

Operating system

macOS Ventura, version 13.5.1

Bug description

For userEvent.hover no-await-sync-events rule reports that it's a sync event, from what I assume the typings could be wrong?
image

Not sure how much benefits does no-await-sync-events rule brings, since we already have no-floating-promises from typescript-eslint package.

When await keyword is being removed, no-floating-promises rule of course fails since type definition is indicating that hover function is returning a promise.

Steps to reproduce

  • clone the repo
  • run npm i
  • run npm run lint -> no error
  • In package.json upgrade eslint-plugin-testing-library to v6.0.0
  • run npm i
  • run npm run lint -> error

Error output/screenshots

image

If we remove await
image

ESLint configuration

"rules": {
    "@typescript-eslint/no-floating-promises": "error"
  }

Rule(s) affected

https://github.com/testing-library/eslint-plugin-testing-library/blob/main/docs/rules/no-await-sync-events.md

Anything else?

No response

Do you want to submit a pull request to fix this bug?

No

@mkosir mkosir added bug Something isn't working triage Pending to be triaged by a maintainer labels Aug 23, 2023
@Belco90
Copy link
Member

Belco90 commented Aug 23, 2023

Hi @mkosir. This is a bug in our plugin, related to #801. Basically, the mentioned error is coming from user-event prior to v14. We are fixing this in #803 so user-event methods reported are based on v14, then all of them will be treated as async when that fix gets released.

Regarding using no-floating-promises TS ESLint rule. Yeah, that rule will catch all the unhandled async usages for you, but only if you use TypeScript. The rules from our plugin about async stuff will also help developers working in plain JS to handle the floating promises in their Testing Library tests. We should probably mention in the docs of all the async rules that they can be disabled if no-floating-promises is already enabled.

@Belco90 Belco90 added question Further information is requested and removed bug Something isn't working triage Pending to be triaged by a maintainer labels Aug 23, 2023
@Belco90 Belco90 self-assigned this Aug 23, 2023
@mkosir
Copy link
Author

mkosir commented Aug 23, 2023

Hi @Belco90 ,thanks for such a fast response 🚀

Regarding using no-floating-promises TS ESLint rule. Yeah, that rule will catch all the unhandled async usages for you, but only if you use TypeScript. The rules from our plugin about async stuff will also help developers working in plain JS to handle the floating promises in their Testing Library tests. We should probably mention in the docs of all the async rules that they can be disabled if no-floating-promises is already enabled.

I see, haven't thought about plain JS 👍

Big fan of this package, keep up the awesome work!

@Belco90 Belco90 closed this as completed Aug 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants