dinerojs / dinero.js

Create, calculate, and format money in JavaScript and TypeScript.
https://v2.dinerojs.com/docs
MIT License
6.28k stars 188 forks source link

[Bug]: formatting big.js objects with scale larger than 21 fails #716

Open bkiac opened 1 year ago

bkiac commented 1 year ago

Is there an existing issue for this?

Current behavior

I'm using big.js with dinero because I need it for cryptocurrencies and large USD values but after following the documentation to create a custom calculator I noticed that formatting breaks if the scale is too large (>= 22)

My first problem was that I receive arbitrary precision USD values from an API and I need to calculate the scale of these values. This is how I did it, but I feel I'm missing something and this has to be an overkill:

const usd = (dollars: string) => {
    // Get cents from dollars
    const cents = new Big(dollars).mul(conversion);
    // Calculate remaining fraction
    const fraction = cents.mod(1);
    const fractionLength = fraction.eq(0) ? 0 : fraction.c.length;
    // Set scale based on fraction length
    const scale = USD.exponent.plus(fractionLength);
    // Make sure integer is passed to dinero
    const amount = new Big(cents.mul(USD.base.pow(fractionLength).toFixed()));
    return dinero({
        currency: USD,
        amount,
        scale,
    });
};

and when I receive a value that has precision 22 or larger, toDecimal function breaks and the output is in scientific notation with multiple decimal points:

const n = usd("1000000000.9876543211211111111111")
toDecimal(n) // 1000000000.9.876543211211111111111e+21

Expected behavior

I had to create a custom formatter to make it work:

const formatter: Formatter<Big> = {
    toNumber: (v) => (v ? v.toNumber() : 0),
    toString: (v) => (v ? v.toFixed() : ''),
};

Unless I'm doing something incredibly stupid with parsing the arbitrary precision, I think this should be added to the docs. I can open a PR if you can point me in the right direction.

And a quick question, why is the amount arg in the formatter functions optional?

Steps to reproduce

https://replit.com/@bkiac/dinerowithbig#index.ts

Version

2.0.0-alpha.13

Environment

any

Code of Conduct

johnhooks commented 1 year ago

I tried to look through this one for you, but its hard to debug the issue without a Big.js calculator in the codebase. I could just paste yours in, but if we're committing time to the issue we might as well solve it for good. @sarahdayan are you interested in a Big.js Dinero calculator and supporting it? Or should we just find this bug?

sarahdayan commented 1 year ago

@johnhooks Big.js is installed as a dev dependency for tests, so we can use test files to debug this issue (for example, in this spec file).

johnhooks commented 1 year ago

I found it. I'll take another look.

johnhooks commented 1 year ago

@bkiac you are 100% on the right track. The solution is to add your own formatter when you use createDinero, because the default formatter will not format Big.js correctly.

formatter = {
  toNumber: Number,
  toString: String,
},

In PR #724 I'm currently doing this:

https://github.com/dinerojs/dinero.js/blob/dff300ed16f46c156d09bc02450162a4a6e5bc9c/test/utils/createBigjsDinero.ts#L6-L23

And a quick question, why is the amount arg in the formatter functions optional?

I'm trying to track down why the formatter has to handle possible undefined values.

johnhooks commented 1 year ago

My first problem was that I receive arbitrary precision USD values from an API and I need to calculate the scale of these values. This is how I did it, but I feel I'm missing something and this has to be an overkill

Just realized this was a multipart question.

dinero.js does not provide a way to determine scale from an input amount. I can definitely see the utility of a function to handle this in a uniform way. But the library expects input to be a "whole" number, it's safest for JavaScript number type and the only option for bigint.

johnhooks commented 1 year ago

@bkiac I couldn't find any reason that the value parameters of the Formatter were optional. PR #725 makes them required. @sarahdayan is that alright with you? Because if value is undefined in any situation I would prefer an error to be thrown rather than receive an arbitrary value.

bkiac commented 1 year ago

ty for taking a look at this, making value and formatter required with a custom calculator is a good solution 👍