-
Notifications
You must be signed in to change notification settings - Fork 1.1k
RFC - manually flush microtasks in afterEach #440
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
RFC - manually flush microtasks in afterEach #440
Conversation
This PR replaces calling async act() in afterEach, with a version that still flushes microtasks, but doesn't wrap with act(). Explanation: If there are any hanging microtasks, that means a test ran without asserting on a valid state. Any async portion that causes updates should be asserted on, or atleast resolved within the scope of a test. This PR should still fire missing act warnings for a test, but won't let it leak to the next one.
Can you give an example of when this would prevent an issue that the current implementation doesn't? Not that I disagree with this, I just want to better understand what it's fixing. |
This looks fine. A tiny bit complicated, but I'm cool with it, especially since it's coming right out of React. We're getting a failure thanks to code coverage. I'm not too concerned about this file though, so you can add Fun fact: I added |
Oh, also, THANK YOU SUNIL! |
I got that updated. I'm pretty 👍 about this change. |
@threepointone, as this is titled "RFC" I just want to be certain that you think this should be added or if this PR was more of an idea/suggestion. I'm for it personally. |
I missed the conversation here, sorry. I think we should do this, yes. I'll rebase the PR later today. |
Hey @threepointone, if you're busy I could do the rebase? |
Yeah could you take this over? Thanks in advance! |
Sorry that it took so long, I've created a new PR with the squashed commit. |
Closing in favor of #514 |
This PR replaces calling async act() in afterEach, with a version that still flushes microtasks, but doesn't wrap with act().
Explanation: If there are any hanging microtasks, that means a test ran without asserting on a valid state. Any async portion that causes updates should be asserted on, or atleast resolved within the scope of a test. This PR should still fire missing act warnings for a test, but won't let it leak to the next one.