jest-dom
jest-dom copied to clipboard
Move Types from @types/* to this repo
Describe the feature you'd like:
Following this comment and this issue, we would like to move the types to this repo also.
Suggested implementation:
Here's a PR done in dom-testing-library: https://github.com/testing-library/dom-testing-library/pull/530
Usually there have been more difficulty with our repo than with other libs in the testing-library org because unlike other libs, this one is not used via direct imports. That usually has been a blocker.
For instance, we used to have the types baked in, and we had to extract them because it did not work properly with VSCode intellisense. See #123 for instance. There was even an effort to port the actual source code to TypeScript (see #246) and it too introduced some issues that made it not suitable for a release.
That being said, I'd be glad if we can pull it off provided it is done so that all the previous issues that we've had with this would not come back.
Usually there have been more difficulty with our repo than with other libs in the testing-library org because unlike other libs, this one is not used via direct imports. That usually has been a blocker.
For instance, we used to have the types baked in, and we had to extract them because it did not work properly with VSCode intellisense. See #123 for instance. There was even an effort to port the actual source code to TypeScript (see #246) and it too introduced some issues that made it not suitable for a release.
That being said, I'd be glad if we can pull it off provided it is done so that all the previous issues that we've had with this would not come back.
Thanks @gnapse, I wasn't aware of the previous attempts that led to us moving this to Definitely-Typed. I'll give it a shot and update here :)
I gotta say, the barrier of entry to contribute types via having to do it in DefinitelyTyped/DefinitelyTyped
is huge. I love TS, and I can barely drag myself to do it (cloning that huge repo or following the instructions to clone sparsely is discouraging). I really wish we could do this somehow. I will take a look at how other jest matchers libraries do it.
cc @testing-library/all-maintainers in case someone has some ideas on how to pull this off.
If I remember correctly there wasn't much to it.
- Add types to the original repository (SemVer minor)
- Remove types from DefinitelyTyped
We may attempt it again, but the main reason we moved the type definitions from being inside the repo was #123.
There were also issues with actually writing the code in TS. See #246 (which funnily enough is 123 * 2 😄)
I think another reason in favour of this proposal would be compatibility with stricter node linkers. I'm finding that when using @testing-library/jest-dom
with pnp or pnpm node linkers, the type definitions declared in this package don't actually work because they are not a dependency of the consuming project.
For example, the node modules structure may look something like this...
my-project
node_modules
@testing-library
jest-dom
node_modules
@types
testing-library__jest-dom
So my-project
can't actually resolve @types/testing-library__jest-dom
as it would just look at its node_modules
directory and its parent node_modules
directories, not the nested directories. The current behaviour is relying on package managers to flatten dependencies so that they can be accessed implicitly, but that can lead to unsafe access of undeclared dependencies.
In these cases I am having to add @types/testing-library__jest-dom
to my project anyway.
In contrast, compared to other extensions to jest matchers like jest-extended
, because the types are exported by the same package, they are correctly resolved by the project.
I'd be happy to submit a PR to implement @eps1lon's suggestion if everyone is happy with that approach?
It'd be awesome if you do that @jdanil.
Can you please also keep in mind the difficulties we found when previously attempting this in #246? Although in that case it was different: the actual code was migrated to TS. Maybe having just type definitions and keeping the code in JS is sufficiently different to not cause any troubles.
Ok I had a bit of a play around today. I think if the types are moved to this package, then the issue in #246 is unavoidable (that you need to explicitly import @testing-library/jest-dom
for the types to be available). That means you can't simply add "setupFilesAfterEnv": ["@testing-library/jest-dom"]
. You need to either create some setup.ts
file and import @testing-library/jest-dom
there, or you can still include it in setupFilesAfterEnv
but you will also need to add a custom.d.ts
file that imports @testing-library/jest-dom
to add the types. jest-extended
also instructs typescript users to setup their configuration this way.
An alternative I was playing around with that doesn't necessarily resolve this issue of moving the type definitions, but avoids the need to add @types/testing-library__jest-dom
as a dependency for users using stricter node linkers was to add a "types": "@types/testing-library__jest-dom"
entry to the package.json
. So @testing-library/jest-dom
exposes the types from DefinitelyTyped. I haven't seen this approach in the wild before, but it worked in my local testing.
I think the way this should be solved is a declaration file in this repo that imports the matcher types from the modules (might use declaration files as a stepping stone while converting modules to TS) and augments the Matchers
in jest namespace.
From there it should be a dual approach:
- For those without the need of extra configuration there should be a
@types/testing-library__jest-dom
package. a) I think we could feed the DefinitelyTyped per GitHub Action. b) The declaration could import types from@testing-library/jest-dom
so that it automatically reflects the installed version. - Everybody else could either add
@types/testing-library__jest-dom
as dependency or add the declaration folder from@testing-library/jest-dom
to thetypeRoots
.
Can you go ahead with a PR? We could do the same we did with #246, publishing a beta or alpha release to see if it all works as expected when this gets published.
Couldn't we solve this problem by recommending to use /// <reference ... />
comments in test setup files? create-react-app, Vite, and Vitest do something similar.
Or adding to the tsconfig.json
"types"
field is another approach that is often used as well. I'd really love to get the type story straightened out here. Storybook is trying to update our version of expect to 28, and the module augmentation in jest-dom is breaking. It all seems very fragile...
:tada: This issue has been resolved in version 6.0.0 :tada:
The release is available on:
Your semantic-release bot :package::rocket: