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

Support Integers outside the range [-(2**53)+1, (2**53)-1] #292

Open
azaslavsky opened this issue Oct 4, 2022 · 6 comments
Open

Support Integers outside the range [-(2**53)+1, (2**53)-1] #292

azaslavsky opened this issue Oct 4, 2022 · 6 comments

Comments

@azaslavsky
Copy link

azaslavsky commented Oct 4, 2022

This is issue is somewhat related these two issues, but is not so much concerned with handling BigInt literals (ex: 12n) as simply supporting very absolutely large numbers, namely those that fall outside the [-(2**53)+1, (2**53)-1] range recommended by the JSON spec.

My specific use case is potentially using JSON5 as the human-readable for of a binary serialization format. For this purpose, expanding the range of support to [-(2**63)+1, (2**64)-1] is sufficient. However, in the Javascript case this requires using BigInt anyway, which can be arbitrarily large, so expanding to this range would implicitly allow any valid JSON number to be successfully parsed and stringified.

This is currently not supported, as seen here:

let j5 = require('json5');
let parsed = j5.parse('{"min_int64": -9223372036854775808, "max_int64": 9223372036854775807, "max_uint64": 18446744073709551615}');
console.log(parsed);

/* Logs the following - all 3 values are incorrect!
{
  min_int64: -9223372036854776000,
  max_int64: 9223372036854776000,
  max_uint64: 18446744073709552000
} */

I wonder if it wouldn't be possible to extend the library to automatically process these to and from BigInts when parsing and stringifying, respectively? To maintain ES5 compatibility, I imagine this could look something like:

  1. Check that typeof BigInt == 'function' on startup. If this returns false, then no flag is set, and all code proceeds as it does today (ie, mangling very large integers in the manner seen above). If true, the library sets some flag, USE_BIG_INT or similar, to true.
  2. Modify the parser and stringifier to check this flag, and iff the flag is true, optionally process the flag to/from BigInt if necessary.

Would this be a desirable feature for this library? If so, I am happy to do the implementation.

@jordanbtucker
Copy link
Member

jordanbtucker commented Oct 4, 2022

Thank you for the suggestion. I think this could be a useful feature, although I would prefer it be an opt-in feature rather than something automatically detected. When software works differently on different platforms it can cause confusion and hard to diagnose bugs. Plus it would be a breaking change if it wasn't opt-in.

Here's what I'm imagining for an API:

JSON5.parse(text, {bigint: true});

JSON5.stringify(value, {bigint: true});

For parse, if the user sets the bigint option to true, but BigInt isn't supported by the current platform, then we throw an error. If the bigint option is omitted or set to false, then integers are parsed as Numbers.

For stringify, if the user sets the bigint option to true, then BigInt values will be output, otherwise, an error will be thrown the same as JSON.stringify.

What do you think?

@jordanbtucker
Copy link
Member

Also, we need to decide if this would this parse all integers as BigInt, or just ones outside the range of Number.MIN_SAFE_INTEGER and Number.MAX_SAFE_INTEGER. Do we need a setting like {bigint: 'all'}?

@shango420

This comment was marked as off-topic.

@shango420

This comment was marked as off-topic.

@jedwards1211
Copy link

@jordanbtucker what do you think about supporting bigint literals like { foo: 5n }?

@jordanbtucker
Copy link
Member

jordanbtucker commented Oct 25, 2023

@jedwards1211 That would require a change to the JSON5 spec, and it has been discussed in json5/json5-spec#36. In addition to the reasons provided in that issue, the syntax of JSON5 has been formalized for over five years, so adding new syntax would create fragmentation in the ecosystem, essentially creating multiple versions of JSON5. It also breaks the core tenant of adhering to the syntax of ES5, which keeps the syntax relatively simple.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants
@jordanbtucker @jedwards1211 @azaslavsky @shango420 and others