Skip to content

False positives on await-async-query and no-await-sync-query #122

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

Closed
Belco90 opened this issue May 4, 2020 · 8 comments · Fixed by #208
Closed

False positives on await-async-query and no-await-sync-query #122

Belco90 opened this issue May 4, 2020 · 8 comments · Fixed by #208
Assignees
Labels
bug Something isn't working released

Comments

@Belco90
Copy link
Member

Belco90 commented May 4, 2020

Related to #120, #114

Since we are checking testing library queries when calling queries member expressions for await-async-query and no-await-sync-query, we are getting some false positives in some cases:

  • when using queries in cypress files
  • when using methods with the same name in other places (e.g. findByTestId in enzyme tests in a codebase mixing both enzyme and testing library)

The main issue here is that we are checking the queries methods in every single file no matter where the method is coming from, but we should only check those imported from testing library or returned by something imported from testing library. However, I think a first approach could be at least check if testing library is imported in the file to check the methods or not.

@Belco90 Belco90 added the bug Something isn't working label May 4, 2020
@afontcu
Copy link
Member

afontcu commented Jun 6, 2020

Hi! Looks like #137 was merged but this issue is still valid, right? (Just to double check 😇)

@Belco90
Copy link
Member Author

Belco90 commented Jun 6, 2020

Yes, it was only fixed for no-await-sync-query but I'm afraid the issue is still there for await-async-query.

@kentcdodds
Copy link
Member

I'm seeing this as well. Also prefer-screen-queries is firing on cy.findByRole... etc.

@Belco90
Copy link
Member Author

Belco90 commented Jun 30, 2020

Yes, still pending to fix await-async-query.

About prefer-screen-queries one, that's new I think. There is this workaround for another cypress related issue that might work in the meantime.

I hope I can find some time soon to fix those, specially the await-async-query one which is one of the first rules so it needs to be updated properly.

@thomaslombart
Copy link
Collaborator

As of today, there are two rules that have similar implementations:

#137 seems to have fixed await-async-utils but didn't include the fix for await-async-query. I see two solutions to this:

  1. We merge the rules await-async-utils and await-async-query since they both check the usage of await for imported methods. This would result in a breaking change though.
  2. We create a util/function that allows to check if an async imported method (whatever the name) has been used throughout the code. Then we make use of that util in these two rules.

WDYT?

@Belco90
Copy link
Member Author

Belco90 commented Jul 20, 2020

  1. We could achieve that in next v4 as is still pending.
  2. We have now a module for common functionality named node-utils.ts. We can move there that shared code.

I'd prefer go for 2 to leave those 2 rules separated for now.

@thomaslombart
Copy link
Collaborator

Option 2 sounds good to me. I'll work on it in the next few days 👍

@Belco90
Copy link
Member Author

Belco90 commented Aug 5, 2020

🎉 This issue has been resolved in version 3.4.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working released
Projects
None yet
4 participants