dinerojs / dinero.js

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

Imprecise number conversion #84

Closed toninorair closed 4 years ago

toninorair commented 4 years ago

console.log("Dinero = ", Dinero({amount: 100000 * 10 ** 18, precision: 18}).toUnit()); output - 99999.99999999999

Interesting that changing precision from 18 to 19 works perfectly fine returning 100000.

devcrev commented 4 years ago

The documentation says the "amount" option should be "The amount in minor currency units (as an integer)." The documentation also says "THROWS TypeError: If amount or precision is invalid."

Perhaps there should be a "throw" when the amount > Number.MAX_SAFE_INTEGER or amount < Number.MIN_SAFE_INTEGER. Those numbers are +/- 9007199254740991.

It is interesting that when using the maximum and minimum you still get the wrong result, i.e.

console.log("Dinero = ", Dinero({amount: Number.MAX_SAFE_INTEGER}).toUnit());
Dinero =  90071992547409.9
console.log("Dinero = ", Dinero({amount: Number.MIN_SAFE_INTEGER}).toUnit());
Dinero =  -90071992547409.9

Both outputs should end in .91

These work correctly though:

console.log("Dinero = ", Dinero({amount: Number.MAX_SAFE_INTEGER - 1}).toUnit());
Dinero =  90071992547409.9
console.log("Dinero = ", Dinero({amount: Number.MIN_SAFE_INTEGER + 1}).toUnit());
Dinero =  -90071992547409.9
console.log("Dinero = ", Dinero({amount: Number.MAX_SAFE_INTEGER - 2}).toUnit());
Dinero =  90071992547409.89
console.log("Dinero = ", Dinero({amount: Number.MIN_SAFE_INTEGER + 2}).toUnit());
Dinero =  -90071992547409.89
sarahdayan commented 4 years ago

Indeed, 100000 * 10 ** 18 is above Number.MAX_SAFE_INTEGER, so you're hitting the limitation for 64-bit floating point IEE 754 numbers. Dinero.js can't perform calculations safely with such amounts as numbers, neither do any JS program that calculates with numbers.

In v2, you'll be able to use whatever you want (number, bigint, a third-party library like Big.js, etc.) by implementing your own calculator. You can read more about it in the RFC. Feel free to comment!

@devcrev It looks like 90071992547409.91 can't be properly represented in JavaScript (certainly due to how it goes back and forth between decimal and binary representation). If you try to input it in your runtime, it rounds down to 90071992547409.9.

Capture d’écran 2020-03-07 à 15 04 18

This article and this converter are quite helpful.

I'll go more in-depth into the problem later on, so I can give you a more precise explanation. It will probably either go in the docs or a blog post on frontstuff.io.

There's nothing to do in v1. I won't throw if user input a number over Number.MAX_SAFE_INTEGER, as this would be breaking. However, a documentation entry would be more than welcome (so far it was implied, but explicit is always better than implicit).

If anyone wants to take a stab at it before I do, feel free!

sarahdayan commented 4 years ago

Just updated the documentation.

sarahdayan commented 4 years ago

:tada: This issue has been resolved in version 1.8.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: