-
Notifications
You must be signed in to change notification settings - Fork 2
[CI-3307] useProductInfo hook tests #72
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
[CI-3307] useProductInfo hook tests #72
Conversation
CI-3307 [MVP - OS UI PLP] - Add tests for useProductCard hook
Business Outcome:
Definition of done:
|
| } | ||
|
|
||
| const getPrice = state?.itemFieldGetters?.getPrice; | ||
| const getPrice = tryCatchify(state?.itemFieldGetters?.getPrice); |
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 don't know what we should be doing here other than try catching it.
I was thinking of doing this to everything in itemFieldGetters.
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.
cspeller still complaining. Lets add the word Catchify to the list of cspeller words
src/utils.tsx
Outdated
| try { | ||
| return func(...args); | ||
| } catch (e) { | ||
| // do nothing |
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.
Can we add something like the logger and have call it here? It would only console log in dev mode. That way we can let the user know there is a problem with the implementation during development but don't break anything in prod if the value is missing
esezen
left a comment
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.
LGTM!
Pull Request Checklist
Before you submit a pull request, please make sure you have to following:
PR Type
What kind of change does this PR introduce?