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

expect-puppeteer: change toMatch text matcher to check if argument is regex or not to match #50

Closed
rkoval opened this issue Apr 25, 2018 · 1 comment
Labels
good first issue 🤙 Good for newcomers

Comments

@rkoval
Copy link
Contributor

rkoval commented Apr 25, 2018

First and foremost, I just want to express gratitude for maintaining this project! It's a crucial resource for doing anything with jest and puppeteer.

One pain point though is from usage of text matching option on some of the matchers (e.g., toMatch or toMatchElement). It's confusing (especially for newcomers) that the string you pass to it is actually used as a regex. I realized that it's documented this way, but it would be a lot nicer if the library could do a typeof to check if the argument is a string or object (regex's actual type from what I can tell). Then, it just does a simple includes or an actual match on the textContent respectively. I've run into weird cases where I'm trying to match on money values (e.g., '$1000') and i've found myself beating my head on more than one occasion trying to figure out why text won't match (the $ is not escaped, obviously). Also, sometimes all you really need is a simple string match without needing to remember to escape everything in the regex for what you're matching.

I know that this would bring about a breaking change, so it might not be something you'd be thrilled to add right now. However, I think it could definitely help for readability and usability of this library.

@gregberge
Copy link
Member

I think it is a good move. Since it is not possible to pass object to waitForFunction, the best way to implement it is to use RegExp source and add another argument to say if it is a RegExp or not.

Yeah it is a breaking change but I am OK for it. PR welcome.

gregberge added a commit that referenced this issue May 3, 2018
BREAKING CHANGE: Text is now trimmed and no longer evaluated as a RegExp. If you want this behaviour, use a true RegExp.

Closes #51
Closes #50
gregberge added a commit that referenced this issue May 3, 2018
BREAKING CHANGE: Text is now trimmed and no longer evaluated as a RegExp. If you want this behaviour, use a true RegExp.

Closes #51
Closes #50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue 🤙 Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants