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

User event element types are more narrow than what is returned by RTS queries #648

Closed
GreenGremlin opened this issue Apr 12, 2021 · 5 comments · Fixed by #649
Closed

User event element types are more narrow than what is returned by RTS queries #648

GreenGremlin opened this issue Apr 12, 2021 · 5 comments · Fixed by #649
Labels

Comments

@GreenGremlin
Copy link
Contributor

GreenGremlin commented Apr 12, 2021

  • @testing-library/user-event version:13.1.1
  • Testing Framework and version: Jest v26.6.3
  • DOM Environment: jsdom v16.5.0

Relevant code or config

userEvent.paste(screen.getByRole('textbox'), 'Ralph');
// or
userEvent.upload(screen.getByLabelText('labelText'), file);

What you did:

Compiled using TypeScript.

What happened:

Type error

Argument of type 'HTMLElement' is not assignable to parameter of type 'HTMLInputElement | HTMLTextAreaElement'.
  Type 'HTMLElement' is missing the following properties from type 'HTMLTextAreaElement': autocomplete, cols, defaultValue, dirName, and 26 more.ts(2345)

Reproduction repository:

N/A

Problem description:

Elements returned by react-testing-library queries are of type HTMLElement but user-event functions take more narrow element types like HTMLInputElement | HTMLLabelElement. It looks like the type change was made in @testing-library/user-event v13.

Suggested solution:

The type definitions for userEvent.paste and userEvent.upload should either be relaxed to allow HTMLElement or query methods should be added that return the more narrow element types. Type casting should not be required.

@GreenGremlin GreenGremlin changed the title User event element types more narrow than what is returned by RTS queries User event element types are more narrow than what is returned by RTS queries Apr 12, 2021
@SimpleCookie
Copy link

I can confirm this is a problem for TS users.

@ph-fritsche
Copy link
Member

ph-fritsche commented Apr 14, 2021

Thanks for raising this and providing a PR. ❤️

I'm not sure about this change yet.
I see that it eases typing the test, but it also removes the hint that this API does not support any HTMLElement.
So relaxing it kind of defeats the purpose of typings.

The typings have not been part of the code before v13 as it was JS. So they've been changed to formally match the code.
Personally I'd prefer to narrow the type further to meet the actual limitations of the implementation instead of relaxing it.

A better looking solution might be to add type parameters to the query typings in dom-testing-library.

type GetByRole = <T extends HTMLElement = HTMLElement>(
  container: HTMLElement,
  role: ByRoleMatcher,
  options?: ByRoleOptions,
) => T
type HTMLTextInputElement = HTMLInputElement & {type: 'text'}
userEvent.paste(screen.getByRole<HTMLTextInputElement>('textbox'), 'foo')

But I think the type cast is fine as it flags that there is an assumption in the code.

userEvent.paste(screen.getByRole('textbox') as HTMLTextInputElement, 'foo')

@GreenGremlin
Copy link
Contributor Author

GreenGremlin commented Apr 16, 2021

Type casting is not ideal, since it guarantees nothing. A better solution would be to have a way to filter query results by tag name, which would guarantee the query is returning the expected type. In the meantime, it feels broken for user-event functions to be incompatible with react-testing-library queries, if they can't be used without type casting.

@ph-fritsche
Copy link
Member

ph-fritsche commented Apr 17, 2021

It never guarantees anything, yet we don't apply unknown/any everywhere.
It highlights the assumption (that the element is some special type and not any HTMLElement) to the developer.
The dev is making that assumption there. The query being used does not necessarily return this special type, but - knowing the examined document - the dev judges this assumption to be safe.
(Even HTMLElement is not guaranteed because you could run the query on a document with node type interfaces which intersect with the assumed HTMLElement but don't implement it.)
(Or if you want to break it, don't go further than <svg data-testid="foo">.)
You see where I'm heading?

I agree that providing a filter like our isElementType on the queries might improve the situation.

I also see that using TypeScript can be cumbersome sometimes and you might prefer taking a shortcut.
I'm just not sure yet if the harm done here per requiring extra typing outweighs the harm done per hiding an unsafe assumption.

@github-actions
Copy link

🎉 This issue has been resolved in version 13.1.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants