Fixing problems with large numbers and formatNumber

[danprince]

Use a simple check to prevent the decimal rounding code doesn't try to round integers. This prevents formatNumber from returning NaN for numbers that fit the criteria described in #12707.

Tests show it working with numbers lower and upper limit numbers as well as the example from the above issue.

Fixes #12707
Fixes #8674

[lgalfaso]

It looks like this does not solve the problem entirely. eg.

expect(formatNumber(999999999999999.9, pattern, ',', '.', 7))
    .toBe('999,999,999,999,999.9000000');
    // Returns 'NaN.0000000'

[lgalfaso]

Wondering if changing

if (number.toString().indexOf('.') >= 0) {

for

if (number.toString().indexOf('.') >= 0 && number.toString().split('.')[1].length > fractionSize) {

should take care of all the issues

[danprince]

Wouldn't this end up as a tradeoff between fixing those NaNs but also directing some decimal numbers away from the safe decimal rounding function?

[petebacondarwin]

It looks like #8705 had a different way of solving this, which also tests corner cases from different VMs. @danprince can you take a look at the tests there - and the implementation of the fix.

[petebacondarwin]

@lgalfaso can we give stuff like this a milestone if you are commenting on it (along with its related issue), so that it doesn't get triaged multiple times. Thanks

[danprince]

I would suggest that this is a solution to a much smaller part of the problem that #8705 is try to address. However this is definitely a short term solution and it makes the edge cases more complex to understand.

If the ideal outcome would be to solve the problem to the depth of the idiosyncrasies of specific VM implementations, then you only have to look at the size of decimal.js to see that this is a non-trivial thing to hack into the format number function.

Maybe a more elegant short term solution is to add a disclaimer to the docs for currency (and anything else that relies on formatNumber) that explains these edge cases? Or at the very least reminds users of the restrictions regarding precision with 64-bit floating point numbers?

[Narretz]

We could also make it configurable via service etc. which function is used to format / process the number. Then people could include decimal.js or whatever if they need it. But it's probably difficult to match the APIs.