Open magnattic opened 2 years ago
The thing is that you can avoid package duplication with npm dedupe
or yarn dedupe
. But requiring another install step when we move it to peer dependencies is a hard sell since it's another step.
Typically, if you're in a React test you always import from @testing-library/react
not /dom
. Do you have a repro where duplicated /dom
versions cause issues?
Afaik yarn dedupe
is an optimization feature to reduce the size of dependencies. It should not be necessary to run this to avoid bugs caused by conflicting deps.
I don't have a reproduction repo right now since I am working on a private company repo, but I can try to create one.
However, I am convinced that many problems seen in this issue here can be traced back to people having installed two different versions of @testing-library/dom
: https://github.com/testing-library/react-testing-library/issues/1051
See for example:
But requiring another install step when we move it to peer dependencies is a hard sell since it's another step.
Is it another step though? This is also required for @testing-library/user-event
and in the install guide there it simply says to run
yarn add --dev @testing-library/user-event @testing-library/dom
Something similar could be added for the install docs of @testing-library/react
.
Typically, if you're in a React test you always import from @testing-library/react not /dom.
From my experience, this will not be enough to solve the act()
warnings, if you use @testing-library/user-event
.
Then @react-testing-library/user-event
will use version A of the @react-testing-library/dom
and @react-testing-library/react
will use version B, leading to the problems.
I agree with @magnattic, @testing-library/dom
should be a peer dependency. Today, it is a dependency of the following packages:
@testing-library/react
@testing-library/cypress
eslint-plugin-jest-dom
If you have all these installed in the same project, you can easily run into problems like act
warnings when it should not happen.
I can confirm the issue of multiple @testing-library/dom
versions leading to unnecessary act()
warnings.
In our case, it happened when updating to React 18 in a project with @testing-library/react
and eslint-plugin-jest-dom
. These dependencies were using a different version of @testing-library/dom
each, which led to unnecessary act()
warnings that were resolved by reinstalling so that only one version of @testing-library/dom
is used.
The thing is that you can avoid package duplication with
npm dedupe
oryarn dedupe
. But requiring another install step when we move it to peer dependencies is a hard sell since it's another step.
This issue doesn't affect npm users since npm doesn't have strict peer dependencies. yarn dedupe
can only deduplicate packages with overlapping version ranges, so that doesn't solve the problem unless you specify a very broad version range (e.g., *
) for @testing-library/dom
. This isn't a very intuitive solution though since this isn't how peer dependencies are typically handled in Yarn, and it requires a relatively deep understanding of package management. I would guess that most Yarn users don't use yarn dedupe
, and to support this workaround, they will suddenly have to remember to run it every time they want to upgrade @testing-library/dom
, which is from their point of view a random transitive dependency. The out-of-box experience for most Yarn users is that @testing-library/user-event
doesn't work, and the docs specifically instruct them not to take the action that would resolve their problem:
If you use one of the framework wrappers, it is important that
@testing-library/dom
is resolved to the same installation required by the framework wrapper of your choice. Usually this means that if you use one of the framework wrappers, you should not add@testing-library/dom
to your project dependencies. ~ https://testing-library.com/docs/user-event/install/
npm packages with peer dependencies typically suggest installing the package alongside its peer dependencies in a single command. This command is often presented in a view with a button to copy it in the interest of convenience (as yours already is).
Problem description:
@testing-library/dom
in the same project leads to bugs that are hard to catch, like confusing act warnings@testing-library/dom
is installed@testing-library/dom
is a dependency to@testing-library/react
@testing-library/user-event
has a peer dependency to@testing-library/dom
So if I use both
@testing-library/react
and@testing-library/user-event
(a common use case), my dependency tree needs to look like this right now:And now it's easy for the two versions of
@testing-library/dom
to diverge, leading to bugs. I have to use solutions like yarn'sresolutions
field to keep them in sync.Suggested solution:
Make all packages use the same instance of
@testing-library/dom
by making it a peer dependency of@testing-library/react
.I understand that this has the downside that the user has to install
@testing-library/dom
alongside the react package, even if he doesn't need it. But as long as multiple testing-library packages need the same version of@testing-library/dom
, this seems to be the cleanest solution to ensure that everything works correctly.If it was a peer dependency, the above dependency tree would look like this:
Only one instance, problem solved!