reapit / elements

Reapit Elements UI Component Library
https://elements.reapit.cloud/?path=/docs/welcome--docs
0 stars 2 forks source link

Replace Jest with Vite + Vitest #159

Open kurtdoherty opened 2 months ago

kurtdoherty commented 2 months ago

Context

TL;DR Jest is hard to configure so that the Linaria CSS extraction on components is performed during test runs. Switching to a Vite + Vitest combination is, by comparison, trivial to achieve. This CSS extraction is important when rendering components via React Testing Library (RTL), because without it, the correct styles are not applied to the component under test, undermining many RTL DOM queries.

More details on this issue can be found in #152.

Considerations

While the setup of Vite and Vitest is trivial, it does have a number of important considerations.

  1. @adrian-reapit reported in #152 that they've seen poor performance from Vitest in GitHub Actions. While the Console FE team hasn't had issues in our CI, we're currently using Jenkins, not GitHub Actions, so it's not immediately clear if we'll see performance problems running Vitest in Elements'.
  2. Vitest does not provide global functions by default like Jest does. Rather, Vitest prefers explicit imports for utilities like test, describe and vi (the equivalent of jest). That said, if we don't like the idea of having to add explicit imports for test functions, we can use Vitest's globals option to enable them as globals as per Jest's behaviour.
  3. Since Vitest is ESM first, setting up test doubles for a module's default export looks a little different with Vitest's vi.mock than it does with Jest's jest.mock. This is not a problem, just a matter of education for contributors.
  4. There's no easy way (that I'm aware of) to allow an incremental migration of test files from Jest to Vitest, so I believe this will need to be big bang cut-over. The Console FE team has successfully done this across our significant number of packages that back Console Cloud, so I don't believe it will be too difficult.
  5. If we use Vite + Vitest for our unit tests, we'll also be able to use the same Vite config to build Elements, instead of our current direct use of Rollup. I would consider this to be out-of-scope for this work and, if we go ahead with this proposal, would prefer to see us replace our Rollup config separately.

Proposal

The following work would be done as a single, large PR:

  1. Add Vite, Vitest and Happy DOM as dev dependencies, and setup the Vite + Vitest config;
  2. Rewrite all tests to use vi instead of jest and import test, describe, etc from Vitest (unless we would prefer to turn on globals?);
  3. Update our test package scripts to run Vitest instead of Jest;
  4. Remove all Jest-related config.

note: If we see problems with Happy DOM (e.g. we need APIs that it doesn't support), we can just use JSDOM instead. Happy DOM is faster than JSDOM for some things at the moment but the Console FE team has needed JSDOM for some of our packages.

kurtdoherty commented 1 month ago

@willmcvay @adrian-reapit would be good to get a decision on this before we start writing too many new components 🙏 I can do the work, just looking for input on whether we're happy to make the switch.

kurtdoherty commented 2 days ago

@willmcvay @adrian-reapit We're currently having to make concessions on the quality of the unit tests written for new work because our current Jest setup (including the Linaria mocks discussed in #158) undermines the value we can get out of testing with React Testing Library.

Unless either of you have any blocking concerns about my proposal here, I'm going to move forward with the changes next sprint.