-
Notifications
You must be signed in to change notification settings - Fork 152
Closed
Labels
new ruleNew rule to be included in the pluginNew rule to be included in the pluginpinnedPinned for different reasons. Issues with this label won't be flagged as stale by stalebotPinned for different reasons. Issues with this label won't be flagged as stale by stalebotreleased
Description
Hi! I've been wondering if there was ever a discussion about implementing a rule to warn about accessible queries use. Since I don't think it's really necessary to throw an error or do an autofix, a warning should be just fine. So we could get a warning to possibly refactor this
const loginButton = screen.getByTestId('login-button');
into this
const loginButton = screen.getByRole('button', { name: /login/i });
What do you think? :)
sergiohampel, alexgwolff, tkrotoff, siggy-h, willthebrit and 1 more
Metadata
Metadata
Assignees
Labels
new ruleNew rule to be included in the pluginNew rule to be included in the pluginpinnedPinned for different reasons. Issues with this label won't be flagged as stale by stalebotPinned for different reasons. Issues with this label won't be flagged as stale by stalebotreleased
Type
Projects
Milestone
Relationships
Development
Select code repository
Activity
Belco90 commentedon Feb 10, 2021
Hi @maryfsc, thanks for opening this issue!
This could be an interesting idea to report usages of
ByTestId
queries. However, I'm not really sure about how to suggest fromByTestId
argument which other query to use instead + what pass to it.The example you shared is really direct, but what would you suggest if you have something like this?
I'd go just for a
no-test-id-queries
rule which complains if you use aByTestId
query, without suggesting how to fix it and just saying something like"Using data-testid attributes do not resemble how your software is used and should be avoided if possible"
.maryfsc commentedon Feb 11, 2021
Hey @Belco90 I think that's a pretty good idea 😄 👍
Belco90 commentedon Feb 12, 2021
Nice. Would you like to give this a try? I recommend to do this on v4 since new rules implementation is much easier there (and it should be released soon).
maryfsc commentedon Feb 15, 2021
Yes, I'd like to try! So I'll wait for v4 to release 😉 Thank you!
Belco90 commentedon Feb 15, 2021
That's awesome! We had other people implementing new rules directly on
v4
branch, so you could do that if you want to. However, I have to tell you that the doc around new rules creator is not added yet, guidelines weren't updated properly, and new internal mechanisms introduced in v4 could still change (although it's pretty stable already). So up to you![-]New rule suggestion: prefer-accessible-query[/-][+]New rule suggestion: no-test-id-queries[/+]Belco90 commentedon Apr 20, 2021
Hi again! Since v4 is finished, I hope I can spend time on this soon. It's a really interesting rule for encouraging people to use semantic and accessible queries rather than querying by id.
stale commentedon Jun 19, 2021
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
Belco90 commentedon Jun 20, 2021
I still want to implement this one!
stale commentedon Aug 19, 2021
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
2 remaining items
zaicevas commentedon Nov 24, 2021
Note that, as of now,
role
queries are incredibly slow for larger DOM trees (see testing-library/dom-testing-library#552). Test ids is one of the ways to mitigate this. In my experience, certain tests are almost 1 second (!) faster with test ids, as opposed to with role queries.Belco90 commentedon Nov 24, 2021
@zaicevas that's a good point, definitely something to mention in this rule's doc when implemented. I think the plugin should at least provide some rule to prevent or mitigate the usage of
ByTestId
queries, then it's up to each dev/team to enable it or not.arinthros commentedon Apr 7, 2022
Upvote on the simple rule that either warns or errors if a test id query is used. I'm happy to collab on this, and maybe open a PR if it's simple enough.
Belco90 commentedon Apr 11, 2022
It would be great if you could open a PR! It should be fairly easy by using some of the internal helpers, you can find more indo in the contributing doc.
tkrotoff commentedon Aug 14, 2023
Meanwhile this can be achieved thx to
no-restricted-properties
andreact/forbid-dom-props
:Always test your code:
I suggest to enable
reportUnusedDisableDirectives
: https://eslint.org/docs/latest/use/configure/rules#report-unused-eslint-disable-comments (should be ESLint default)feat: add `no-test-id-queries` rule
no-test-id-queries
rule #1006github-actions commentedon May 13, 2025
🎉 This issue has been resolved in version 7.2.0 🎉
The release is available on:
Your semantic-release bot 📦🚀
feat: add `no-test-id-queries` rule (testing-library#1006)