Skip to content

Conversation

alexlopashev
Copy link

During using this extension (thank for it by the way) I faced a problem: is precision loss in numbers, even if they are integers. I know it's JavaScript number type problem, so I suggest fix - wrap each number in specific patterned string and then unwrap it and show as number.

@callumlocke
Copy link
Owner

Thanks for this but I'm not sure if it's a good idea. Why would you want to preserve details in the rendered JSON that are not going to be visible to a JS engine? The extension is supposed to show you the parsed data structure. If a number parses as 1, then I think it should be displayed as 1, even if it was originally represented as 1.00 or 1e0 in the JSON string that came down the wire.

@alexlopashev
Copy link
Author

In fact this fix was originally proposed for integers, not all decimals. I spent couple hours digging for problems in java code when I used this extension for viewing responses of my application but this was a problem of respresentation long (64 bits) values. For example, this json is rendered not as accurately as I wanted - {"32 bits":2147483647, "64 bits":9223372036854775807}, long value suffered from precision losses. But I haven't encountered problem with decimals, maybe this fix will be more suitable only for integers. I am a living example of that fact that people use this extension not only with their JavaScript-code, but also with C++/Java/Python with 64-bits and more integer types.

@callumlocke
Copy link
Owner

ok I see the issue, if you're using the browser to preview JSON in development but using something like C to consume it, then you want to see the original number from the JSON string.

I am working on v2, and I will use your code, but I'm going to make it an opt-in, because I think it's better for most users to see what the JS engine sees when it parses it

@alexlopashev
Copy link
Author

Cool, thanks

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