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

Use json-bingint to parse JSON body #38

Merged
merged 2 commits into from
Oct 21, 2021

Conversation

krithika369
Copy link
Collaborator

@krithika369 krithika369 commented Oct 21, 2021

The Response.json() API does not have provision to handle BigInt.

This PR introduces an option parseBigInt that can be sent to the useApi hook, which, if set, would use the custom json-bigint library to parse the JSON body instead. This will result in a custom BigNumber object where the number is larger than MAX_SAFE_INTEGER.

Example:
Screenshot 2021-10-21 at 9 40 23 AM

@romanwozniak
Copy link
Contributor

Do we know the implications of using this library compared to response.json()? Is there any difference in performance between the two? I'm not super comfortable to switch to it as it's affecting pretty much every MLP component. Can we make support of it as an option in useApi hook (default to false)?

@krithika369
Copy link
Collaborator Author

@romanwozniak thanks for the feedback. I agree with the concern and looking closely, I see a potential open issue in the library as well: sidorares/json-bigint#39

Added the parseBigInt option to keep it conditional.

Copy link
Contributor

@romanwozniak romanwozniak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, looks good to me!

@krithika369 krithika369 merged commit 64ad63c into caraml-dev:main Oct 21, 2021
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.

2 participants