-
Notifications
You must be signed in to change notification settings - Fork 505
(feat): Add react-testing-library to react templates #927
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
This comment has been minimized.
This comment has been minimized.
4da3770 to
0292873
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution @lukasbals !
This covers most of the necessary changes, but there's some unnecessary changes here that will need to be removed, and some best practices that will require some changes. There's also some smaller portions that I need to double-check don't accidentally break things as well and I'll need to do some manual testing of this since we don't yet have E2E tests for tsdx create. Please see the comments in-line.
src/templates/react.ts
Outdated
| 'react', | ||
| 'react-dom', | ||
| '@testing-library/react', | ||
| '@testing-library/jest-dom', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alphabetically, this should actually be at the top; @testing comes before @types.
Also, we might need to pin an earlier version of @testing-library/jest-dom as this might be a Jest 26/TS 3.8+ version which is slated for TSDX v0.16.0... need to look into that
| const div = document.createElement('div'); | ||
| ReactDOM.render(<Thing />, div); | ||
| ReactDOM.unmountComponentAtNode(div); | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So we probably don't need the ReactDOM test anymore with this, but I need to do some more reading on @testing-library/react internals to see if it covers this usage test (I believe it does, but not 100% sure).
3e7dab7 to
f2b39d1
Compare
|
Hi @agilgur5. I pushed a fixup PR quite some time ago. Is there any further action required from my side? You mentioned you want to double-check some stuff, what's the state there? No rush just wanted to know how things are. |
Added
react-testing-libraryto thereactand thereact-with-storybooktemplate. Additionally, updated the readme-files to link to thereact-testing-librarydocs and implemented sample tests.