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

feat: Use ARIA 1.2 for role queries #1058

Merged
merged 3 commits into from
Oct 18, 2021

Conversation

IanVS
Copy link
Contributor

@IanVS IanVS commented Oct 15, 2021

What:

Updates aria-query to version 5.0.0.

Closes #703
Closes #997

Why:

I hit an issue with vite/rollup stumbling on babel/corejs which was being brought in via aria-query. The latest version of aria-query removes these dependencies, opting to use objects and arrays instead of Maps and Sets. But they did so in a mostly backwards-compatible way, and did not break the usage of the package here in dom-testing-library.

Ref: #995 (but I don't think this PR quite closes it)

In my own app, this change resolves the problems I was having.

How:

Updated and ran tests.

Checklist:

  • Documentation added to the
    docs site N/A
  • Tests
  • TypeScript definitions updated Not possible
  • Ready to be merged

Corresponding jest-dom PR: testing-library/jest-dom#414

I couldn't update the type definitions because there is not a new definitely typed version. I opened a discussion for it. DefinitelyTyped/DefinitelyTyped#56549 (comment)

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 15, 2021

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 894b685:

Sandbox Source
react-testing-library-examples Configuration
react-testing-library demo Issue #703
react-testing-library demo (forked) Issue #997

@codecov
Copy link

codecov bot commented Oct 15, 2021

Codecov Report

Merging #1058 (894b685) into main (b569a1b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##              main     #1058   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           25        25           
  Lines          928       952   +24     
  Branches       287       311   +24     
=========================================
+ Hits           928       952   +24     
Flag Coverage Δ
node-12 100.00% <ø> (ø)
node-14 100.00% <ø> (ø)
node-16.9.1 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
src/events.js 100.00% <0.00%> (ø)
src/helpers.js 100.00% <0.00%> (ø)
src/matches.ts 100.00% <0.00%> (ø)
src/wait-for.js 100.00% <0.00%> (ø)
src/pretty-dom.js 100.00% <0.00%> (ø)
src/queries/role.js 100.00% <0.00%> (ø)
src/role-helpers.js 100.00% <0.00%> (ø)
src/label-helpers.ts 100.00% <0.00%> (ø)
src/queries/label-text.ts 100.00% <0.00%> (ø)
src/queries/display-value.ts 100.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b569a1b...894b685. Read the comment docs.

@eps1lon eps1lon changed the title Update aria-query to 5.0.0 feat: Use ARIA 1.2 for role queries Oct 18, 2021
@eps1lon eps1lon added the enhancement New feature or request label Oct 18, 2021
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Thanks!

Added tests to verify that this fixes #703 and #997

@eps1lon
Copy link
Member

eps1lon commented Oct 18, 2021

Going to release as a major for safe meassures since we now query according to ARIA 1.2 not 1.1.

Actually I don't think that's viable. We want to follow the stable release more closely anyway so querying behavior may change more frequently. Let's see if people prefer stable behavior in their tests over stable spec behavior.

@eps1lon eps1lon added BREAKING CHANGE This change will require a major version bump and removed BREAKING CHANGE This change will require a major version bump labels Oct 18, 2021
@eps1lon eps1lon merged commit 1239182 into testing-library:main Oct 18, 2021
@eps1lon
Copy link
Member

eps1lon commented Oct 18, 2021

@all-contributors add @IanVS for code

@allcontributors
Copy link
Contributor

@eps1lon

I've put up a pull request to add @IanVS! 🎉

@github-actions
Copy link

🎉 This PR is included in version 8.10.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@IanVS
Copy link
Contributor Author

IanVS commented Oct 29, 2021

Thanks for merging this @eps1lon. Note that there's also a PR open for jest-dom as well: testing-library/jest-dom#414.

@IanVS IanVS deleted the update-aria-query branch October 29, 2021 14:44
MatthiasKainer pushed a commit to MatthiasKainer/dom-testing-library that referenced this pull request Nov 16, 2021
Co-authored-by: eps1lon <silbermann.sebastian@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

*ByRole queries don't properly support the banner role getByRole does not work with "term"
2 participants