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 incorrect parsing/rounding of BigInt #920

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ItzNotABug
Copy link
Member

@ItzNotABug ItzNotABug commented Jul 27, 2024

What does this PR do?

Fixes the issue where the big integers are incorrect due to precision loss.

Fixes appwrite/console#1473

Test Plan

(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work.)

Related PRs and Issues

Have you read the Contributing Guidelines on issues?

Yes

@stnguyen90
Copy link
Contributor

@TorstenDittmann, you okay with taking this approach?

@loks0n
Copy link
Member

loks0n commented Sep 3, 2024

Can you share more about the issue? What situation does it occur?

@ItzNotABug
Copy link
Member Author

Can you share more about the issue? What situation does it occur?

The values returned on console are different for min/max due to how PHP and JS round off the int/big int. Values should be -
-9223372036854775808 & 9223372036854775807 but JS rounds off to -9223372036854776000 and -9223372036854776000. This causes issues during updates iirc.

@ivanskodje
Copy link

Fixes the issue where the big integers are incorrect due to precision loss.

Does these changes affect how the Console repo parses the number it receives from the backend? I ask, because in Console we are already receiving the correct number from the backend via JSON - but it is only after it parses it into a number object it gets rounded to an incorrect amount causing this issue.

Ref: appwrite/console#1473

@loks0n
Copy link
Member

loks0n commented Nov 26, 2024

Fixes the issue where the big integers are incorrect due to precision loss.

Does these changes affect how the Console repo parses the number it receives from the backend? I ask, because in Console we are already receiving the correct number from the backend via JSON - but it is only after it parses it into a number object it gets rounded to an incorrect amount causing this issue.

Ref: appwrite/console#1473

Yeah, the console SDK is generated using the web sdk with a few extra endpoints

@ItzNotABug
Copy link
Member Author

@loks0n based on the new changes, we could either use that impl or might add a library to handle this scenario. CLI anyway uses the same library used previously to handle this scenario.

cc @TorstenDittmann.

@ItzNotABug ItzNotABug changed the title Use json-bigint for Integer Parsing Fix incorrect parsing/rounding of BigInt Dec 18, 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
4 participants