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

fix(prefer-explicit-assert): handle getBy* without expect #257

Merged
merged 1 commit into from
Nov 18, 2020
Merged

fix(prefer-explicit-assert): handle getBy* without expect #257

merged 1 commit into from
Nov 18, 2020

Conversation

skovy
Copy link
Collaborator

@skovy skovy commented Nov 17, 2020

This PR fixes an issue with the prefer-explicit-assert assertion configuration option. If assertion is defined, it currently assumes all getBy* queries are wrapped in an expect. However, even with the perfer-explicit-assert there are valid uses that aren't wrapped in an expect:

fireEvent.click(getByText('bar'));
const el = getByText('foo');

Copy link
Member

@Belco90 Belco90 left a comment

Choose a reason for hiding this comment

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

Hi again! You are becoming an expert for this rule 😅

@Belco90 Belco90 merged commit b289638 into testing-library:master Nov 18, 2020
@skovy
Copy link
Collaborator Author

skovy commented Nov 18, 2020

Haha yea, working through the issues that are surfaced while running in a real codebase 😅

I appreciate your feedback and reviews, thanks!

@Belco90
Copy link
Member

Belco90 commented Nov 19, 2020

🎉 This PR is included in version 3.10.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@skovy skovy deleted the skovy/fix-assertion-for-unwrapped-getby branch November 19, 2020 01:05
@julienw
Copy link
Contributor

julienw commented Nov 30, 2020

I'm a bit surprised: we definitely had such legitimate uses in our project, but this wasn't flagged by the rule even without this patch. I'm puzzled :-) Oh now I understand this is when using the option assertion.

@Belco90
Copy link
Member

Belco90 commented Nov 30, 2020

@julienw You edited before I answered properly! Indeed, this only applies to assertion option which is not enabled by default.

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

Successfully merging this pull request may close these issues.

3 participants