-
-
Notifications
You must be signed in to change notification settings - Fork 67
fix: not crash if getSuggestedQuery throw an exception #215
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: not crash if getSuggestedQuery throw an exception #215
Conversation
try { | ||
return getSuggestedQuery(element, 'get', method); | ||
} catch (e) { | ||
return undefined; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not a big fan of having a try/catch
inside a loop. Also, shouldn't this be fixed upstream?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because if other queries don't throw an exception I want to show them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand. And isn't this fixable in @testing-library/dom
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This problem is that it is a new usecase.
We can solve the problem anyway but I think is better to maintain this code to avoid other problem of this kind.
I'm closing this one, as you've fixed it upstream (testing-library/dom-testing-library@373dbc4). The dependency has been updated in #220, fixing the root issue. Thanks for the effort 👍, and the upstream fix 😎 |
I suggest you to keep this PR because you can have other exceptions on |
Do you have concrete examples? Or do you mean as protection for future changes? |
only as a protection to avoid that in case of error on dom-testing-library the playground will crash |
fix #214
What: I have fixed the issue
Why: Because it is a bad error
How: if
getSuggestedQuery
throw an exception the method is skippedChecklist: