fix: Only use a single clock

[eps1lon]

BREAKING CHANGE:

1. Remove deprecated waitFormDOMChange
2. The timeout in waitFor(callback, { interval, timeout } ) now uses the same clock as interval
   Previously timeout was always using the real clock while interval was using the global clock which could've been mocked out. For the old behavior I'd recommend waitFor(callback, { interval, timeout: Number.PositiveInfinity }) and rely on your test runner to timeout considering real timers.

TODO:

• Should Number.PositiveInfinity be the new default for timeout?

What:

Only run with a single clock.

Closes #939
Closes #756
Closes #880

Hopefully also fixes flaky test failures such as https://github.com/testing-library/dom-testing-library/runs/2727330116

Why:

The previous approach breaks jest 27. I would also consider it suprising behavior (agreeing with Closes #880). If I pass waitFor(callback, { interval: 50, timeout: 900 }) then I expect both times to run with the same clock. Previously the behavior of waitFor was different depending on fake or real timers which I consider problematic.

I also don't think we need to force the timeout to run with the real clock. Test runners already implement this behavior. So if people don't want the timeout to use the same clock as interval then they can use waitFor(callback, { timeout: Number.PositiveInfinity }).

How:

Backported f1a8a14 (#962) from #962. This ensures that the new approach works with jest 26 (this PR) and jest 27 (#962)

• use globally declared setTimout instead of setImmediate and setTimeout from the real clock
• remove deprecated waitForDOMChange since it relied on the now removed runWithRealTimers

Checklist:

• Documentation added to the
  docs site
• Tests
• [ ] Typescript definitions updated
• Ready to be merged
  

/cc @testing-library/all-maintainers Since this might be fairly invasive.

[codesandbox-ci]

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 24f082b:

Sandbox	Source
react-testing-library-examples	Configuration
react-testing-library demo	Issue #756

[codecov]

Codecov Report

Merging #966 (24f082b) into main (c273ed5) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #966   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        25    -1     
  Lines          966       914   -52     
  Branches       293       282   -11     
=========================================
- Hits           966       914   -52     

Flag	Coverage Δ
node-10.14.2	100.00% <100.00%> (ø)
node-12	100.00% <100.00%> (ø)
node-14	100.00% <100.00%> (ø)
node-15	100.00% <100.00%> (ø)
node-16	100.00% <100.00%> (ø)

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

Impacted Files	Coverage Δ
src/helpers.js	100.00% <100.00%> (ø)
src/wait-for.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 86fb094...24f082b. Read the comment docs.

[timdeschryver]

I'm no expert here, but I can say that the failing tests in the ATL repository are resolved with this change (on Jest 26).
If this gets released, should we also remove the remaining deprecated methods (wait and waitForElement) ?

[eps1lon]

If this gets released, should we also remove the remaining deprecated methods (wait and waitForElement) ?

Yep, this PR already targets an alpha branch so we'll collect some more breaking changes if this gets released.

[kentcdodds]

Previously the behavior of waitFor was different depending on fake or real timers which I consider problematic.

This is enough to convince me. The goal of our utilities is to be able to use the async utilities and have all the tests run the same whether the timers are faked or not. And the workaround for the old behavior is pretty sensible IMO.

This looks good to me, and I love all the code that can be removed with this change 👍

[eps1lon]

Update: I also noticed flaky test failures such as https://github.com/testing-library/dom-testing-library/runs/2727330116 (also remember one PR) and these instances are hopefully also fixed by this change

[kentcdodds]

Looks like everything's been resolved here and considering this is just going to the alpha branch I'm going to go ahead with the merge 👍

Thanks for the collaboration!

[github-actions]

🎉 This PR is included in version 8.0.0-alpha.1 🎉

The release is available on:

• npm package (@alpha dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[ValentinH]

While updating Jest to v27, I faced many broken tests where waitForElementToBeRemoved() was used along with fake timers. After finding this PR, I tried to force the usage of @testing-library/dom@v8.0.0-alpha.1 and it fixed my issues.

As I'm not sure when v8 will be released to the stable channel, I'm wondering if the fix to use a single clock could be back-ported to v7? Or is this fix tightly linked with the removal of waitFormDOMChange and must wait for a major version release ?

[eps1lon]

I'm wondering if the fix to use a single clock could be back-ported to v7?

It includes breaking changes that are not coupled with waitForDOMChange. See item 2 on the breaking changes list in the PR description:

The timeout in waitFor(callback, { interval, timeout } ) now uses the same clock as interval
Previously timeout was always using the real clock while interval was using the global clock which could've been mocked out. For the old behavior I'd recommend waitFor(callback, { interval, timeout: Number.PositiveInfinity }) and rely on your test runner to timeout considering real timers.