-
Notifications
You must be signed in to change notification settings - Fork 470
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(ByRole): Constrain API #1211
Conversation
Only allow `string` as a `role`. Drop support for `exact`, `trim`, `collapseWhitespace`, and `normalizer` options.
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit deaeef2:
|
Codecov Report
@@ Coverage Diff @@
## alpha #1211 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 24 24
Lines 1000 992 -8
Branches 328 322 -6
=========================================
- Hits 1000 992 -8
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I like the change, I think this guards us against misusage of it and also remove some confusion
Hello! I know byRole often has perf issues when used on large dom nodes. Do you expect this PR to improve the performance? |
BREAKING CHANGE: Only allow
string
as arole
.Drop support for
exact
,trim
,collapseWhitespace
, andnormalizer
options.What:
Closes #1202
Why:
Options are not needed as far as I can tell. From the tests we have, it seems to me this encourages rather bad queries. Will ship this soon-ish (since it's just an alpha) to test.
How:
Checklist:
docs site: ByRole: Remove MatchOptions testing-library-docs#1214