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

fix(await-async-events): improve fixer #675

Merged
merged 1 commit into from
Oct 14, 2022

Conversation

skovy
Copy link
Collaborator

@skovy skovy commented Oct 12, 2022

Changes

  • Improve the fixer for await-async-events to also add missing async keyword to wrapping function expression

Context

Resolves #674

@skovy skovy added v6 Next major v6 enhancement New feature or request labels Oct 12, 2022
@skovy skovy linked an issue Oct 12, 2022 that may be closed by this pull request
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, that was quick! Everything looks fine, but I would like to ask for some more tests:

  • An invalid test case where the outer test already has the async operator, so we test is respected
  • An invalid test case where there is no outer function (i.e. just a separate async event method not awaited), so we test the fixer doesn't crash if no outer function is found

@skovy
Copy link
Collaborator Author

skovy commented Oct 12, 2022

An invalid test case where the outer test already has the async operator, so we test is respected

This should exist in several of the tests. I noticed the diff is a bit hard to read the way it's summarized so it might need to be expanded.

An invalid test case where there is no outer function (i.e. just a separate async event method not awaited), so we test the fixer doesn't crash if no outer function is found

Good point. Will do.

@skovy skovy force-pushed the pr/improve-await-async-events-fixer branch 3 times, most recently from e1f3639 to e33ea7b Compare October 12, 2022 15:33
@skovy skovy requested a review from Belco90 October 13, 2022 01:58
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.

Awesome! Thanks for sorting it out so quick!

@skovy skovy force-pushed the pr/improve-await-async-events-fixer branch from e33ea7b to d3ef713 Compare October 13, 2022 23:30
@skovy
Copy link
Collaborator Author

skovy commented Oct 13, 2022

I updated the fixer logic to only add an await before userEvent if the containing function expression can be found. The first approach you suggested.

Don't insert await if we can't find & validate the async-ness of the containing function, just leave it as an unfixable error

This should never happen in practice, and if it does, we'll ignore it instead of being potentially syntactically incorrect.

@Belco90 Belco90 merged commit 9d5554c into alpha Oct 14, 2022
@Belco90 Belco90 deleted the pr/improve-await-async-events-fixer branch October 14, 2022 11:53
@github-actions
Copy link

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

The release is available on:

Your semantic-release bot 📦🚀

@github-actions
Copy link

🎉 This PR is included in version 6.0.0 🎉

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
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Improve await-async-events fixer
4 participants