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: avoid reporting react-test-renderer utils as queries #709

Closed
wants to merge 2 commits into from

Conversation

Belco90
Copy link
Member

@Belco90 Belco90 commented Dec 24, 2022

Checks

Changes

  • Avoid reporting react-test-renderer utils as find* queries
  • Avoid reporting react-test-renderer utils as custom queries
  • Add tests

Context

Fixes #671
Fixes #673

@Belco90 Belco90 requested a review from a team December 24, 2022 18:16
@Belco90 Belco90 self-assigned this Dec 24, 2022
@Belco90 Belco90 added the bug Something isn't working label Dec 24, 2022
return (
isQuery(node) &&
node.name.startsWith('find') &&
!EXCLUDED_FIND_BY_QUERIES.includes(node.name)
Copy link
Member Author

Choose a reason for hiding this comment

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

Actually... I could add this to isQuery instead.

Comment on lines +138 to +142
'findByType',
'findByProps',
'findAll',
'findAllByType',
'findAllByProps',

Choose a reason for hiding this comment

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

And what if someone uses a custom query with one of these names? We still want them to be reported. no? How can we make sure that we are dealing with a react-test-renderer query?
This relates also to this comment.

Copy link
Member Author

@Belco90 Belco90 Dec 25, 2022

Choose a reason for hiding this comment

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

That would be the ideal fix indeed. However, since the utils names seem quite specific and can be chained from built-in queries in React Native, this PR could be a quick workaround.

I personally don't have the time to implement the ideal fix to check if queries are coming from react-test-renderer, so it's a matter of deciding if we are happy with this workaround (and its trade-offs) or we are only interested in the ideal fix.

Copy link
Member Author

@Belco90 Belco90 Dec 25, 2022

Choose a reason for hiding this comment

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

Another idea could be adding a config for React Native to the plugin, which adds automatically the corresponding query names to the settings to be ignored as custom queries.

This would be a workaround too, but perhaps makes more sense since it would affect just users using the RN preset.

@Belco90
Copy link
Member Author

Belco90 commented Dec 26, 2022

Thinking about this again... this doesn't make sense. I'll close this PR and update the issue to mention how the ideal fix should look like and a temporary workaround for those React Native users running into false positives.

@Belco90 Belco90 closed this Dec 26, 2022
@Belco90 Belco90 deleted the 673_ignore_react-test-renderer_utils branch December 26, 2022 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
2 participants