Skip to content

Fix options types for extend API #43

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

Merged

Conversation

jrolfs
Copy link
Member

@jrolfs jrolfs commented Aug 14, 2020

Finally a PR from me that's intentional 😅

This fixes the type definitions for the options parameter for the queries when using the extend entry point.

Some of the queries accept a selector option and some don't so I just made sure that the derived IScopedQueryMethods pulls the options type from the respective definition in IQueryMethods.

Copy link
Collaborator

@patrickhulce patrickhulce left a comment

Choose a reason for hiding this comment

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

great catch thanks @jrolfs! 🎉

@patrickhulce
Copy link
Collaborator

oof sorry @jrolfs I forgot to merge this the other day and just merged a conflicting PR 😬 I think it might have fixed the same underlying issue though, if you just want to revert the typedef changes and ensure the tests still work?

@jrolfs jrolfs force-pushed the fix/options-types branch from 857bac4 to 9409ea8 Compare August 18, 2020 17:42
@jrolfs
Copy link
Member Author

jrolfs commented Aug 18, 2020

Haha, sigh, yeah I got one-upped in the TypeScript department here 😛. I just cherry-picked this into the Playwright fork... I think these tests are still worth merging?

@patrickhulce
Copy link
Collaborator

I think these tests are still worth merging?

Most def 👍 thanks! 🎉

@patrickhulce patrickhulce merged commit a1c99b6 into testing-library:master Aug 18, 2020
@jrolfs jrolfs deleted the fix/options-types branch August 18, 2020 18:00
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