Skip to content
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

(feat): Add react-testing-library to react templates #927

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

lukasbals
Copy link

@lukasbals lukasbals commented Nov 7, 2020

Added react-testing-library to the react and the react-with-storybook template. Additionally, updated the readme-files to link to the react-testing-library docs and implemented sample tests.

@vercel

This comment has been minimized.

Copy link
Collaborator

@agilgur5 agilgur5 left a 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.

package.json Outdated Show resolved Hide resolved
templates/react/README.md Outdated Show resolved Hide resolved
'react',
'react-dom',
'@testing-library/react',
'@testing-library/jest-dom',
Copy link
Collaborator

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

templates/react-with-storybook/test/index.test.tsx Outdated Show resolved Hide resolved
@@ -8,4 +10,13 @@ describe('Thing', () => {
ReactDOM.render(<Thing />, div);
ReactDOM.unmountComponentAtNode(div);
});
Copy link
Collaborator

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).

src/templates/react.ts Outdated Show resolved Hide resolved
src/templates/react.ts Outdated Show resolved Hide resolved
@lukasbals
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants