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

Use recommended pattern in testing example #28404

Merged
merged 11 commits into from Aug 25, 2021
Merged

Use recommended pattern in testing example #28404

merged 11 commits into from Aug 25, 2021

Conversation

htunnicliff
Copy link
Contributor

@htunnicliff htunnicliff commented Aug 23, 2021

Since the official linter for React Testing Library (eslint-plugin-testing-library) recommends using screen to write queries, this MR updates the Testing Library example to follow the pattern recommended by the linter.

DOM Testing Library (and other Testing Library frameworks built on top of it) exports a screen object which has every query (and a debug method). This works better with autocomplete and makes each test a little simpler to write and maintain.

This rule aims to force writing tests using built-in queries directly from screen object rather than destructuring them from render result. Given the screen component does not expose utility methods such as rerender() or the container property, it is correct to use the render returned value in those scenarios.

See the prefer-screen-queries rules docs for more info: https://github.com/testing-library/eslint-plugin-testing-library/blob/main/docs/rules/prefer-screen-queries.md

Documentation / Examples

  • Make sure the linting passes

Copy link
Member

@leerob leerob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you update the corresponding example as well? Thank you!

@htunnicliff
Copy link
Contributor Author

htunnicliff commented Aug 23, 2021

@leerob will do!

Are there any plans to incorporate the recommended linter(s) for Testing Library and Cypress, either as standalone dependencies in examples or within next lint itself?

@leerob
Copy link
Member

leerob commented Aug 23, 2021

I like the idea of extending the default ESLint setup on the example to include this package! 👍

@ijjk ijjk added the examples Issue/PR related to examples label Aug 23, 2021
@htunnicliff
Copy link
Contributor Author

@leerob I added linting configuration and updated the example test. For clarity, I also swapped in the name "Home" for the tests, since the example is not testing the App but rather the Home component. I also updated the name of testing-library.js to index.test.jsx, which more closely matches real-world test file names.

@delbaoliveira
Copy link
Contributor

Hey @htunnicliff, thank you for submitting this PR and updating the example! 💪🏼

delbaoliveira
delbaoliveira previously approved these changes Aug 23, 2021
@htunnicliff
Copy link
Contributor Author

@delbaoliveira from my end it looks like this PR is still marked as having "requested changes." Is there anything else I should do to help get this merged?

leerob
leerob previously approved these changes Aug 24, 2021
Copy link
Member

@leerob leerob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much!

@leerob
Copy link
Member

leerob commented Aug 25, 2021

Looking like linting is failing here, which is why it didn't auto merge.

htunnicliff and others added 7 commits August 24, 2021 21:52
Since the official linter for testing library, `eslint-plugin-testing-library` recommends using `screen` to write queries, this MR updates the testing library example to follow the pattern recommended by the linter.

> DOM Testing Library (and other Testing Library frameworks built on top of it) exports a screen object which has every query (and a debug method). This works better with autocomplete and makes each test a little simpler to write and maintain.

> This rule aims to force writing tests using built-in queries directly from screen object rather than destructuring them from render result. Given the screen component does not expose utility methods such as rerender() or the container property, it is correct to use the render returned value in those scenarios.

See the `prefer-screen-queries` rules docs for more info: https://github.com/testing-library/eslint-plugin-testing-library/blob/main/docs/rules/prefer-screen-queries.md
This was referenced Sep 3, 2021
@vercel vercel locked as resolved and limited conversation to collaborators Jan 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
examples Issue/PR related to examples
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants