[@storybook/test] Move @testing-library/dom to a peerDependency

[shilman]

Discussed in #27501

^{Originally posted by MatanBobi June 1, 2024}

Is your feature request related to a problem? Please describe.

In React Testing Library, we've experienced multiple issues that occur once people install dom-testing-library on their own (as part of user-event for example) and also install a library that bundles DTL (like @storybook/test and also @testing-library/react). These issues occur once there's a version mismatch and multiple version of DTL get installed.

Describe the solution you'd like

Moving DTL to a peerDependency will help with solving this issue.

Describe alternatives you've considered

No response

Are you able to assist to bring the feature to reality?

yes, I can

Additional context

I'm a member of the testing-library organization and due to multiple issues we had with multiple version of DTL installed, we've decided to create a PR to move DTL to a peerDependency in React-Testing-Library:
testing-library/react-testing-library#1305

This is something we've thought about for quite a long time and we highly recommend other libraries in the testing-library ecosystem to also do.
I'm here and available to assist in any issues that occur or even contribute this change on my own.
Feel free to reach out to me or any of us from the testing-library organization.

[kasperpeulen]

@MatanBobi Our setup is a bit complicated.

We prebundle testing-library with tsup:

But because tsup somehow messes the types up during prebundling, we have @testing-library/dom, @testing-library/jest-dom and @testing-library/user-event as a direct dependency, just for the types. But for the runtime of those packages we prebundle them.

We are considering 2 different scenario's.

1. We stop prebundling @testing-library/dom, @testing-library/jest-dom and @testing-library/user-event, and make it a peer dependency as you suggested.

In this case, should all of them be a peer dep?

2. We continue prebundling and try to find a workaround for the tsup issues, so that it will be an actual devDependency instead.

Will this also solve your issue?

[MatanBobi]

AFAIR, @testing-library/jest-dom doesn't rely on @testing-library/dom at all so you can keep that there.
Regarding @testing-library/user-event, I think it can also remain there as it already has a peer dependency on @testing-library/dom and AFAIK it doesn't have any specific state that might conflict. So we can ask users to only install @testing-library/dom as a peer dependency :)

[kasperpeulen]

Thanks for the reply @MatanBobi

And if we completely pre-bundle all of them (so make them devDependencies instead), would that also solve your issue?

[MatanBobi]

Hmmm, it's a good question. I'm not that much into the details of how the module resolution will work in that case. The problem we had was that inside RTL people who were using user-event sometimes received the DTL bundled with RTL and sometimes the one they've installed for user-event. Since I didn't experience this issue on my own but opened it due to a user commenting they're experiencing this issue I might not be the right person to say if it will work or not.
Here's the comment that raised the issue:
testing-library/react-testing-library#1216 (comment)

[kasperpeulen]

Okay, thanks for the context!

I think that issue is similar as this issue:
#27173

I just merged a PR, that at least uses the latest version of DTL, which I think will mitigate this issue a bit.

I do agree that we either should go for peerDeps or devDeps to fully resolve the issue.
I think that devDeps (prebundling) probably would be even more ideal, as it comes to this issue specifically.
Will keep you updated.

[MatanBobi]

I think that issue is similar as this issue: #27173

Yes that definitely looks similar.
Thanks @kasperpeulen for the quick replies and attention :)

[yannbf]

Writing down some thoughts while talking to @kasperpeulen:

• We will very likely make @testing-library/dom a peer dependency in Storybook 9.0, but we can not do it earlier because it would be a breaking change.
• In the meantime, we are investigating if we can find a workaround for the tsup issue (mentioned earlier in the thread), and prebundle the lib, so that it won't give conflicts anymore (which caused the act issues when installing Storybook)

[MatanBobi]

Thanks @yannbf, I totally understand regarding releasing it as part of a new major as it's indeed a breaking change (that's what we did also).
Thanks again!