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

feat: provide better support for non-decimal currencies #309

Merged
merged 22 commits into from
Dec 4, 2022

Conversation

sarahdayan
Copy link
Collaborator

@sarahdayan sarahdayan commented Aug 1, 2021

This PR changes the way Dinero.js does formatting, providing better support for non-decimal currencies and large integers. Many changes are going in this PR, as this touches several concepts of the library.

Currencies now accept multi-bases

Currencies can have multiple bases. It's the case for the pre-decimal pound sterling, the livre tournois of the French Old Regime, or the Harry Potter wizarding currency of Great Britain. Users creating custom currencies (e.g., for games) may also want to create such currencies.

Until now, Dinero.js allowed you to support such currencies by providing the number of units of the smallest subdivision to the biggest (e.g., 240 pence in a pound) as the base. It meant that toUnit could only return a double representation of the amount, and that you'd have to write a lot of logic to properly format it.

Accepting multiple-bases lets users express this complexity and enables better formatting in the new toUnits function.

toUnit is replaced with toUnits

The toUnit function currently returns a double, which is problematic at several levels:

  • IEEE 754 doubles are unreliable, which is why Dinero.js avoids manipulating them altogether.
  • It makes no sense to ever return a non-decimal currency in a decimal representation.
  • Large integers can't be accurately casted to numbers, so this operation can't work well with them.

The decimal representation (e.g., 10.5) is prevalent in systems using a decimal currency, but this is only a human representation, and only matters when you're displaying an amount on your site or app. Programming-wise, this representation isn't particularly helpful, and it's even limiting.

This PR removes toUnit and replaces it with toUnits, which returns each subdivision as an integer (a number, a bigint, a Big, depending on what you use as the amount type), in an array.

This accounts for decimal and non-decimal currencies, without forcing the "human" representation of one over the other. Thanks to multi-bases, the function handles the heavy lifting of distributing an amount into the right amount of each of its subdivision (e.g., 267 pence is 1 pound, 2 shillings and 3 pence, toUnits returns [1, 2, 3]). Since toUnits is now used by toFormat under the hood (instead of toUnit), it makes formatting even easier.

toFormat is replaced with toDecimal

Until now, toFormat was the catch-all formatting function for any formatting need. It was also "only" a wrapper around toUnit, exposing the right information in a transformer function for users to format and display objects.

An intermediary design was to expose both the data from toUnits and a decimal representation in toFormat. But ultimately, there's no point in having a single function that exposes everything: it's computationally more expensive and it does too many things.

Instead, this PR introduces toDecimal, a formatter function that returns a stringified decimal representation. This is more reliable because of the string format which prevents the system from converting it to an inaccurate binary representation.

Another benefit is that the string representation retains the exponent, meaning that $10.50, which used to be returned as 10.5 will now be returned as "10.50". If users need this as a double, despite the potential lack of accuracy (e.g., to manipulate it or use it with the Intl API), they can easily cast it with Number or parseFloat, but that's their responsibility.

toDecimal is only meant to be used with decimal, single-base objects. For multi-base or non-decimal needs, toUnits is recommended.

toUnits and toDecimal take a transformer as its second parameter

declare function f<TAmount, TOutput>(d: Dinero<TAmount>, t: Transformer<TAmount, TOutput>): TOutput;

This makes it easier for users to customize the output based on the exposed value:

  • A tuple in toUnits
  • A stringified double in toDecimal
const formatted = toDecimal(d, ({ value, currency }) => `${currency.code} ${value}`);

// Instead of

const decimal = toDecimal(d);
const formatted = `${toSnapshot(d).currency.code} ${decimal}`;

PR notes

BREAKING CHANGE: the toUnit and the toFormat functions were removed.

fix #294

@vercel
Copy link

vercel bot commented Aug 1, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/dinerojs/dinerojs/G99Pp5thYF5uMaFako93DFaP1fv8
✅ Preview: https://dinerojs-git-feat-to-units-dinerojs.vercel.app

@codesandbox-ci
Copy link

codesandbox-ci bot commented Aug 1, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 2bf6376:

Sandbox Source
@dinero.js/example-cart-react Configuration
@dinero.js/example-cart-vue Configuration
@dinero.js/example-pricing-react Configuration
@dinero.js/example-starter Configuration
silent-sun-lrzboj Issue #294

@sarahdayan sarahdayan changed the title feat: replace toUnit with toUnits feat: provide better support for non-decimal currencies Oct 16, 2021
@sarahdayan
Copy link
Collaborator Author

sarahdayan commented Dec 3, 2022

@johnhooks That works for me, and makes the build slightly smaller by not setting default values. Check 2bf6376 and let me know what you think :)

@johnhooks
Copy link
Contributor

Looks great, I'd say it's ready!

@sarahdayan sarahdayan merged commit e7e9a19 into main Dec 4, 2022
@sarahdayan sarahdayan deleted the feat/to-units branch December 4, 2022 11:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[Bug]: toUnit can't be accurate when used with big integers
2 participants