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.

Sign up for GitHub

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

Jump to bottom

fix(no-dom-imports): false negatives with several testing library imports #657

Merged
merged 2 commits into from Oct 2, 2022
Merged

fix(no-dom-imports): false negatives with several testing library imports #657

merged 2 commits into from Oct 2, 2022

Conversation

sjarva
Copy link
Collaborator

@sjarva sjarva commented Oct 1, 2022 •
edited

Checks

  • I have read the contributing guidelines.
  • If some rule is added/updated/removed, I've regenerated the rules list (npm run generate:rules-list)
  • If some rule meta info is changed, I've regenerated the plugin shared configs (npm run generate:configs)

Changes

  • adds a new util function getAllTestingLibraryImportNodes to get all testing-library related imports
  • in no-dom-import loop through all the testing-library imports, and report an error on that import(s) that imports from testing-library/dom

Context

Closes #586

P.S. If you feel that this PR is following the rules of Hactoberfest and this gets merged, it would be wonderful if this PR would be tagged with a hacktoberfest-accepted tag/label. Thanks!

@sjarva
fix: refactor helper functions to keep a list testing-library imports
78a3d4e 
Previously only first testing-library import was saved, now all of them are saved.
@sjarva
fix: false negatives for no-dom-import with several imports
6e2df31 
Use the new refactored utils function to get a list of all testing-library imports
and make sure that none of them are a dom import.

Ref: testing-library#586
@sjarva sjarva changed the title fix: refactor helper functions to keep a list testing-library imports fix(no-dom-imports): false negatives with several testing library imports Oct 1, 2022
@sjarva sjarva marked this pull request as ready for review October 1, 2022 15:26
@sjarva sjarva mentioned this pull request Oct 1, 2022
Closed
Belco90
Belco90 approved these changes Oct 2, 2022
View reviewed changes
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.

Great approach for fixing this! Label for hacktoberfest added.

lib/create-testing-library-rule/detect-testing-library-utils.ts Show resolved Hide resolved
@Belco90 Belco90 merged commit 0ae1f25 into testing-library:main Oct 2, 2022
@github-actions
Copy link

github-actions bot commented Oct 2, 2022

🎉 This PR is included in version 5.7.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sjarva sjarva deleted the pr/false-negatives-no-dom-import branch October 2, 2022 15:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Belco90 Belco90 Belco90 approved these changes

Assignees
No one assigned
Labels
hacktoberfest-accepted released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bug: no-dom-import false negatives with several Testing Library imports
2 participants
@sjarva @Belco90