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

Handle BigInt numbers from API #920

Closed
wants to merge 3 commits into from

Conversation

"extends": "react-app"
"extends": "react-app",
"env": {
"es2020": true
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Otherwise, BigInt constructor gives an ESLint error 🤦

Copy link

Pull Request Test Coverage Report for Build 11275100558

Details

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.02%) to 70.957%

Files with Coverage Reduction New Missed Lines %
packages/react-api/src/api/lds.js 1 72.73%
Totals Coverage Status
Change from base Build 11232686264: 0.02%
Covered Lines: 2894
Relevant Lines: 3751

💛 - Coveralls

Copy link

Visit the preview URL for this PR (updated for commit 6a75fc4):

https://cartodb-fb-storybook-react-dev--pr920-bug-445700-handl-zhncxf6c.web.app

(expires Thu, 17 Oct 2024 13:36:48 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 517cc4d31d7e09cf277774e034094b67c301cd4c


const res = await getStats({ source: QUERY_SOURCE, column: 'injuries' });

expect(res.rows[0].quadbin).toEqual(BigInt(5256683984082960383));
Copy link
Member

Choose a reason for hiding this comment

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

Careful here! Writing this out as BigInt(5256683984082960383) will truncate the number before it's passed to the BigInt function, so these are not equal, note the n suffix:

BigInt(5256683984082960383) === BigInt(5256683984082960383n) // false!

Comment on lines +4 to +9
const bigIntReviver = (key, value) => {
if (typeof value === 'number' && value > Number.MAX_SAFE_INTEGER) {
return BigInt(value);
}
return value;
};
Copy link
Member

Choose a reason for hiding this comment

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

See above — unfortunately I think that it's necessary to receive the value as a string and not an integer. By the time the reviver receives a numeric value, I think it will be too late to recover the original integer.

Possible solution in https://stackoverflow.com/a/69644630, though maybe this is complex enough that we want a dependency to help...

@jmgaya jmgaya closed this Oct 11, 2024
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