Skip to content

update typings with dom-testing-library queries and export them #130

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
merged 1 commit into from
Jul 2, 2018

Conversation

jpavon
Copy link
Collaborator

@jpavon jpavon commented Jul 2, 2018

What: Update queries typings to match dom-testing-library

Why: Not up to date typings with dom-testing-library

How: Modify typings/index.d.ts.

Checklist:

  • Documentation
  • Tests
  • Ready to be merged
  • Added myself to contributors table

@kentcdodds kentcdodds added the TypeScript Related to TypeScript. Note: only certain maintainers handle TypeScript labeled issues. label Jul 2, 2018
Copy link
Collaborator

@pbomb pbomb left a comment

Choose a reason for hiding this comment

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

It's hard to parse the diff because so many lines were moved around. It's hard to tell what's new vs what was just moved. Also, I didn't think all those newly exported functions are actually exported from react-testing-library. I just thought they were part of the RenderResult type, which was already defined that way.

Can you provide some more context as to why these changes are being made?

@jpavon
Copy link
Collaborator Author

jpavon commented Jul 2, 2018

@pbomb I changed the order to match with the order here: https://github.com/kentcdodds/dom-testing-library/blob/master/typings/queries.d.ts and they are exported because this functions can also be imported from 'react-testing-library'

@pbomb
Copy link
Collaborator

pbomb commented Jul 2, 2018

Okay, making the orders consistent seems reasonable to do. 👍

I still didn't think the dom-testing-library functions were re-exported from react-testing-library. Where do you see that in the code? @kentcdodds can you confirm if the dom-testing-library query functions are re-exported from react-testing-library?

@pbomb
Copy link
Collaborator

pbomb commented Jul 2, 2018

ah, nice! I totally missed that when I read the file the first time. This PR looks good to me!

@pbomb
Copy link
Collaborator

pbomb commented Jul 2, 2018

Thanks @jpavon!

@kentcdodds kentcdodds merged commit 36e1349 into testing-library:master Jul 2, 2018
@kentcdodds
Copy link
Member

🎉 This PR is included in version 4.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@jpavon
Copy link
Collaborator Author

jpavon commented Jul 2, 2018

@pbomb @kentcdodds I'm thinking having export * from 'dom-testing-library' inside the index.d.ts makes more sense and would remove a lot of duplication, let me know what you think I can create a pr in the next days.

@kentcdodds
Copy link
Member

Sounds good to me

julienw pushed a commit to julienw/react-testing-library that referenced this pull request Dec 20, 2018
…g-library#130)

**What**:
Added attribute argument to type  `QueryByAttribute` and `AllByAttribute`

**Why**:
This argument seems to be falsely missing.

Usage of `AllByAttribute` can bee seen here:
https://github.com/BenjaminEckardt/dom-testing-library/blob/master/src/queries.js#L60

Usage of `QueryByAttribute` is a bit less obvious due to currying but can be seen here:
https://github.com/kentcdodds/dom-testing-library/blob/master/src/queries.js#L153

**How**:
Just added the missing attribute.

**Checklist**:
- [ ] Documentation N/A
- [x] Tests
- [x] Ready to be merged
- [x] Added myself to contributors table
lucbpz pushed a commit to lucbpz/react-testing-library that referenced this pull request Jul 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TypeScript Related to TypeScript. Note: only certain maintainers handle TypeScript labeled issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants