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

Format specifier for BigInt #148

Open
shisama opened this issue Aug 5, 2018 · 25 comments · May be fixed by #224
Open

Format specifier for BigInt #148

shisama opened this issue Aug 5, 2018 · 25 comments · May be fixed by #224

Comments

@shisama
Copy link

shisama commented Aug 5, 2018

Chrome 68 supports BigInt and also Node.js v10 supports it with --harmony-bigint flag. But console.log doesn't support BigInt in them.

Chrome 68

> console.log("%d", 1180591620717411303424n)
NaN
> console.log("%i", 1180591620717411303424n)
NaN

Node.js v10.7.0 with --harmony-bigint

> console.log("%d", 1180591620717411303424n)
1.1805916207174113e+21
> console.log("%i", 1180591620717411303424n)
1.1805916207174113e+21

I think expected result is

> console.log("%d", 1180591620717411303424n)
1180591620717411303424n
> console.log("%i", 1180591620717411303424n)
1180591620717411303424n

However, spec shows %d or %i uses %parseInt%(element, 10) for type conversion.
What specifier is the best for BigInt? Has this already been discussing?

@silverwind
Copy link

silverwind commented Aug 9, 2018

Keep in mind that Node.js is currently violating the spec for the %d specifier where it uses the Number constructor which outputs those undesired scientific prefixes. The specced parseInt on the other hand can output series of digits for BigInt because its toString method which does not include the notion of a scientific notation.

So I think, we may not even need a spec change.

@silverwind
Copy link

Actually upon deeper investigation parseInt is also unsuitable for BigInt because it has NumberToString baked into it which represents everything above 1e21 using exponents.

So I think we'd actually need a console spec change, I see two options:

  1. Let %i and %d also handle BigInt. This would introduce different behavior depending on the type.
  2. Introduce a new %I dedicated for BigInt to retain a 1:1 mapping of type to conversion function.

@terinjokes
Copy link
Collaborator

I believe the issue isn't NumberToString (since BigInt isn't a Number), but parseInt's call to ToInt32.

%parseInt%(1180591620717411303424n, "10") <- 1.1805916207174113e21
  ToString(1180591620717411303424n) <- "1180591620717411303424" (https://tc39.github.io/proposal-bigint/#sec-tostring-applied-to-the-bigint-type)
  ToInt32("1180591620717411303424") <- 1.1805916207174113e21
   ToNumber("1180591620717411303424") <- 1.1805916207174113e21

I would prefer to define %d and %i specifiers to handle BigInt. The current description does not state int32, but instead "Element which substitutes is converted to an integer" which is vague enough.

On the other hand, BigInt.ToNumber() is defined to throw an error, and I'm unsure if using the same formatters will cause confusion with developers.

@domenic
Copy link
Member

domenic commented Aug 9, 2018

My two cents: agreed handling BigInt is good. Probably https://console.spec.whatwg.org/#formatter step 3.2 should add another special-case for BigInt, like it currently does for Symbol. Also the third column of https://console.spec.whatwg.org/#formatting-specifiers should be updated I guess, not sure how though.

@silverwind
Copy link

silverwind commented Aug 9, 2018

Also the third column of https://console.spec.whatwg.org/#formatting-specifiers should be updated

Maybe add a additional column before it for the source type. This could also include the Symbol special case.

@fmartin5
Copy link
Contributor

fmartin5 commented Aug 9, 2018

Options (1) and (2) do not look mutually exclusive: the %d and %i specifiers could handle BigInt values by returning NaN just like they do for Symbol values, while the proposed %I specifier could still produce specific results for BigInt formatting.

@shisama
Copy link
Author

shisama commented Aug 13, 2018

If new specifier is necessary for BigInt, the proposed %I looks good.

@paulirish
Copy link

paulirish commented Aug 13, 2018

In Chrome, we just allowed console.log("%d") to support BigInts:
https://chromium-review.googlesource.com/c/chromium/src/+/1169777 cc @ak239

If this group decides we need different behavior, we can align.

@terinjokes
Copy link
Collaborator

terinjokes commented Aug 13, 2018

I don't mind expanding %i and %d to support BigInt, and updating our table to include it.

Paul, does Chrome have interest in also supporting %i?

Do we have input from Firefox, Safari, or Edge?

@alexkozy
Copy link

In Chrome we support both and both of them support BigInt.

@terinjokes
Copy link
Collaborator

@ak239 thanks for confirming. Gerrit wasn't loading well for me on Muni.

@shisama
Copy link
Author

shisama commented Aug 14, 2018

@paulirish @ak239 Thanks. Now I confirmed console.log("%d", 1n); and console.log("%i", 1n); print 1n on console in Chrome Canary(version 70.0.3521.0).

@silverwind
Copy link

The Chrome implementation appends the n suffix to the formatted number but I think it would be more generally useful to hide such implementation details and output just the number.

@shisama
Copy link
Author

shisama commented Oct 18, 2018

Thank you all for your comments.
Chrome 70 is now stable.
In my local Chrome, Both console.log("%d",1180591620717411303424n) and console.log("%i", 1180591620717411303424n) output 1180591620717411303424n.

Do you think it should be added to the specification that %d and %i support BigInt and that if it is BigInt 'n' suffix is given?
Please comment if you have any opinions.

@domfarolino
Copy link
Member

Sorry for the delay on getting to this. Plan on carving out time for this soon if nobody gets to it first.

@tuchida
Copy link

tuchida commented Mar 1, 2022

What is the correct display when passing BigInt to %f?

Chrome/96.0.4664.110

console.log('%f', 10n) // NaN
parseFloat(10n, 10) // 10
parseFloat(10n) // 10

Firefox/97.0

console.log('%f', 10n) // undefined
parseFloat(10n, 10) // 10
parseFloat(10n) // 10

@shisama
Copy link
Author

shisama commented Jun 11, 2023

@domfarolino
Sorry for the delay on the response.
I recently implemented that %d and %i work for BigInt.

So all major browsers support %d and %i for BigInt.

Chrome 114.0.5735.106
Screenshot 2023-06-11 at 23 04 03

Firefox 114.0.1
Screenshot 2023-06-11 at 23 05 12

Safari 16.2
Screenshot 2023-06-11 at 23 05 40

However, each browser differently work for %f now.

Chrome 114.0.5735.106
Screenshot 2023-06-11 at 23 07 20

Firefox 114.0.1
Screenshot 2023-06-11 at 23 07 53

Safari 16.2
Screenshot 2023-06-11 at 23 07 32

I think that %d and %i should be defined as standards to support the format specifier of BigInt. But I’m still not sure that %f should be defined for that. Should we first start adding only %d and %i onto the spec? Or should we wait until we decide if %f should be supported?

@domfarolino
Copy link
Member

Thanks for following-up on this and reminding be about this thread. I'm trying to figure out what action needs to be taken based on the desired result here. Years ago Chrome would output NaN for BigInts with the %d format specifier as the OP points, out which is obviously undesirable. Since then, parseInt() has been given support for BigInt, and Chrome follows the spec by outputting exactly what parseInt()/parseFloat() would output on BigInts for the %d/%f specifiers respectively. So if we're OK with this behavior & the spec, then no action needs to be taken — we get this result for free.

However, Firefox and Safari seem to have prettier outputs by actually listing all the integers without scientific notation (and Safari even puts commas in the right places etc.). If we want this special pretty output for BigInts, then I think we need to add the special case for BigInt to the %d and %i specifiers in Formatter as Domenic suggested earlier. I'm fine with this; it seems like a good outcome and mostly matches what two browsers are doing today. I imagine the special case would just consist of a call to BigInt::toString(), which just returns the concatenation of all digits in the BigInt, without the complicated scientific notation stuff that Number::toString() has.

Does this sound right to everyone, and does anyone have strong opinions on just delegating to parseInt() etc., vs creating special pretty output without scientific notation? I'm just trying to catch up on this thread and make sure we're all on the same page, since it should be pretty easy to resolve this issue.

@terinjokes
Copy link
Collaborator

As an alternative , we could specify %i and %d print the result of converting a BigInt to Number and formatting like normal (it sounds like Chrome might be doing this already). We could add %g to format the BigInt with full precision, and %e that you specifically want the scientific notation?

@domfarolino
Copy link
Member

As an alternative , we could specify %i and %d print the result of converting a BigInt to Number and formatting like normal (it sounds like Chrome might be doing this already).

Doesn't the spec already do this? Since we don't special-case anything now, the spec will just call %parseInt()% on BigInts for the %d and %i specifiers, which I think is the same result as calling parseInt(bigIntFoo) (Chrome appears to do this).

So I guess I'm trying to figure out if we agree that the current spec just passes BigInts to %parseInt()%, and then after that we can figure out if we want to go a step further by spec'ing the special-casing that Firefox and Safari appear to do.

@terinjokes
Copy link
Collaborator

terinjokes commented Jul 11, 2023

I think the behavior of going through %parseInt()% as Chrome does is the correct behavior. Instead of changing how %d and %i work to special case the Firefox and Safari behavior, the alternative is to add a new specifier for "Big" types.

@domfarolino
Copy link
Member

Right. I'm personally a fan of less specifiers and instead special-casing BigInt support for %d and %i, since those are just the canonical integer specifiers, and big integers are indeed integers, just... big.

@terinjokes
Copy link
Collaborator

sprintf already has formatters for larger precision, and adding them wouldn't change the meaning of the existing formatters (and potentially break anyone expecting the current Chrome format).

Also, what's the status of BigDecimal? If it were to be added, would we have to add another special case?

@domfarolino
Copy link
Member

BigDecimal is stage 1 apparently: https://github.com/tc39/proposal-decimal#ecma-tc39-javascript-decimal-proposal. I guess I was just assuming we could squeeze it into the %f specifier should the time come. It is true that other languages provide additional syntax to specify precision, but it seems that's usually provided in addition to a specifier like %d, %i, or %f, as opposed to a new specifiers altogether.

@terinjokes
Copy link
Collaborator

terinjokes commented Jul 11, 2023

but it seems that's usually provided in addition to a specifier like %d, %i, or %f, as opposed to a new specifiers altogether.

And you could continue using those, you'd just get the behavior of %parseInt()% and %parseFloat()% as requested. I can be convinced otherwise, but I'd hate to change the existing meanings, which are currently very clear of what will happen.

@domfarolino domfarolino linked a pull request Jul 12, 2023 that will close this issue
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

9 participants