Skip to content
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.

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

feat!: local types, supporting jest, @jest/globals, vitest #511

Merged
merged 3 commits into from
Aug 13, 2023

Conversation

jgoz
Copy link
Collaborator

@jgoz jgoz commented Aug 12, 2023

What:

Moves matcher types back into this package and adds support for @jest/globals and vitest, both in terms of automatically extending expect and augmenting the appropriate TypeScript interfaces.

There are three breaking changes here:

  1. Drop support for node < 14
  2. Removes the extend-expect script. Users should use the default import path or one of the new test platform-specific paths to automatically extend the appropriate "expect" instance. extend-expect was not documented in the Readme, so this change should have minimal impact.
  3. Remove the @types/ dependency. This might affect JavaScript users who were relying on the automatic inclusion of the @types/ package to get auto-complete in their editor for jest-dom assertions.

But in my local testing with the changes in this PR, following the Readme was sufficient to get full auto-complete support even for JS-only users. As long as they have a jest-setup file that imports the bare export and add a dependency on @types/jest, recent versions of VS Code will figure it out.

In addition to the above changes, @jest/globals and vitest users can now use jest-dom without caveats. Users can now use the following import paths to automatically extend "expect" for their chosen test platform:

  • @testing-library/jest-dom — jest+@types/jest
  • @testing-library/jest-dom/jest-globals — @jest/globals
  • @testing-library/jest-dom/vitest — vitest

For example:

import '@testing-library/jest-dom/jest-globals'

Importing from one of the above paths will augment the appropriate matcher interface for the given test platform, assuming the import is done in a .ts file that is included in the user's tsconfig.json.

It's also (still) possible to import the matchers directly without side effects:

import * as matchers from '@testing-library/jest-dom/matchers'

Why:

There have been numerous issues filed over the last couple years as the JS testing ecosystem has evolved and users wanted to use this library in more environments. It was usually possible, thanks to broad compatibility efforts in the ecosystem at large, but using jest-dom outside of Jest was not always straightforward.

Originally, moving the types to DT was in service of a noble "it should just work" goal, but in practice, it traded convenience in simple cases for confusion in more complex cases. It also broke with convention around DT packages — why does the actual package have a dependency on its own types? Why not simply move the types into the package? Those were valid questions that had legitimate answers, but they grew stale with time.

Fixes #314
Fixes #427
Closes #472
Fixes #506
Fixes #476
Closes #484
Closes #496
Closes #483
Closes #460

How:

  • Move latest typings from @types/testing-library__jest-dom into this repo
  • Add jest.d.ts for extending global interfaces when using @types/jest
  • Add jest-globals.d.ts for extending interfaces when using @jest/config
  • Add vitest.d.ts for extending interfaces when using vitest
  • Add optional peer dependencies on jest, @types/jest, @jest/globals, and vitest
  • Add /jest-globals and /vitest exports for automatically extending @jest/globals and vitest, respectively

Checklist:

  • Documentation
  • Tests
  • Updated Type Definitions
  • Ready to be merged

Moves matcher types back into this package and adds support for
@jest/globals and vitest.

BREAKING CHANGE: Removes the extend-expect script. Users should use
the default import path or one of the new test platform-specific
paths to automatically extend the appropriate "expect" instance.

extend-expect was not documented in the Readme, so this change should
have minimal impact.

Users can now use the following import paths to automatically extend
"expect" for their chosen test platform:

- @testing-library/jest-dom - jest (@types/jest)
- @testing-library/jest-dom/jest-globals - @jest/globals
- @testing-library/jest-dom/vitest - vitest

For example:

import '@testing-library/jest-dom/jest-globals'

Importing from one of the above paths will augment the appropriate
matcher interface for the given test platform, assuming the import
is done in a .ts file that is included in the user's tsconfig.json.

It's also (still) possible to import the matchers directly without
side effects:

import * as matchers from '@testing-library/jest-dom/matchers'
@@ -0,0 +1,4 @@
const globals = require('@jest/globals')
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In my testing, @jest/globals had to use CommonJS

@@ -0,0 +1,4 @@
import {expect} from 'vitest'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

And Vitest only supports ESM 🤷

Copy link
Member

Choose a reason for hiding this comment

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

In that case, should this file extension be .mjs?

Have you test this setup in a vitest project? If not, could you verify this will work as expected?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Great question. Using .mjs would be technically correct, but then all imported files would need to be ESM, which is not the case for matchers.js. Basically, vitest fails with "Error: Cannot find module".

I think we could change the extension after dual-publishing ES (#437?), but for now this works in vitest.

Have you test this setup in a vitest project?

Yeah, I've got 4 test projects that I tested this PR against: Jest+JS, Jest+TS, @jest/globals+TS, vitest+TS. I'll publish them soon.

@@ -0,0 +1 @@
/// <reference path="types/vitest.d.ts" />
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

A combination of <reference> and export {} in the referenced definitions seemed to be required to get TypeScript to augment the appropriate interfaces when this file was imported from node_modules.

@codecov
Copy link

codecov bot commented Aug 12, 2023

Codecov Report

Merging #511 (85dbd5b) into main (d717c66) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #511   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           27        27           
  Lines          664       664           
  Branches       251       251           
=========================================
  Hits           664       664           
Files Changed Coverage Δ
src/to-be-in-the-document.js 100.00% <ø> (ø)
src/to-contain-element.js 100.00% <ø> (ø)
src/to-contain-html.js 100.00% <ø> (ø)
src/utils.js 100.00% <ø> (ø)
src/index.js 100.00% <100.00%> (ø)

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

@jgoz jgoz mentioned this pull request Aug 12, 2023
4 tasks
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Thank you so much for doing this work! I think it's going to be a very positive thing. I just have a couple questions.

@@ -144,6 +180,18 @@ haven't already:
],
```

### With another Jest-compatible `expect`
Copy link
Member

Choose a reason for hiding this comment

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

Would this be the approach if you're not using vitest's globals.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not 100% sure what you mean by vitest's globals. Vitest's default mode is globals: false, which requires explicit imports of describe, expect, etc., and the new vitest entrypoint is designed to work with that. I didn't actually think to test with globals: true and importing the vitest/globals types — I'll do that tomorrow.

This section was intended as a hedge against future testing libraries that might emerge that retain some level of compatibility with Jest. But if you think it might be more confusing than helpful, I'm happy to remove it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I didn't actually think to test with globals: true and importing the vitest/globals types — I'll do that tomorrow.

Tested this and it works.

@@ -0,0 +1,4 @@
import {expect} from 'vitest'
Copy link
Member

Choose a reason for hiding this comment

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

In that case, should this file extension be .mjs?

Have you test this setup in a vitest project? If not, could you verify this will work as expected?

@kentcdodds kentcdodds merged commit 4b764b9 into testing-library:main Aug 13, 2023
7 checks passed
@kentcdodds
Copy link
Member

Thank you so much for helping with this!

@github-actions
Copy link

🎉 This PR is included in version 6.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@MattyBalaam
Copy link

Very happy to see this 😊

@TrevorBurnham
Copy link

Really excited to see this! FYI when I first plugged in this change (upgrading @testing-library/jest-dom from 5.16.5 to 6.0.0, upgrading vitest from 0.30.1 to 0.34.1 to fix a compatibility issue, then dropping @types/testing-library__jest-dom and adding import "@testing-library/jest-dom/vitest" to my test setup file), I got this error when running my tests:

Error: @vitejs/plugin-react can't detect preamble. Something is wrong. See https://github.com/vitejs/vite-plugin-react/pull/11#discussion_r430879201

No idea why that happened, but the error went away when I upgraded @vitejs/plugin-react from 3.1.0 to 4.0.4. Sharing this in hopes that it saves other devs some headaches. 🙏🏻

jtoar added a commit to redwoodjs/redwood that referenced this pull request Dec 15, 2023
This reverts #9673 and
#9711.

With v6 of `@testing-library/jest-dom`, `yarn rw type-check` fails in
Redwood projects because it can't locate the types for
`@testing-libary/jest-dom`. They absorbed the definitely types package
(`@types/testing-library__jest-dom`) in this PR:
testing-library/jest-dom#511. Here's an example
of a failing run:
https://github.com/redwoodjs/redwood/actions/runs/7213177056/job/19668365614?pr=9708.

The logical thing to do is remove `@testing-libary/jest-dom` from the
`types` array in `web/tsconfig.json`, but when we do that we get a
different failure from TS saying that it doesn't understand the extended
matchers provided by `testing-library/jest-dom` like
`toBeInTheDocument`.

@Tobbe filed an issue in their repo here
testing-library/jest-dom#559.

In the PR I linked to, the maintainer notes that...

> But in my local testing with the changes in this PR, following the
Readme was sufficient to get full auto-complete support even for JS-only
users. As long as they have a jest-setup file that imports the bare
export and add a dependency on @types/jest, recent versions of VS Code
will figure it out.

Users don't have jest setup files in their project (we just point to a
jest preset) or `@types/jest` as an explicit dev dependency.

I took a look at it but couldn't crack it today. Instead of holding up
other PRs in CI, better to revert it for now and come back to later when
we have more leads.
sungik-choi added a commit to channel-io/bezier-react that referenced this pull request Feb 27, 2024
<!--
  How to write a good PR title:
- Follow [the Conventional Commits
specification](https://www.conventionalcommits.org/en/v1.0.0/).
  - Give as much context as necessary and as little as possible
  - Prefix it with [WIP] while it’s a work in progress
-->

## Self Checklist

- [x] I wrote a PR title in **English** and added an appropriate
**label** to the PR.
- [x] I wrote the commit message in **English** and to follow [**the
Conventional Commits
specification**](https://www.conventionalcommits.org/en/v1.0.0/).
- [x] I [added the
**changeset**](https://github.com/changesets/changesets/blob/main/docs/adding-a-changeset.md)
about the changes that needed to be released. (or didn't have to)
- [x] I wrote or updated **documentation** related to the changes. (or
didn't have to)
- [x] I wrote or updated **tests** related to the changes. (or didn't
have to)
- [x] I tested the changes in various browsers. (or didn't have to)
  - Windows: Chrome, Edge, (Optional) Firefox
  - macOS: Chrome, Edge, Safari, (Optional) Firefox

## Summary
<!-- Please brief explanation of the changes made -->

Rollup(#1674), eslint, typescript(채널 lint 설정이 최신화되지 않음)을 제외한 라이브러리들을 최신
버전으로 업데이트합니다.

## Details
<!-- Please elaborate description of the changes -->

- 업데이트 과정에서 발생한 타입 충돌 등을 해결했습니다. 레퍼런스 및 커밋을 참고해주세요.
- browserlist를 업데이트했습니다. 정확한 연유는 모르겠으나, 업그레이드 이후 `postcss-preset-env` 에
의해 cascade layer, `:has` 등의 모던 CSS 기능들이 빌드 과정에서 제거되는 현상이 발생했습니다. 가장 좁은
브라우저 커버리지를 가지는 `:has` 기준으로 브라우저 리스트를 별도 파일로 분리하고, 주석을 추가했습니다.
- `yarn dedupe` 를 실행하여 중복 패키지 버전을 최소화하도록 했습니다.

### Breaking change? (Yes/No)
<!-- If Yes, please describe the impact and migration path for users -->

No

## References
<!-- Please list any other resources or points the reviewer should be
aware of -->

- ~~https://storybook.js.org/blog/optimize-storybook-7-6/ : 스토리북 빌드 시
babel대신 swc를 사용하도록 하는 설정을 추가~~
- testing-library/jest-dom#511 :
testing-library가 @types를 포함하지 않도록 변경되면서, tsconfig에 jest config 파일을 추가
- [브라우저
커버리지](https://browsersl.ist/#q=%3E%3D+1%25+in+KR%2C%0A%3E%3D+1%25+in+JP%2C%0A%3E%3D+1%25+in+US%2C%0Anot+dead%2C%0AChrome+%3E%3D+105%2C%0AEdge+%3E%3D+105%2C%0Anot+Edge+%3C+105%2C%0AFirefox+%3E%3D+121%2C%0AiOS+%3E%3D+15.4%2C%0ASafari+%3E%3D+15.4%2C%0AAndroid+%3E%3D+121%2C%0AOpera+%3E%3D+91%0ASamsung+%3E%3D+20&region=KR).
기존 브라우저 커버리지보다 커버리지가 높습니다.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants