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

Fix examples + types #77

Merged
merged 4 commits into from
May 6, 2018

Conversation

alexkrolick
Copy link
Collaborator

@alexkrolick alexkrolick commented May 6, 2018

Closes #76

What:

Update docs to match dom-testing-library v2 api

Why:

How:

Checklist:

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

@alexkrolick alexkrolick changed the title Fix/examples types Fix examples + types May 6, 2018
kentcdodds
kentcdodds previously approved these changes May 6, 2018
Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

I'd love it if @maddijoyce could review this as well.

@alexkrolick
Copy link
Collaborator Author

alexkrolick commented May 6, 2018

@maddijoyce the typescript syntax is almost certainly wrong 😃

Copy link
Collaborator

@maddijoyce maddijoyce left a comment

Choose a reason for hiding this comment

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

The options for getByLabelText need a slight change.

) => HTMLElement | null
getByLabelText: (
id: TextMatch,
options?: {selector: string; TextMatchOptions},
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably be
options?: { selector?: string; } & TextMatchOptions

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

does that mean "all the keys from TextMatchOptions plus selector?"

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yeah that's correct, it's a union type. I also made the selector optional, because it's * by default.

@maddijoyce
Copy link
Collaborator

All looks good there now. As far as separating the types into their respective libraries, I'll put a PR through this week.

@alexkrolick
Copy link
Collaborator Author

alexkrolick commented May 6, 2018

The inline type signatures in the README for the wrapped methods don't have the new options yet, but maybe that can be a followup

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Coolio! Thanks friends! 👏

@kentcdodds kentcdodds merged commit 1dfcd73 into testing-library:master May 6, 2018
@alexkrolick alexkrolick deleted the fix/examples-types branch May 6, 2018 04:38
@kentcdodds
Copy link
Member

🎉 This PR is included in version 3.0.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

julienw pushed a commit to julienw/react-testing-library that referenced this pull request Dec 20, 2018
…her than globally (testing-library#77)

You have to handle shims yourself if not using jest without this
lucbpz pushed a commit to lucbpz/react-testing-library that referenced this pull request Jul 26, 2020
chore: 🤖 update @babel/preset-env
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.

Breaking change proposal: Use exact string match by default
3 participants