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

fix!: decode large non-safe numbers with no decimal as BigInt #46

Merged
merged 1 commit into from
Aug 6, 2021

Conversation

rvagg
Copy link
Member

@rvagg rvagg commented Aug 5, 2021

Heads up @achingbrain, this is going to make dag-json do the same thing that dag-cbor does, returns BigInt when you're outside of safe integer range. We're currently in a lossy situation with those numbers, but with this change we get to safely pull the integer out as it is in the encoded JSON.

But, it's a breaking change because previously it's all just Number.


BREAKING CHANGE

Because previously, all numbers just get decoded as Number, after this
change any number in the JSON that doesn't have a decimal place and is
outside of the JavaScript safe integer range will be provided as a
BigInt.

BREAKING CHANGE

Because previously, all numbers just get decoded as `Number`, after this
change any number in the JSON that doesn't have a decimal place and is
outside of the JavaScript safe integer range will be provided as a
`BigInt`.
@rvagg
Copy link
Member Author

rvagg commented Aug 5, 2021

picked up with https://github.com/ipld/codec-fixtures, failing tests in the JS stack:

  • int--11959030306112471732
  • int--9007199254740992
  • int--9007199254740993
  • int--9223372036854776000
  • int-11959030306112471731
  • int-18446744073709551615
  • int-9223372036854775807

this changes fixes all of those and lets us round-trip properly through the different codecs

@rvagg rvagg merged commit 328eda1 into master Aug 6, 2021
@rvagg rvagg deleted the rvagg/bigint branch August 6, 2021 01:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant