testing-library / jest-dom

:owl: Custom jest matchers to test the state of the DOM
https://testing-library.com/docs/ecosystem-jest-dom
MIT License
4.44k stars 401 forks source link

Is it possible to have a version not depending on jest, @jest/globals ? #608

Closed ddolcimascolo closed 3 months ago

ddolcimascolo commented 4 months ago

Describe the feature you'd like:

A version of this package that does not have jest / @jest/globals as peer dependencies. These dependencies are installed automatically by npm (in recent versions) and if you're using Vitest and not Jest, you still end up installing tons of jest-related dependencies.

Any advice on how this could get implemented? I'm willing to help btw :)

Suggested implementation:

Describe alternatives you've considered:

Teachability, Documentation, Adoption, Migration Strategy:

gnapse commented 4 months ago

Any advice on how this could get implemented? I'm willing to help btw :)

That's what I was planning to ask you as I read the first paragraph 😅

I guess we could try to make jest only a dev dependency, and remove the automatic setup entry points. That is, the "how to use instructions" will no longer include the ability to simply import a module from this library. The instructions will always be around this way of using the library, documented in our README:

With another Jest-compatible expect

If you are using a different test runner that is compatible with Jest's expect interface, it might be possible to use it with this library:

import * as matchers from '@testing-library/jest-dom/matchers'
import {expect} from 'my-test-runner/expect' // <- change this import path to jest or vitest or whatever

expect.extend(matchers)

There's the issue of the name, but I think that's secondary. It can always be named jest-dom. Though confusing, but it can be made independent of jest.

This would have to be a breaking change, but if it works, we can do it.

ddolcimascolo commented 4 months ago

What do you think of splitting this package into N packages with

This can scale if more test runners get supported in the future

The breaking change will only be for Vitest users (going from jest-dom to vitest-dom). Upgrade for Jest users will be transparent

gnapse commented 4 months ago

Yes, that could work too. But it complicates things. It involves setting up a mono-repo. I can help with reviewing and releasing, but not with implementing it.

Also, it's a change big enough (in terms of its implications) that it warrants asking the opinion of @testing-library/core-maintainers.

kentcdodds commented 4 months ago

I've been using @jest-dom with vitest without issue. I'm not sure what the problem is...

kentcdodds commented 4 months ago

Oh, just noticed the peerDeps. Yeah, I'd say just remove that and move on 🤷‍♂️

kentcdodds commented 4 months ago

Though, they're all marked as optional so it's really not a big deal IMO.

kentcdodds commented 4 months ago

These dependencies are installed automatically by npm (in recent versions) and if you're using Vitest and not Jest, you still end up installing tons of jest-related dependencies.

In my experience this is not true. For example, in the epic stack:

npm ls @jest/globals
epic-stack-template@ /Users/kentcdodds/code/epicweb-dev/epic-stack
└── (empty)

The only package in that project from @jest is @jest/schemas and that's coming from vitest:

npm ls @jest/schemas
epic-stack-template@ /Users/kentcdodds/code/epicweb-dev/epic-stack
└─┬ vitest@1.6.0
  ├─┬ @vitest/snapshot@1.6.0
  │ └─┬ pretty-format@29.7.0
  │   └── @jest/schemas@29.6.3
  └─┬ @vitest/utils@1.6.0
    └─┬ pretty-format@29.7.0
      └── @jest/schemas@29.6.3 deduped

I'm pretty sure this is not an issue.

ddolcimascolo commented 4 months ago

Thanks for checking Kent, let me double check why I have these installed then. My npm ls clearly states that they come from jest-dom but the setup in my project being a bit complex (I'm thinking npm workspaces for instance) I might have something else along the way.

David

ddolcimascolo commented 4 months ago

Ah @kentcdodds, the epic stack project uses the legacy-peer-deps npm option :) it's in .npmrc

For the record: https://docs.npmjs.com/cli/v10/using-npm/config#legacy-peer-deps

kentcdodds commented 4 months ago

Oh yeah, this is because the new peer deps behavior is stupid. And I guess this is one more reason to disable it 🤷‍♂️

kentcdodds commented 4 months ago

Anyway, not everyone's gonna want to disable it so I guess just remove the peerDeps and move on. Much better than complicating things with a ton of packages

ddolcimascolo commented 4 months ago

Great. Do you want me to open a PR for that?

ddolcimascolo commented 4 months ago

Done https://github.com/testing-library/jest-dom/pull/610

ardeois commented 1 month ago

This PR seems to have broken jest-dom when using pnpm

PNPM by default will not hoist packages to the top level node_modules, which means if jest-dom imports vitest it will fail unless:

Having vitest declared as an optional peer dependency was I think the best option, I'm not sure to understand why this was removed?