Skip to content

fix(await-async-utils): false positive when destructuring #722

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 10 commits into from
Feb 8, 2023
Merged

fix(await-async-utils): false positive when destructuring #722

merged 10 commits into from
Feb 8, 2023

Conversation

patriscus
Copy link
Contributor

Checks

Changes

  • Use early return via a condition isAsyncUtilOrKnownAliasAroundIt to reduce indentation
  • Upon visiting VariableDeclarator nodes, detect destructured property aliases and assignments of known async wrapper functions
  • Add getMessageId function
  • Add test cases

Context

Fixes #715

@patriscus
Copy link
Contributor Author

I'd also appreciate it if someone could also help me in figuring out why the lint step fails - seems like it's around files I haven't even touched? 🤔

@Belco90 Belco90 self-requested a review January 29, 2023 12:48
@Belco90 Belco90 added the bug Something isn't working label Jan 29, 2023
@Belco90 Belco90 changed the title PR: Fix false positive in 'await-async-utils' fix(await-async-utils): false positive when destructuring Jan 29, 2023
@Belco90
Copy link
Member

Belco90 commented Jan 29, 2023

Thanks @patriscus! I'll try to take a look in the following days.

The linting errors reported... I'm afraid that's because we don't pin the dev dependencies, so a new version got installed, reporting more errors. I'll fix that in a separate PR, sorry about that.

@Belco90 Belco90 mentioned this pull request Jan 29, 2023
Merged
1 task
@Belco90
Copy link
Member

Belco90 commented Jan 29, 2023

Linting errors should be gone now if you update your branch with main one.

Belco90
Belco90 previously approved these changes Feb 3, 2023
View reviewed changes
Copy link
Member

@Belco90 Belco90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Whoa, the PR looks great! You just need to iterate the tests over the ...ASYNC_UTILS.map() and should be ready to go. Thanks!

Belco90
Belco90 reviewed Feb 3, 2023
View reviewed changes
Copy link
Member

@Belco90 Belco90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved by mistake.

Belco90
Belco90 approved these changes Feb 8, 2023
View reviewed changes
Copy link
Member

@Belco90 Belco90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing! Looking great now, thanks for your contribution!

@Belco90 Belco90 merged commit 34a0a55 into testing-library:main Feb 8, 2023
@github-actions
Copy link

github-actions bot commented Feb 8, 2023

🎉 This PR is included in version 5.10.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

github-actions bot commented Aug 5, 2023

🎉 This PR is included in version 6.0.0-alpha.15 🎉

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
Reviewers

@Belco90 Belco90 Belco90 approved these changes

Assignees

@patriscus patriscus

Labels
bug Something isn't working released on @alpha released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

False positive for await-async-utils
2 participants
@patriscus @Belco90