Skip to content

feat(await-async-event): add basic fixer - #656

Merged
Belco90 merged 1 commit into
testing-library:alphafrom
skovy:pr/await-async-event-fixer
Oct 2, 2022
Merged

Belco90 merged 1 commit into
testing-library:alphafrom
skovy:pr/await-async-event-fixer

Conversation

@skovy

@skovy skovy commented Oct 1, 2022 •

Copy link
Copy Markdown
Collaborator

Checks

  • I have read the contributing guidelines.
  • If some rule is added/updated/removed, I've regenerated the rules list (npm run generate:rules-list)
  • If some rule meta info is changed, I've regenerated the plugin shared configs (npm run generate:configs)

Changes

  • Add fixer for await-async-event rule

I was thinking if this fixer should update the it/test to also be aysnc if not, but that felt like out of the scope of this rule/fixer? It cares if it's awaited, not managing if it's an async function wrapping?

Context

Branched off and a follow-up to #652 to make the migration easier to user-event v14.

Related to one of the rules mentioned in #202.

@MichaelDeBoey MichaelDeBoey changed the title feat: await-async-event basic fixer feat(await-async-event): add basic fixer Oct 1, 2022
@MichaelDeBoey
MichaelDeBoey marked this pull request as draft October 1, 2022 18:27
@Belco90

Belco90 commented Oct 2, 2022

Copy link
Copy Markdown
Member

Waiting until #652 gets merged (you could point this PR to the branch from that PR to make explicit it's branched off from there).

I'm happy without managing the async operator from the outer function. I think other autofixes from this plugin work that way.

@skovy

skovy commented Oct 2, 2022

Copy link
Copy Markdown
Collaborator Author

you could point this PR to the branch from that PR to make explicit it's branched off from there

The only branch options are the ones in this repo, I think since that pull request is a branch from my fork I can't base it on that branch? I can wait for that one to be merged and I can clean up this PR after.

@skovy
skovy marked this pull request as ready for review October 2, 2022 19:17

@Belco90 Belco90 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@skovy thank you for adding the autofix implementation for this rule!

@Belco90
Belco90 merged commit 3fae55d into testing-library:alpha Oct 2, 2022
@github-actions

github-actions Bot commented Oct 2, 2022

Copy link
Copy Markdown

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

The release is available on:

Your semantic-release bot 📦🚀

@epmatsw

epmatsw commented Oct 6, 2022

Copy link
Copy Markdown

Fwiw, I think this fixer should either 1) convert the function to async or 2) refuse to fix the issue if the function isn't async. As-is, it just produces syntactically invalid code and requires manual attention in those cases anyways.

@Belco90

Belco90 commented Oct 7, 2022

Copy link
Copy Markdown
Member

Fwiw, I think this fixer should either 1) convert the function to async or 2) refuse to fix the issue if the function isn't async. As-is, it just produces syntactically invalid code and requires manual attention in those cases anyways.

Actually… I checked again our current fixable rules and there is none adding the await operator (I don't know why, but I assumed there was one at least already). So I agree with @epmatsw: the fixer should add the async to the outer function (if found).

It should be fairly easy to do this. What do you think @skovy?

@Belco90

Belco90 commented Oct 11, 2022

Copy link
Copy Markdown
Member

I've created #674 for improving the fixer. @epmatsw thanks for your thoughts on this!

@skovy
skovy deleted the pr/await-async-event-fixer branch October 11, 2022 15:40
@skovy

skovy commented Oct 11, 2022

Copy link
Copy Markdown
Collaborator Author

Thanks @Belco90 - I'll try to take a look at the issue over the next few days/weeks if someone doesn't get to it before that 👍

@epmatsw

epmatsw commented Oct 11, 2022

Copy link
Copy Markdown

No problem! Thanks @skovy and @Belco90 for the hard work, can’t wait to get started using this!

@y-hsgw y-hsgw mentioned this pull request Sep 16, 2025
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.

4 participants