FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add BigInt support by jasonpaulos · Pull Request #211 · msgpack/msgpack-javascript · GitHub

/ msgpack-javascript Public

Add BigInt support - #211

Closed
jasonpaulos wants to merge 3 commits into
msgpack:mainfrom
jasonpaulos:bigint-support
ConversationCommits (3)ChecksFiles changed
Closed

Add BigInt support#211
jasonpaulos wants to merge 3 commits into
msgpack:mainfrom
jasonpaulos:bigint-support

Conversation

jasonpaulos commented May 5, 2022 •
edited
Loading

Copy link
Copy Markdown
Contributor

This PR adds native support for JavaScript BigInts.

I didn't want to cause a breaking change in any way, so I've introduced a few different decoding options that essentially allow library users to opt into this feature. Additionally, since no BigInts are returned by this library by default, everything else should remain functional in environments where BigInt is not supported.

Encoding

Similar to Numbers, BigInts are encoded using the smallest possible MessagePack int type. The added benefit of encoding a BigInt is that integers larger than Number.MAX_SAFE_INTEGER can be encoded with the correct precision.

If you attempt to encode a BigInt larger than the maximum uint64 value or smaller than the minimum int64 value, an error will be thrown, since MessagePack does not support larger integer types.

Decoding

Decoding BigInts is more nuanced. I believe there is no single correct way to do this, since consumers of this library will want different behavior depending on their use cases. For example, some may never want to see a BigInt from this library, some may only want to see BigInts if the value being decoded cannot fit into a Number, and some may want to always receive BigInts for consistency.

Instead of choosing just a single way to decoding integers, I've offered the following possibilities, which can be specified with the new DecodeOptions.intMode option:

  • IntMode.UNSAFE_NUMBER: Always returns the value as a number. Be aware that there will be a loss of precision if the value is outside the range of Number.MIN_SAFE_INTEGER to Number.MAX_SAFE_INTEGER.
  • IntMode.SAFE_NUMBER: Always returns the value as a number, but throws an error if the value is outside of the range of Number.MIN_SAFE_INTEGER to Number.MAX_SAFE_INTEGER.
  • IntMode.MIXED: Returns all values inside the range of Number.MIN_SAFE_INTEGER to Number.MAX_SAFE_INTEGER as numbers and all values outside that range as bigints.
  • IntMode.BIGINT: Always returns the value as a bigint, even if it is small enough to safely fit in a number.

For backwards compatibility, IntMode.UNSAFE_NUMBER is the default value for decoding. As a result, there should be no change in behavior for users who do not specify this option.

At Algorand, we've had success adopting a similar approach for JSON integer decoding: algorand/js-algorand-sdk#260

Tests

New tests have been added to ensure the new functionality works as intended. Additionally, I've enabled the bignum tests from msgpack-test-js.

Feedback is appreciated, please let me know if there are any questions or suggested changes!

Closes #115

jasonpaulos mentioned this pull request May 5, 2022
Open
arendjr mentioned this pull request May 6, 2022
Open

jasonpaulos commented Jun 17, 2022

Copy link
Copy Markdown
Contributor Author

Hi @gfx, is this PR something you would consider adding to the library?

sagacity commented Jul 20, 2022

Copy link
Copy Markdown

We'd also love it if this could be merged. The current ExtensionCodec example isn't really usable since it encodes large integers as strings, which would require special casing on the decoding side (which, in our case, isn't in JS).

sinclairzx81 mentioned this pull request Nov 14, 2022
Closed

jasonpaulos commented Jan 3, 2023

Copy link
Copy Markdown
Contributor Author

Hi @gfx, could you please approve the workflow run for this PR so github actions will run?

gfx commented Jan 11, 2023

Copy link
Copy Markdown
Member

Hi. Thank you for the pull request, and sorry for my delayed response.

I'd like to include the BigInt support in the next major release (v3.0). Please wait for my review & decision.

CHOYSEN commented Mar 1, 2023

Copy link
Copy Markdown

Any update?

This was referenced Jan 24, 2024
Open
Closed
Merged

gfx commented Feb 5, 2025

Copy link
Copy Markdown
Member

Added another solution: useBigInt64

Please use it.

gfx closed this Feb 5, 2025

codecov-commenter commented Feb 5, 2025

Copy link
Copy Markdown

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 97.43590% with 2 lines in your changes missing coverage. Please review.

Project coverage is 98.05%. Comparing base (d59e1af) to head (461429d).
Report is 106 commits behind head on main.

Files with missing lines Patch % Lines
src/Encoder.ts 95.83% 1 Missing ⚠️
src/utils/int.ts 97.22% 1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main     #211      +/-   ##
==========================================
- Coverage   98.14%   98.05%   -0.10%     
==========================================
  Files          16       16              
  Lines         969     1026      +57     
  Branches      206      225      +19     
==========================================
+ Hits          951     1006      +55     
- Misses         18       20       +2

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

BigInt should be handled by native codec instead of extension codec

5 participants

Back | FazBrowse Home | New Git URL