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: update getByTestId types #74

Merged

Conversation

alexkrolick
Copy link
Collaborator

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.

Sure 👍

@kentcdodds kentcdodds merged commit b1b38b5 into testing-library:master May 4, 2018
@alexkrolick
Copy link
Collaborator Author

alexkrolick commented May 4, 2018

I'm actually wondering if getByAltText and other queries should also switch to exact match by default - since regex and functions are supported case-insensitive-partial-match seems a bit gratuitous. Especially since getBy returns the first match automatically which could by some partial match you don't expect.

@kentcdodds
Copy link
Member

🎉 This PR is included in version 2.5.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

@alexkrolick alexkrolick deleted the fix/getByTestId-types branch May 4, 2018 19:38
@kentcdodds
Copy link
Member

I'm actually wondering if getByAltText and other queries should also switch to exact match by default - since regex and functions are supported case-insensitive-partial-match seems a bit gratuitous.

That's fair. We should make that change in a major version bump though. I was thinking that if you wanted to do a more specific match you could use a regex with ^ and $, but maybe we should make it the inverse: use a regex to be more loose and a string to be more strict.

The thing is, if we consider it from a user's standpoint, they don't care whether the letters are capitals or wither the button says: "Submit form" or "Submit". It's all the same to them. I think that's what motivated the partial match for strings in my mind.

@alexkrolick
Copy link
Collaborator Author

alexkrolick commented May 4, 2018

if we consider it from a user's standpoint,

True, but there are other standpoints to consider, i.e., the product spec says button text should be capitalized or a styleguide says all copy should end in ., or for testing compliance with some user-provided theme/translation.

but maybe we should make it the inverse: use a regex to be more loose and a string to be more strict.

I think this makes sense because the most basic "includes text" regex is just /mysubstring/ which is almost the same as "mysubstring".

I could see implementing this by exposing the final options argument to the getAllByAttribute helper to the public methods that use it so that exact: true could toggled.

https://github.com/kentcdodds/dom-testing-library/blob/master/src/queries.js#L73

julienw pushed a commit to julienw/react-testing-library that referenced this pull request Dec 20, 2018
…textNode changes (testing-library#74)

* ensure waitForElement responds to attribute and textNode changes by default

* Add rbrtsmith as a contributor

* Update README with change mutationObserverOptions

* Add missing new line in test
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.

None yet

2 participants