build: upgrade and migrate to Dinero.js v2 #1030
Conversation
|
@sarahdayan Thanks for this PR! For details on logging in as a user, see the note in the README. That said, you can just run the Cypress tests locally that are failing. |
|
|
||
| export const dineroUSD = (amount: number) => dinero({ amount, currency: USD }); | ||
|
|
||
| export const format = (dineroObject: Dinero<number>, fractionDigits?: number) => |
There was a problem hiding this comment.
@sarahdayan I see that Dinero v2 is in alpha and I'd like to provide some feedback around this DX.
While the flexibility for this format method is great, I'm wondering why the need to define such a large callback around a given currency? This DX feels a bit unintuitive and for a currency's "default", I would expect the library to handle this for me. Have you've considered including default helpers such as this or simplifying this piece for v2?
There was a problem hiding this comment.
Hey @kevinold and thanks for the feedback!
You're right, the flexibility comes at the expense of simplicity here. I'm currently rethinking this function for several other reasons, and providing a default return value is part of the thinking process.
The main reason why picking a default is difficult is that Dinero also handles non-decimal currencies. While it might make sense to return "USD 10.50" by default for dinero({ amount: 1050, currency: USD }), there's no good option when passing non-decimal objects. More context in this issue.
I've reworked things and now should be able to expose enough information to toFormat to make formatting easier.
Hey Cypress team!
You're using Dinero.js 1.8.1 in this app, and this makes me happy 😊 I recently released Dinero.js v2, which is a lot lighter, safer, and adapted to modern front-end practices.
See documentation
I took the liberty to perform the migration based on your usage, and did my best to follow the coding style and practices I observed in the codebase. Please let me know if you need me to change anything, or feel free to amend the PR yourself if you prefer.
Edit: Ouch, looks like UI tests are failing, which I believe is due to formatting issues. I wasn't able to test locally because I wasn't entirely sure how I was supposed to log into the app. Does anyone have input on this?