Skip to content

fix: Prevent "missing act" warning for queued microtasks#1137

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
eps1lon merged 5 commits into from
Feb 16, 2023
Merged

fix: Prevent "missing act" warning for queued microtasks #1137

eps1lon merged 5 commits into from
Feb 16, 2023
+182 −61

Conversation

@eps1lon
Copy link
Member

@eps1loneps1lon commented Oct 8, 2022•
edited
Loading

What:

Closes#1125
Closes#1051

Why:

Once the callback passed to waitFor resolves, there could be other microtasks scheduled.
If we now yield back from waitFor we restore the REACT_IS_ACT_ENVIRONMENT. However, the tester awaits the waitFor and before the runtime yields back to the event loop, it needs (by spec) to drain the scheduled microtasks. This may include state updates which now run without REACT_IS_ACT_ENVIRONMENT which triggers warning.

How:

Drain microtask queue before resolving waitFor call.

Checklist:

@codesandbox-ci
Copy link

codesandbox-cibot commented Oct 8, 2022•
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit bfd1db7:

SandboxSource
ReactConfiguration
react-testing-library-examplesConfiguration

@codecov
Copy link

codecovbot commented Oct 8, 2022•
edited
Loading

Codecov Report

Merging #1137 (bfd1db7) into alpha (1934bf2) will not change coverage.
The diff coverage is 100.00%.

@@ Coverage Diff @@## alpha #1137 +/- ## ========================================= Coverage 100.00% 100.00% ========================================= Files 4 4 Lines 181 192 +11 Branches 34 38 +4 ========================================= + Hits 181 192 +11
FlagCoverage Δ
experimental100.00% <100.00%> (ø)
latest100.00% <100.00%> (ø)
next100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted FilesCoverage Δ
src/pure.js100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@eps1loneps1lonforce-pushed the fix/nanotasks-troll branch from ab13864 to 065c316CompareOctober 8, 2022 11:54
@eps1loneps1lon mentioned this pull request Oct 8, 2022
Merged
2 tasks
@eps1loneps1lonforce-pushed the fix/nanotasks-troll branch from 065c316 to fb22f8aCompareOctober 8, 2022 11:59
MatanBobi
MatanBobi approved these changes Oct 9, 2022
View reviewed changes
Copy link
Member

@MatanBobiMatanBobi left a comment

Choose a reason for hiding this comment

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

Looks good! :)

@robin-drexler
Copy link

robin-drexler commented Nov 21, 2022

👋 @eps1lon, curious would this PR also address #1051 or this is a different issue? 🤔

Thanks a lot!

@dgdaviddgdavid mentioned this pull request Nov 24, 2022
Merged
@fastndead
Copy link

fastndead commented Dec 30, 2022

Hey! is there an ETA for this fix? We've ran into the bunch not wrapped in act() warnings after React 18/RLT 13 update, too many to spot -fix, but these changes actually reduce the amount of failed test by 65%, so I'm really looking forward to this being released

@heath-pack
Copy link

heath-pack commented Jan 25, 2023

Would love to get this in soon to try and reduce the noise from our specs, then I can see if we have any "actual" act warnings left

@diogoredin
Copy link

diogoredin commented Feb 1, 2023

Hey @eps1lon, any chance this going to get merged soon? We are also facing some unexpected failing tests & act warnings after moving to RTL 13 (using the legacy root still). Thank you for your work 🙏

@eps1lon
Copy link
MemberAuthor

eps1lon commented Feb 15, 2023

Hey! is there an ETA for this fix? We've ran into the bunch not wrapped in act() warnings after React 18/RLT 13 update, too many to spot -fix, but these changes actually reduce the amount of failed test by 65%, so I'm really looking forward to this being released

This is very helpful information, thank you!

We have a new major planned that will include some small breaking changes from /dom. I was hesitant to merge this PR but including it in a new major makes it a lot safer to land.

I'll clean this up, test on some larger repos and check how it performs.

@eps1loneps1lon changed the base branch from main to alphaFebruary 15, 2023 20:13
@eps1loneps1lon changed the title fix: Prevent "missing act" warning for in-flight promisesfix: Prevent "missing act" warning for queued microtasksFeb 16, 2023
@eps1loneps1lon mentioned this pull request Feb 16, 2023
Closed
eps1lon added a commit to eps1lon/act-warning-mcve that referenced this pull request Feb 16, 2023
@eps1lon
fix: No more "missing act" warning
312f3d7
Using testing-library/react-testing-library#1137
This was referenced Feb 16, 2023
Draft
Draft
Closed
@eps1loneps1lon linked an issue Feb 16, 2023 that may be closed by this pull request
React 18. set state in finally throws "act" warning, though test is passing #1051
Closed
@eps1loneps1lon marked this pull request as ready for review February 16, 2023 17:05
@eps1loneps1lon merged commit f78839b into testing-library:alphaFeb 16, 2023
@eps1loneps1lon deleted the fix/nanotasks-troll branch February 16, 2023 22:46
This was referenced Feb 16, 2023
Merged
Closed
@VinnlVinnl mentioned this pull request Feb 20, 2023
Closed
@bpintobpinto mentioned this pull request Feb 26, 2023
Closed
@jsnajdrjsnajdr mentioned this pull request Mar 15, 2023
Merged
jsnajdr
jsnajdr reviewed Mar 18, 2023
View reviewed changes
src/pure.js
if(jestFakeTimersAreEnabled()){
jest.advanceTimersByTime(0)
}
})
Copy link

@jsnajdrjsnajdrMar 18, 2023

Choose a reason for hiding this comment

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

An observation: if I comment out this new await code that drains the microtask queue, all the end-to-end.js tests are still passing. Both macrotask and microtask, in all timer variants.

Seems the tests would need to be amended somehow to really reproduce the bug.

Copy link
MemberAuthor

@eps1loneps1lonMay 22, 2023

Choose a reason for hiding this comment

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

Added tests in #1215

@wojtekmajwojtekmaj mentioned this pull request Mar 29, 2023
Open
@eps1loneps1lon mentioned this pull request May 22, 2023
Merged
iumehara added a commit to iumehara/full-stack-tdd that referenced this pull request Jun 5, 2023
@iumehara
Update dependency '@testing-library/react' to 14.0.0, which fixes bug…
9f4ad64
… where act() warnings are displayed incorrectly testing-library/react-testing-library#1137
@ak-beam
Copy link

ak-beam commented Aug 10, 2023

@eps1lon Would it be possible to backport this to the 12.x release series for codebases still on React 17?

@MatanBobi
Copy link
Member

MatanBobi commented Aug 13, 2023

@ak-beam this fix was only merged to alpha and isn't merged to master yet.
Having said that, I don't think this will be backported because it uses IS_REACT_ACT_ENVIRONMENT which is only available in React 18.

@mgadewollmgadewoll mentioned this pull request Apr 15, 2024
Merged
1 task
@dyantakodyantako mentioned this pull request Sep 9, 2025
Open
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MatanBobiMatanBobiMatanBobi approved these changes

+2 more reviewers

@jsnajdrjsnajdrjsnajdr left review comments

@markmssdmarkmssdmarkmssd approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

9 participants

@eps1lon@robin-drexler@fastndead@heath-pack@diogoredin@ak-beam@MatanBobi@jsnajdr@markmssd