Revert "Add peer dependency to User Event docs" #1329
Conversation
✅ Deploy Preview for testing-library ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
|
AFAIU from the code, |
|
Not likely since npm 7 installs peer dependencies automatically (again). Context: I just checked blame and this was apparently my fault. 😅 At the time npm 7 was new so I couldn't depend on peer dependencies being installed automatically. Now npm 7 has been included with Node for 3 years and support for older releases ended 6 months ago, so I'm not as concerned about compatibility. If a user has a package manager that doesn't install peer dependencies (including npm 3-6 and Yarn), it will still warn by default. |
nickmccurdy
changed the title
@testing-library/dom
docs/user-event/install.mdx
Outdated
| If you use one of the | ||
| [framework wrappers](../dom-testing-library/install.mdx#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. |
There was a problem hiding this comment.
I think we should keep this note.
When people upgrade e.g. their version of @testing-library/react the @testing-library/dom package isn't always deduplicated. I'd rather add examples how to diagnose and fix this to further reduce retyped text when I suspect this to be the root of someeone's problem (e.g. "user-event only types the first N characters").
There was a problem hiding this comment.
Are you suggesting we revert this deletion and add examples of specific use cases? Could you please give some examples? Peer dependency management is in a much better state than it used to be, and I'm not sure it's worth the maintenance burden of us documenting anymore. I want to either document more use cases which I believe isn't worth the effort, or remove this as no documentation is usually better than outdated or inaccurate documentation.
There was a problem hiding this comment.
Yes, I would keep this paragraph on duplicate versions of /dom.
I'm under the impression this ranks as the second most common reason for act warnings and wrong input with UserEvent after missing await. Both isn't our fault and should not happen, but it does and we still have to deal with people running into these problems.
Therefore I'd like to have a section I can point to if I suspect this problem. The exact combinations of packages and package managers change over time and maintaining a list of them is out of scope of our docs. But if we feel that this paragraph isn't perspicuous, maybe we should add some explanation/example.
|
Since people don't really care about the warnings from the |
|
A note about what? Can you give an example? |
That explicitly installing |
|
Our I'd really prefer to keep the original revert of my change (the first commit in this pull ignoring formatting changes) as it's now outdated advice and it's not even consistent with User Event's readme. If you have any strong opinions against this please say so, preferably with explicit examples of potentially problematic use cases. |
We'll probably update those once the major version for DTL will merged. |
Reverts #888
user-event's recommendation for installingdomappears to be outdated, as package managers (except npm 3-6 and Yarn) automatically install dependencies anyway.