Opt-out of Jest FakeTimers

[timdeschryver]

Describe the feature you'd like:

I had to pin the DTL version to 7.29.4 in ATL because some tests that were using waitFor and waitForElementToBeRemoved were failing (I think this was also the root cause for #901).
This issue still persists to this date (I just tried to upgrade the DTL to its latest version).

After some time debugging, I think that the problem is that DTL falsely assumes that the Angular tests are using jest's fake timers. This is because Angular patches the timers API.

Suggested implementation:

I would like to add an option to the config to opt-out of this behavior since I don't see a better way to check if fake timers are used.

Describe alternatives you've considered:

Could we patch the useFakeTimers method, and use that to know if fake timers are used? This would replace the existing check.

Teachability, Documentation, Adoption, Migration Strategy:

/

[timdeschryver]

I think the problem is also a bit larger than using fake timers.
Even calling jest.useRealTimers() will mess up some tests.
Angular has a built-in way to advance time (not by using jest.advanceTimers), by invoking jest.useRealTimers() the Angular timers are confused and don't advance in time correctly.

[eps1lon]

I'm more and more convinced that we should remove all the jest timer handling and replace with a timer API that people configure.

Assuming jest is just not appropriate anymore with our reach and it causes us too often problems. Something like

// test.js
beforeEach(() => {
  domTestingLibrary.useFakeTimers(jest)
})
test(...);
afterEach(() => {
  domTestingLibrary.useRealTimers()
})

// @testing-library/dom 
interface TimersAdapter {
  // whatever methods we currently use from jest
}

export function useFakeTimers(newTimersAdapter: TimersAdapter) {
  timersAdapter = newTimersAdapter;
  // This call is for convenience
  timersAdapter.useFakeTimers();
}

export function useRealTimers() {
  timersAdapter = noopTimers;
  timersAdapter.useRealTimers();
}

And then different timer libraries would implement their own TimersAdapter.

We can then think about warning if we detect that timers were mocked but no call to domTestingLibrary.useFakeTimers() happened.

[timdeschryver]

I like the suggestion @eps1lon as it decouples DTL from jest.

[MatanBobi]

I like the suggestion @eps1lon as it decouples DTL from jest.

I agree that decoupling DTL from jest is a good idea, I'd prefer configuring the the test environment in the configure and not wrapping useFakeTimers and useRealTimers so users won't need to adapt every test suite they have at the moment..

[9still]

@timdeschryver - I filed #961, and I agree that the issue is essentially a dup of this one. I also like the suggestion made by @eps1lon, but if I am reading it correctly, it essentially outsources the problem to a family of new libraries - e.g. @testing-library/jest-modern-fake-timers-adapter, but I am curious whether these new adapters would still belong in the testing-library ecosystem (I suspect most people won't want to write their own).

My main point is - if the intent is to split the logic off to separate adapters, would it make sense to harden the logic & fix things like #961 to the extent possible first & then do the split? If we make some more assumptions about the underlying timer implementation & essentially implement separate solutions for jest legacy & jest modern timers, I think #961 would not be that difficult to fix & it would likely solve the main issue raised here.

(It just seems like it's going to become high priority to fix the issue, because Jest 27 is out & defaults to modern timers, but it might take a while to fully implement the solution @eps1lon is proposing)

[IanVS]

Am I correct in thinking that it's not possible to use @sinonjs/fake-timers directly, because testing-library only has special handling for jest? Jest is now using sinon's fake-timers, so it seems like it should work, but I'm having a very hard time getting waitFor to work correctly. I would love if testing-library were decoupled further from jest, as is being suggested here.

[timdeschryver]

@IanVS Dom Testing Library v8 removes the coupling with Jest (and also makes it compatible with Jest v27 and the modern timers).
This version was released a couple of hours ago, so I would suggest to update to v8.
If it's still causing troubles after the upgrade, feel free to open a new issue.

Closed by #966

[IanVS]

Hi, @timdeschryver, thanks for the note. I see that the referenced PR did do some cleanup, but as far as I can tell it is still coupled to jest. See

dom-testing-library/src/wait-for.js

Line 52 in d347302

const usingJestFakeTimers = jestFakeTimersAreEnabled()

and

dom-testing-library/src/wait-for.js

Line 74 in d347302

jest.advanceTimersByTime(interval)

, for example. That seems to be where advancing timers happens, and it only works with jest timers, and not sinon's fake timer directly. It seems like maybe this issue should stay open, unless I'm misunderstanding the intent here.

I tried version 8, but without success so far.

[timdeschryver]

@IanVS ah sorry, I've misunderstood the question/problem.
I think what you're looking for is something as suggested in #939 (comment).

Since it might have a big impact, I would suggest you to create a new issue for it.
This is something that should be discussed and agreed on.

[IanVS]

Hmmm, why does that need a new issue? Isn't it the whole point of this issue? It seems like a mistake to lose the context and subscribers from this current issue... I think this one was marked as closed incorrectly, as the PR you referenced did not actually solve the issue here.

[eps1lon]

This issue was about jest fake timers. We never support any other fake timers so a new issue is warranted.

[IanVS]

OK thanks, I've opened #987.