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: add isInstanceOfElement helper #885

Conversation

ph-fritsche
Copy link
Member

Add a helper to verify the element type across libraries using testing-library/dom under the hood.

What:

Export a helper in @testing-library/dist/helpers to determine if an element is an instance of a specific HTMLElement type.

Why:

@testing-library/user-event requires a better way to determine the element type.
As the requirement is not specific to user-event, I think it would be the cleaner solution to add it here.

How:

The helper tries to find the element constructor on the window.

As the window is neither required to be associated with the observed document nor is it required to provide any element constructors, the helper provides a fallback for other environments than Jest+JSDOM.

Checklist:

  • N/A Documentation added to the
    docs site
  • Tests
  • N/A? Typescript definitions updated
    There are no typescript definitions for internal methods at the moment.
  • Ready to be merged

@codesandbox-ci
Copy link

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 37907da:

Sandbox Source
react-testing-library-examples Configuration

@codecov
Copy link

codecov bot commented Jan 31, 2021

Codecov Report

Merging #885 (79f8284) into master (5bc9364) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #885   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           26        26           
  Lines          951       959    +8     
  Branches       288       290    +2     
=========================================
+ Hits           951       959    +8     
Impacted Files Coverage Δ
src/helpers.js 100.00% <100.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 5bc9364...37907da. Read the comment docs.

@eps1lon
Copy link
Member

eps1lon commented Feb 1, 2021

As the requirement is not specific to user-event, I think it would be the cleaner solution to add it here.

But only user-event needs this? Could you only implement this in user-event and then we'll see if we need it in other places as well and whether @testing-library/dom is the right place for this abstraction.

@ph-fritsche
Copy link
Member Author

See testing-library/user-event#552

I've opened pull requests for both so we could implement it there and wait if this PR gets merged here.

As far as I know only user-event depends on identifying element types at the moment, but as the helper itself is strictly DOM related and has nothing to do with simulating events, collecting such abstractions here might prevent creating similar bugs in different places.

@eps1lon
Copy link
Member

eps1lon commented Feb 1, 2021

Thanks for the suggestions. We're going to let this incubate in user-event and see if and in what form we need it in /dom.

I suspect that most use-cases can live with a much simpler nodeType+localName check. That's what we're doing in dom-accessibility-api and we haven't heard any problems so far.

@eps1lon eps1lon closed this Feb 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants