Skip to content

Breaking change proposal: Use exact string match by default #30

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

Closed
alexkrolick opened this issue May 4, 2018 · 1 comment · Fixed by #31
Closed

Breaking change proposal: Use exact string match by default #30

alexkrolick opened this issue May 4, 2018 · 1 comment · Fixed by #31
Assignees
Labels
BREAKING CHANGE This change will require a major version bump help wanted Extra attention is needed

Comments

@alexkrolick
Copy link
Collaborator

Same as testing-library/react-testing-library#76 which would also expose this API

Describe the feature you'd like:

Make exact match the default instead of partial case-insensitive match. Regex would be recommended for those cases instead.

  • partial: /mystring/
  • case-insensitive partial: /mystring/i
  • prefix: /^mystring/
  • postfix: /mystring$/
  • case-insensitive full string: /^mystring$/i

One thing that would more involved to replicate with inline regexes is that the current implementation trims the string and replaces symbols with spaces:

- const normalizedText = textToMatch.trim().replace(/\s+/g, ' ')

See testing-library/react-testing-library#74 (comment)

Suggested implementation:

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

Describe alternatives you've considered:

Leave the matcher alone

Teachability, Documentation, Adoption, Migration Strategy:

Show common regex patterns like the ones above^ in the docs.

Is this a breaking change?

Yes

@kentcdodds kentcdodds added help wanted Extra attention is needed BREAKING CHANGE This change will require a major version bump labels May 4, 2018
@alexkrolick alexkrolick self-assigned this May 4, 2018
@kentcdodds
Copy link
Member

🎉 This issue has been resolved in version 2.0.0 🎉

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
BREAKING CHANGE This change will require a major version bump help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants