BitBoxSwiss / bitbox-wallet-app

The BitBoxApp for desktop and mobile.
https://bitbox.swiss/app
Apache License 2.0
245 stars 82 forks source link

frontend: bump @testing-library/react version #2771

Closed NicolaLS closed 1 week ago

NicolaLS commented 4 weeks ago

Bump @testing-library/react to latest version 16.0.0. Also installs @testing-library/dom as a peer dependency, see: https://github.com/testing-library/react-testing-library/releases/tag/v16.0.0

There are breaking changes from 15.0.0 to 16.0.0 (none from 14 to 15) but they do not cause any problems, see link above. Tested with CI and webdev (just opened a software keystore and clicked on some stuff / webdev not really relevant to test this)

NicolaLS commented 4 weeks ago

@testing-library/dom was moved to a peer dependency and needs to be explicitly installed.

I'm a bit confused what this means, isn't it pulled in automatically when it is declared as a peer dependency? npm ls @testing-library/dom shows it is installed now but from that sentence it sounds like we are supposed to explicitly put it in the package.json? @thisconnect

thisconnect commented 4 weeks ago

@types/react-dom needs to be installed if you're typechecking files using @testing-library/react

yeah I think so. deferring to @shonsirsha

shonsirsha commented 4 weeks ago

About peer dep, AFAIK, yes we'd need to install it explicitly. This is a good short article that helped me understand it: https://flaviocopes.com/npm-peer-dependencies/.

npm ls @testing-library/dom shows it is installed now..

I tried going to this branch, make buildweb, and it says empty on mine?

empty
shonsirsha commented 4 weeks ago

Sorry, @NicolaLS - I was in the wrong dir. I tried npm ls @testing-library/dom and indeed it shows that it's installed. Even after removing both node_modules and packge-lock.

npm ls @testing-library/dom shows it is installed now but from that sentence ... ?

The answer to your question is most likely to be this. My npm version is 9.5.0 so I expected this behaviour. I'm assuming your npm ver is >= 7?

So yeah I guess it's fine to leave it without adding that pkg explicitly in the package.json - unless we want to control the version of the @testing-library/dom. However, it seems that all tests are still running fine so no need for now I'd say.

NicolaLS commented 3 weeks ago

Sorry, @NicolaLS - I was in the wrong dir. I tried npm ls @testing-library/dom and indeed it shows that it's installed. Even after removing both node_modules and packge-lock.

npm ls @testing-library/dom shows it is installed now but from that sentence ... ?

The answer to your question is most likely to be this. My npm version is 9.5.0 so I expected this behaviour. I'm assuming your npm ver is >= 7?

So yeah I guess it's fine to leave it without adding that pkg explicitly in the package.json - unless we want to control the version of the @testing-library/dom. However, it seems that all tests are still running fine so no need for now I'd say.

Yes my npm is 10.8.1. Thanks for the article!!