Skip to content
This repository has been archived by the owner on Oct 10, 2019. It is now read-only.

BigInt.prototype.toString() should return literal-form with 'n' suffix and constructor should accept it as well, e.g. BigInt('1234n') #160

Closed
kaizhu256 opened this issue Jul 16, 2018 · 12 comments

Comments

@kaizhu256
Copy link

from an integration-perspective, its less error-prone to revive/parse BigInt from JSON if its visually distinct from Number. the 'n' suffix can provide that distinction.

otherwise, we end up with bad design-patterns, like being unable to easily debug/distinguish BigInt from Number when inspecting JSON-text. e.g.:

{
    "aa": 1234,

    "bb": "1234", // is this a Number, String, or BigInt?
                  // (integration-headaches ensue due to uncertainty ...)

    "cc": "1234n" // easily recognized as BigInt
                  // (and gives clarity when debugging the reviver/parser).
}
@ljharb
Copy link
Member

ljharb commented Jul 16, 2018

#5

@kaizhu256
Copy link
Author

@ljharb thx for reference

@littledan
Copy link
Member

I'm not sure what you're getting at with JSON. It still seems potentially error-prone to turn certain magic strings into BigInt.

There are some libraries which handle BigInt JSON serialization, such as granola. I'd recommend using their pattern, of interpreting certain objects as BigInt, rather than certain strings.

@kaizhu256
Copy link
Author

re @littledan's comment in issue #5:

how about just additionally allowing 'n' suffix in constructor, e.g. BigInt('1234n')? it wouldn't adversely affect compatibility for what's currently-shipped, and makes it easier to implement custom parsers for JSON-stringified form '{"aa":"1234n"}' (for those of us wanting idiot-proof format that cannot be accidentally parsed by Number and parseInt).

also curious, what magic strings are problematic? do u mean base64 mistaken as bigint? as thats not really the point. the point is to make BigInt distinguishable from Number in JSON (for both humans and machines).

@littledan
Copy link
Member

@kaizhu256

how about just additionally allowing 'n' suffix in constructor, e.g. BigInt('1234n')?

I don't want to just allow any arbitrary string in just because we can think of a case where it's convenient. That would be a never-ending process--you can always think of reasons why something would be convenient. In this case, it's sort of a category error: the n suffix is something to do with JavaScript code, and the BigInt constructor takes as an argument a string which represents an integer.

If you know you have a string which represents a BigInt ending in n, you can easily convert it to a BigInt by simply slicing off that n: BigInt(myString.slice(0, -1)).

also curious, what magic strings are problematic?

By magic strings being problematic, I mean distinguishing whether "123n" should be treated as a string or a BigInt. What if your application happens to have a string with those contents floating around? It would seem pretty weird to suddenly interpret it as a BigInt.

@claudepache
Copy link

@kaizhu256

from an integration-perspective, its less error-prone to revive/parse BigInt from JSON if its visually distinct from Number. the 'n' suffix can provide that distinction.

Note that JSON parsing and stringification is unrelated to the .toString() method or the constructor. Compare Infinity.toString() with JSON.stringify(Infinity), and JSON.parse("false") with Boolean("false").

@jakobkummerow
Copy link
Collaborator

'{"aa":"1234n"}' (for those of us wanting idiot-proof format that cannot be accidentally parsed by Number and parseInt)

Note that the trailing "n" doesn't prevent such accidents:

parseInt("1234n") === 1234

@littledan
Copy link
Member

For context: Don't use parseInt or parseFloat if you want reasonable error checking! Among other things, they ignore trailing garbage, not just n.

@doodadjs
Copy link

I'll say no trailing "n" for "BigInt#toString", because that's generally used for humans. But for "BigInt#toSource", it could desired.

@ljharb
Copy link
Member

ljharb commented Jul 28, 2018

toSource isn’t in the standard.

@littledan
Copy link
Member

@kaizhu256 The rationale has been extensively discussed above. Is there anything more we should do in this issue?

@kaizhu256
Copy link
Author

thx for insight on irrelevance of 'n'. i still think its possible to have guidance for JSON-serialization before stage-4, though. closing this issue.

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

No branches or pull requests

6 participants