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

Numbers past 53 bits break toCurrency and toNumber #511

Closed
JackieCalapristi opened this issue Oct 2, 2018 · 4 comments
Closed

Numbers past 53 bits break toCurrency and toNumber #511

JackieCalapristi opened this issue Oct 2, 2018 · 4 comments
Assignees
Labels

Comments

@JackieCalapristi
Copy link

See here:

screen shot 2018-10-02 at 12 07 42 pm

Upon further investigation, this is actually a problem with javascript's toFixed() function used by i18n-js here:

return decimalAdjust('round', number, -precision).toFixed(precision);

And the reason toFixed() breaks on numbers like this is because JS only supports 53 bit integers which 300000000000020000 is larger than. See more info on this here: http://2ality.com/2012/07/large-integers.html.

Ideally we would want to find a patch for this that doesn't break rounding for imprecise floating points (which appears to be the reason toFixed() was added to this line here:

return decimalAdjust('round', number, -precision).toFixed(precision);
).

Not as ideally we could add a disclaimer for this known issue instead or in the meantime. I'm happy to help in any way I can on this!

@PikachuEXE
Copy link

Do you any known patch that solves this issue?
I have never encountered any large enough number yet

@PikachuEXE PikachuEXE self-assigned this Oct 3, 2018
@PikachuEXE
Copy link

Added to known issues until there is a permanent fix available
608d552

@JackieCalapristi
Copy link
Author

JackieCalapristi commented Oct 10, 2018

This is a widely recognized issue with Javascript. Javascript is very self-aware about this as well.

This issue can be solved with a library called BigInteger.

Before implementing a patch we could also:

  • Warn when user input goes past Number.MAX_SAFE_INTEGER for all functions affected by this.
  • Write a disclaimer in an appropriate readme for all functions affected by this. We could even reference TC39 proposals in the works to address this.

@PikachuEXE
Copy link

I wonder how the "warning" be sent
Since this library might be used in browser / NodeJS process / even mobile app (screenshot in #513)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants