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

Add @testing-library to the default templates #7881

Merged
merged 9 commits into from
Oct 29, 2019

Conversation

kentcdodds
Copy link
Contributor

Closes #6358

passing test

@kentcdodds
Copy link
Contributor Author

I think I've done something wrong, but I'm not sure what 😅

@mrmckeb
Copy link
Contributor

mrmckeb commented Oct 25, 2019

Hi @kentcdodds, I see you broke everything! 😆

This is probably because we've merged in our new templates... #7716. If you want, I can show you how they work - let me know.

If you want to take a look yourself - they're yet to be fully documented - there's a template.json file in each template, and you can add dependencies to that file.
https://github.com/facebook/create-react-app/tree/master/packages/cra-template
https://github.com/facebook/create-react-app/tree/master/packages/cra-template-typescript

Those dependencies will be installed as the template is copied over.

This should be a very quick change to the the PR. Can you also this to the TypeScript template for consistency?


Templates will be released very soon, you can preview them now in our beta channel:

npx create-react-app@next --scripts-version=react-scripts@next --template=cra-template[-typescript]@next

All of that will become the below, once released.

npx create-react-app # No TS
npx create-react-app --template typescript

@ianschmitz
Copy link
Contributor

Hey @kentcdodds. @iansu and I have reserved a template package for RTL that we'd be happy to give ownership to you. Come find us at the pool at react conf! 😛

@kentcdodds
Copy link
Contributor Author

I talked with @iansu at the conference and he told me about the templates thing which is cool. We discussed the value of including RTL in the default template which is what inspired this PR.

So I take it that instead of adding things to the args array like I'm doing, I just need to add them to dependencies in the template.json files? I'll do that. Thanks!

@kentcdodds
Copy link
Contributor Author

lol, I have no idea what I'm doing here. Sorry. Help is appreciated.

@iansu
Copy link
Contributor

iansu commented Oct 28, 2019

It looks like you're on the right track. I just tried creating a new app locally, installing the @testing-library dependencies, updating setupTests.js and updating the test to match yours from this PR. After doing that yarn test worked. The test failed but I think that's unrelated.

So for some reason on CI it either hasn't installed the @testing-library dependencies or the resolver can't find them. I can't think of a good way to debug that at the moment though. @mrmckeb can you think of any reason the template dependencies might not be present?

@ianschmitz ianschmitz added this to the 3.3 milestone Oct 29, 2019
@ianschmitz
Copy link
Contributor

I'm on it.

@ianschmitz
Copy link
Contributor

@kentcdodds i think i got things sorted for you. Note that the starter test in the templates wasn't quite right as it was trying to find a heading element, but our templates don't have any 😛. I refactored it to something similar to get it passing, but feel free to tweak it to something that you think is more useful.

@mrmckeb i ran into an issue here in our e2e-simple suite. It was running the test suite from react-scripts, which uses a relative path to run tests within the template. However because the template didn't have the @testing-library/* dependencies declared in the template installed during the test run, it failed during the module import. I added it to the root package.json to fix this temporarily, but i think we could probably solve this in a cleaner way so we don't need to hoist the dependencies up.

@kentcdodds
Copy link
Contributor Author

Thanks @ianschmitz! Actually the test was good as it was. h1/h2/etc. all have the implicit role of "heading." The screenshot in my original post was showing that test working in a CRA-bootstrapped app. So I switched it back to that because I think it's probably a better initial recommendation.

Thanks a ton for the help here. I think this will be really good for everyone.

@kentcdodds
Copy link
Contributor Author

lol, just realized that you were correct in that you don't render a heading anymore (I need to update my local template 😅). Let me fix that back. Thanks!

@kentcdodds
Copy link
Contributor Author

Alright! As far as I'm concerned, this is ready to go!

@ianschmitz ianschmitz changed the title add @testing-library support to the default template Add @testing-library to the default templates Oct 29, 2019
@ianschmitz ianschmitz merged commit 2de57fe into facebook:master Oct 29, 2019
@ianschmitz
Copy link
Contributor

Woooo! Thanks @kentcdodds 😄

@kentcdodds kentcdodds deleted the pr/rtl branch October 29, 2019 22:18
@kentcdodds
Copy link
Contributor Author

Awesome! I can't wait for the release! Please do let me know so I can try it out as a last check :)

@mrmckeb
Copy link
Contributor

mrmckeb commented Oct 30, 2019

Sorry all, I've been in London - but thanks for picking this up.

We have a bad test that needs to be fixed.

@lock lock bot locked and limited conversation to collaborators Nov 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Thoughts on including react-testing-library in the default template?
5 participants