react-testing-library icon indicating copy to clipboard operation
react-testing-library copied to clipboard

Update to v14 breaks @testing-library/user-event on Vitest

Open wojtekmaj opened this issue 1 year ago • 28 comments

What you did:

A simple update from v13 to v14 broke my Vitest-based test where I was using await user.click(...) as the promise no longer resolves.

Reproduction:

Run repo at the following commit: https://github.com/wojtekmaj/react-async-button/commit/fa41b3b9900a25d76141bcf2080f94f7ee5f5dee

Suggested solution:

After long debug session, I have determined that

  • Monkey patching asyncWrapper to be just cb => cb() resolves the issue.
  • Removing the following code added in testing-library/react-testing-library#1137 resolves the issue:

https://github.com/testing-library/react-testing-library/blob/f78839bf4147a777a823e33a429bcf5de9562f9e/src/pure.js#L41-L52

  • Moving if (jestFakeTimersAreEnabled()) { ... } to wrap the entire block mentioned above resolves the issue.
  • Calling vi.advanceTimersByTime(0); manually after user.click(...) but before awaiting returned promise, even multiple times, does NOT help
  • The only workaround that worked for me was this: https://github.com/wojtekmaj/react-async-button/commit/2d26f217a375b7020ddf42f76891254586fc3ce4

So my suggestion is to:

  • Roll back the fix and perhaps reintroduce when advanceTimers will be configurable and not jest dependent
  • OR move if (jestFakeTimersAreEnabled()) { ... } to wrap the entire block mentioned above, acknowledging that the fix is now Jest-only.

wojtekmaj avatar Mar 21 '23 23:03 wojtekmaj

Moving to user-event until we have a repro that's just using @testing-library/react

eps1lon avatar Mar 25 '23 17:03 eps1lon

I was about to submit what I think is the same issue. await userEvent.click causes any test that is ran after calling jest.useFakeTimers to fail in v14.4.3. Took me a pretty long time to isolate it, but it appears to be the combination of using fake timers and awaiting userEvent, as remove either of those things and the tests pass.

Here is a repro using @testing-library/react: https://github.com/Lokua/user-event-repro

^ downgrading to v13 or using fireEvent, no issue.

Happy to create a separate issue if you feel this is not the same.

Edit: If this is the same issue, I suggest renaming it to better reflect the actual bug so others can find it more easily. Perhaps v14 causes tests using fake timers to fail when awaiting userEvent.click

Lokua avatar Mar 28 '23 21:03 Lokua

@Lokua You need to set the advanceTimers option if you're working with fake timers.

We recommend using a setup function:

function setup(jsx) {
  return {
    user: userEvent.setup({
      advanceTimers: jest.advanceTimersByTime,
    }),
    ...render(jsx),
  }
}

test('some click', async () => {
  jest.useFakeTimers()
  const { user } = setup(<button/>)

  await user.click(screen.getByRole('button'))
})

ph-fritsche avatar Mar 29 '23 07:03 ph-fritsche

@wojtekmaj @eps1lon

This is an issue with the setTimeout call in asyncWrapper and is independent of user-event. It just happens to use it.

When we use setTimeout in user-event we also call the advanceTimers callback. I think we should add the same option to @testing-library/react.

import DTL from '@testing-library/dom'
const userEventConfig = {
  delay: 0,
  advanceTimers: jest.advanceTimersByTime,
}
const userEventWait = () => Promise.all([
   new Promise(r => setTimeout(r, userEventConfig.delay)),
   userEventConfig.advanceTimers(userEventConfig.delay),
])
const someUserEventApi = () => {
  return DTL.getConfig().asyncWrapper(() => {
     DTL.getConfig().eventWrapper(() => document.dispatchEvent(new Event('foo')))
     DTL.getConfig().eventWrapper(() => document.dispatchEvent(new Event('bar')))
     await userEventWait()
     DTL.getConfig().eventWrapper(() => document.dispatchEvent(new Event('baz')))
     await userEventWait()
   })
}

await someUserEventApi() // with non-jest fake timers this will time out because of https://github.com/testing-library/react-testing-library/blob/f78839bf4147a777a823e33a429bcf5de9562f9e/src/pure.js#L44-L52

ph-fritsche avatar Mar 29 '23 07:03 ph-fritsche

@Lokua You need to set the advanceTimers option if you're working with fake timers.

We recommend using a setup function:

function setup(jsx) {
  return {
    user: userEvent.setup({
      advanceTimers: jest.advanceTimersByTime,
    }),
    ...render(jsx),
  }
}

test('some click', async () => {
  jest.useFakeTimers()
  const { user } = render(<button/>)

  await user.click(screen.getByRole('button'))
})

Thank you very much for the info. I get it now. Just for clarity, in your example, setup should actually be called render, or vis a versa, yes?

Lokua avatar Mar 29 '23 16:03 Lokua

@Lokua Yes, the setup function should be called in the test, I fixed the code example above.

ph-fritsche avatar Mar 29 '23 21:03 ph-fritsche

@ph-fritsche I ran into the same issue but I couldn't get my scenario to work just with advanceTimers option. See repro link: https://stackblitz.com/edit/vitest-dev-vitest-2xdjiw?file=test%2Fcountdown.test.tsx

import Countdown from "src/components/Countdown";
import { render, screen, act } from "@testing-library/react";
import userEvent from "@testing-library/user-event";

describe("Mocking timers", () => {
  beforeEach(() => {
    vi.useFakeTimers();
  });

  afterEach(() => {
    vi.runOnlyPendingTimers();
    vi.useRealTimers();
  });

  it("mocks timers", async () => {
    const user = userEvent.setup({
      advanceTimers: ms => vi.advanceTimersByTime(ms),
    });

    render(<Countdown from={5} />);

    await user.click(screen.getByRole("button"));

    expect(screen.getByRole("alert")).toHaveTextContent("5");
    await act(() => vi.runAllTimers());
    expect(screen.getByRole("alert")).toHaveTextContent("0");
  });
});

If I do:

vi.useFakeTimers({ shouldAdvanceTime: true });

It makes the test pass but I believe it's not a good option to use.

runofthemillgeek avatar Mar 31 '23 20:03 runofthemillgeek

Same problem

azat-io avatar Apr 06 '23 15:04 azat-io

I believe this might be related to #1187 as vitest uses sinon fake timers

bfsgr avatar Apr 11 '23 19:04 bfsgr

Is there any ETA to fix this issue? It should be testing-framework agnostic instead of sticking with Jest

One temporarily workaround for Vitest users can be like this:

In your test suites using fake timers

import { beforeAll, vi, describe } from 'vitest';

describe('this suite uses fake timers', () => {
  // Temporarily workaround for bug in @testing-library/react when use user-event with `vi.useFakeTimers()`
  beforeAll(() => {
    const _jest = globalThis.jest;
  
    globalThis.jest = {
      ...globalThis.jest,
      advanceTimersByTime: vi.advanceTimersByTime.bind(vi)
    };
  
    return () => void (globalThis.jest = _jest);
  });
})

xsjcTony avatar Apr 13 '23 02:04 xsjcTony

How did Vitest fake timers ever work before? We generally only support Jest fake timers at the moment. Using any other fake timers means you're on your own.

We'd love to add support for more fake timers but that should come with a general purpose API so that we can support any fake timers (e.g. https://github.com/testing-library/react-testing-library/issues/1187).

eps1lon avatar May 19 '23 09:05 eps1lon

From what I can see here, react-testing-library is hard-coded to use jest.advanceTimersByTime after upgrading to v14 Current code: https://github.com/testing-library/react-testing-library/blob/main/src/pure.js#L49-L51 Corresponding PR: https://github.com/testing-library/react-testing-library/commit/f78839bf4147a777a823e33a429bcf5de9562f9e#diff-2ef28f1bd92d5dcd1f2a04d56814d3adaee10cc939b4a7d7c861af3a3cbbccb7 It's not working like user-event where user is allowed to pass their advanceTimersByTime functions, like documented here: https://testing-library.com/docs/user-event/options#advancetimers

According to this, my workaround above will work 100% perfectly just by binding jest.advanceTimersByTime to vi.advanceTimersByTime, with binding of this to vi.

So basically I've no idea why react-testing-library just sticks with Jest, which is not making sense to me, but as long as I got a stable workaround there so it's all goof for me.

But whatever, thanks for the great testing library!

xsjcTony avatar May 19 '23 10:05 xsjcTony

So basically I've no idea why react-testing-library just sticks with Jest

Because it is the most popular testing framework so supporting their fake timers out of the box made sense to help adoption.

I'd like to support more timers but so far no community contributions have been made to do that. And since I'm not using Vitest in any projects I'm involved in, I didn't have a use-case for myself. PRs are welcome though.

eps1lon avatar May 20 '23 06:05 eps1lon

Supporting one framework's fake timers is one thing, making the library framework-dependent by breaking it for all other testing frameworks is another. This code should NOT be run on non-Jest environments, and it does. Please see my original post.

wojtekmaj avatar May 20 '23 12:05 wojtekmaj

Does it break in vitest with real timers or with their fake timers? I

eps1lon avatar May 20 '23 13:05 eps1lon

Fake timers

xsjcTony avatar May 20 '23 13:05 xsjcTony

Ran into same problem. Shared my workaround here: https://github.com/testing-library/user-event/issues/1115#issuecomment-1565730917

iulspop avatar May 27 '23 23:05 iulspop

Supporting Vitest timers should be as easy as calling vi instead of jest, and historically we've had messier code supporting differences in DOM implementations, so I think it's worth trying. If anyone's interested I can experiment with a PR.

nickserv avatar May 28 '23 05:05 nickserv

@nickmccurdy would absolutely love that! It's literally the only thing stopping me from moving to Vitest 🙏

mryechkin avatar May 28 '23 13:05 mryechkin

We're using sinon fake timers, and we don't have jest. For more seamless experience (until it's fully supported), I added the following workaround:

import sinon from 'sinon'

const _sinonUseFakeTimers = sinon.useFakeTimers;
let _sinonClock;
sinon.useFakeTimers = function (...args) {
  _sinonClock = _sinonUseFakeTimers.call(sinon, ...args);
  globalThis.jest = { advanceTimersByTime: _sinonClock.tickAsync.bind(_sinonClock) };
  return _sinonClock;
};

Might be helpful for those who have similar setup. It allows to focus on the testing, not on hacking things, and can be deleted once sinon timers are supported. Idea was borrowed from https://github.com/testing-library/react-testing-library/issues/1197#issuecomment-1506222246.

davletovalmir avatar Jun 21 '23 03:06 davletovalmir

Supporting Vitest timers should be as easy as calling vi instead of jest, and historically we've had messier code supporting differences in DOM implementations, so I think it's worth trying. If anyone's interested I can experiment with a PR.

It would be great to be able to pass your framework in the configure function. Something like:

import {configure} from '@testing-library/dom'
import {vi} from 'vitest'

// ...

configure({framework: vi})

The object passed through would adhere to a specific interface.

Even if the code has to be messy right now, you will eventually need something that's much more agnostic. Jest won't remain the dominant framework forever.

falldowngoboone avatar Jul 23 '23 16:07 falldowngoboone

I found that userEvent can work nicely with vitest fake timers using:

    const user = userEvent.setup({
      advanceTimers: ms => vi.advanceTimersByTime(ms),
    });

~But I didn't find a way to make waitFor work correctly.~

Edit, I take that back, I was still using v13 it turns out. With v14, the suggested approach above does seem to work for waitFor;

    globalThis.jest = {
      ...globalThis.jest,
      advanceTimersByTime: vi.advanceTimersByTime.bind(vi)
    };

IanVS avatar Aug 25 '23 19:08 IanVS

Unfortunately the above solution didn't work for me. I've downgraded to v13.5.0 which has worked.

Is a fix being worked on for future releases?

senock-dag avatar Sep 29 '23 16:09 senock-dag

@IanVS 's method works for me. Should we add this to the doc?

xuhdev avatar Feb 15 '24 21:02 xuhdev

The following worked for my case:

vi.useFakeTimers({
      shouldAdvanceTime: true
});

danielholmes avatar Apr 25 '24 01:04 danielholmes

The following worked for my case:

vi.useFakeTimers({
      shouldAdvanceTime: true
});

I don't think that's a real solution. What shouldAdvanceTime option does is advance fake timers with normal time, which kinds of defeat the purpose of using fake timers, and make your test flaky.

So far I didn't find any solution which would work for Vitest.

robertjk avatar Apr 25 '24 18:04 robertjk

I don't think that's a real solution. What shouldAdvanceTime option does is advance fake timers with normal time, which kinds of defeat the purpose of using fake timers, and make your test flaky.

I agree it's not a real solution. But if you're dealing with dates in terms of days (my use case), weeks, months or years it's a perfectly fine workaround for now. If your use case needs the precision of milliseconds or seconds then it's not a good workaround.

danielholmes avatar Apr 25 '24 23:04 danielholmes

I don't think that's a real solution. What shouldAdvanceTime option does is advance fake timers with normal time, which kinds of defeat the purpose of using fake timers, and make your test flaky.

I agree it's not a real solution. But if you're dealing with dates in terms of days (my use case), weeks, months or years it's a perfectly fine workaround for now. If your use case needs the precision of milliseconds or seconds then it's not a good workaround.

Fair enough - in some use cases it could be a valid solution.

robertjk avatar Apr 27 '24 13:04 robertjk