DateTimeFormat fractionalSecondDigits: conflict between MDN and spec

[justingrant]

MDN shows that 0 is a valid value for the fractionalSecondDigits. The spec (and Chrome) throws when zero is passed for this option. Here's the docs:

I assume this was thought through and there's a good reason for zero being excluded in the 402 spec. Why was the reason?

FWIW, Temporal allows zero for this option, so regardless of whether zero is allowed or not, we should probably align across Temporal and 402. AFAIK, there's already a plan to expand this option to allow for nanoseconds precision, so as part of those changes I'd vote for allowing zero because it seems like a valid choice, but if you disagree then let's discuss. There's also an 'auto' option allowed on the Temporal side.

Here's the TS declaration of this on in Temporal:

fractionalSecondDigits?: 'auto' | 0 | 1 | 2 | 3 | 4 | 5 | 6 | 7 | 8 | 9;

Here's a repro to see the error

new Intl.DateTimeFormat(
  'en-us', 
  { hour: 'numeric', minute: 'numeric', second: 'numeric', fractionalSecondDigits: 0 }
).format(new Date())
// MDN: OK
// Spec and Chrome: RangeError: fractionalSecondDigits value is out of range.

[anba]

#347 initially always set fractionalSecondDigits, but I proposed to omit it when it's not present in the resolved date-time pattern. Then there was some confusion about how to handle undefined vs. 0, but that was resolved eventually. Later on Frank proposed to remove 0 from the set of allowed options altogether, presumably because that would have avoided this confusion about undefined vs. 0 in the first place.

---

If we were to allow 0, I strongly propose to implement this by setting 0 to undefined after reading "fractionalSecondDigits" from the options object. That means to specify this as:

Let value be ? GetNumberOption(options, "fractionalSecondDigits", 0, 3, undefined).
If value is 0, set value to undefined.

[ryzokuken]

This should be a simple normative PR, right? @justingrant would you like to take a stab at this?

[sffc]

Discussion from 2021-09-09: https://github.com/tc39/ecma402/blob/master/meetings/notes-2021-09-09.md#datetimeformat-fractionalseconddigits-conflict-between-mdn-and-spec

Conclusion: Seems reasonable to move forward with a spec change. Still some open questions from Anba and SFC.

[sffc]

My question on this issue is the timing of when we add 4-9. Should we add it into the Temporal proposal, or should we make the change sooner?

[romulocintra]

@sffc Do we have a consensus on this? I see a few possibilities here:

1. Should we accept the 0 and set it to undefined , so we can be aligned with MDN?

// Anba's suggestion
1. Let value be ? GetNumberOption(options, "fractionalSecondDigits", 0, 3, undefined).
1. If value is 0, set value to undefined.

2. Should this support auto and 0 ?

3. And how about 4-9, should they be included here or in Temporal?

[ray007]

0 for "none" sounds reasonable to me.

[sffc]

@sffc Do we have a consensus on this?

We discussed it in 2021-09 it appears, with some open questions. I would say we should do the following:

1. Accept 0 to mean the same as undefined
2. Accept 4 through 9 in preparation for Temporal; in the mean time, digits in those positions are always 0
3. Do not add "auto"

[FrankYFTang]

I put in some comments into #715 but let me copy here too

I am very concerned about this PR and oppose this getting merged into ECMA402 as a ECMA402 PR. Instead, I propose these changes be merged into the Temporal proposal.

One thing I would like to point out clearly, according to TG2 meeting notes, we never discussed the details during the meeting. In both meeting, I was always direct to “ to discuss offline “

TG2 Oct meeting
https://github.com/tc39/ecma402/blob/master/meetings/notes-2022-10-06.md#normative-update-spec-to-align-with-mdn--preparation-for-temporal

TG2 Nov meeting
https://github.com/tc39/ecma402/blob/master/meetings/notes-2022-11-03.md#normative-update-spec-to-align-with-mdn--preparation-for-temporal-715

So here I am, discuss it offline with you.

We need to discuss this PR with two different issues:

1. Dealing with 0
2. Dealing with 4-9

First of all, in the current shape of this PR, we change 3 different places of in ECMA402
In InitializeDateTimeFormat This is needed for 1) and 2) above
Section “Abstract Operations for DateTimeFormat Objects” This is only needed for 2) above. (no impact to 1).
In BasicFormatMatcher This is only needed for 2) above. (no impact to 1) ).

For dealing with 0, the current shape of ECMA402 disallows 0. And from what I read in #590 there are two only reasons mentioned:
MDN states it accept 0 (which is a MDN bug which should be fixed in MDN)
“Temporal allows zero for this option, so regardless of whether zero is allowed or not, we should probably align across Temporal and 402. “ . This is a valid one, but the only changes needed for Temporal about this is in InitializeDateTimeFormat. But Temporal already have a section to modify InitializeDateTimeFormat in https://tc39.es/proposal-temporal/#sec-temporal-initializedatetimeformat I see no reason such change cannot be just amended to Temporal spec only, if that is the ONLY reason we want to accept 0. And I see no value to add 0 now into ECMA402 prior to that.

Now, let’s talk about 4-9. The change by itself, from my point of view, is not only unnecessary, but very confusing in this stage and should not be landed into ECMA402:

let x = 3;
let y = (new Intl.DateTimeFormat("en", {fractionalSecondDigits: x })).resolvedOptions().fractionalSecondDigits;
x == y 

Prior to PR 715, we got true here.

With this PR 715, it still get true. But now consider the following

x = 4
let y = (new Intl.DateTimeFormat("en", {fractionalSecondDigits: x })).resolvedOptions().fractionalSecondDigits;

without this PR, the new will simply throw RangeError because 4 is not an acceptable value

but with this PR, the new will success and y will be set to undefined.
and x == y will become false

But.... what will be the df format?
It will format without any fracitional second digits

and this is very confusing because the 4 didn't through, neither get honor in the format, it was just be treated as undefined.

now, you will say, Temporal need to support fractionalSecondDigits 0, and 4-9 so we need to change, it. ok where are that stated in Temporal spec now?

Let's see, the word "fractionalSecondDigits" currently appeared 26 times inside
https://tc39.es/proposal-temporal/

It appear twice in ToSecondsStringPrecision and that function impact the following functions:

• Temporal.PlainTime.prototype.toString
• Temporal.PlainDateTime.prototype.toString
• Temporal.ZonedDateTime.prototype.toString
• Temporal.Duration.prototype.toString
• Temporal.Instant.prototype.toString

then the other 24 times which mentioned the word "fractionalSecondDigits" are all in Chapter 15 which amending ECMA402
Now, according to the Temporal spec which passed Stage 3 (and the current shape of the spec text), the fractionalSecondDigits is only needed to support 1-3 for toLocaleString and Intl.DateTimeFormat (see step 36-b-i in https://tc39.es/proposal-temporal/#sec-temporal-initializedatetimeformat ) . That is what have already reach consense. There were no consense reached to support toLocaleString or Intl.DateTimeFormat beyond 3 to reach nanosecond precision.

If we follow the Temporal spec text which reach Stage 3 (and what is on the spec today) we would throw RangeError with the following code:

let t = Temporal.Now.plainDateTimeISO();
t.toLocaleString("en", {fractionalSecondDigits:4});
// throw RangeError
t.toLocaleString("en", {fractionalSecondDigits:9});
// throw RangeError
which is the spec we all agreed in TC39 to support so far when it get Stage 3 advacement. I never oppose the Temporal reach Stage 3 because it only require 3 fractionalSecondDigits but not beyond it. If the spec text require more than 3 I would block it when it asked to advance to Stage 3. But since the champion didn't I didn't block.

But... why I would block? The reason is simple. We will have implementation difficult to support sub millisecond precision for these toLocaleString or Intl.DateTimeFormat.prototype.format . Currently, we are depending on ICU to do the format, but ICU cannot handle nanosecond precision yet. (at least not in 72.1)

Let me illusrate the difficulty of supporting nanosecond precision in Temporal.*.prototype.toLocaleString and Intl.DateTimeFormat.prototype.format(|ToParts)

Currently, all known browsers implement it by (either C API or C++ API) using ICU.

Here is the ICU documentation:

https://unicode-org.github.io/icu-docs/apidoc/released/icu4c/classicu_1_1SimpleDateFormat.html#a3118634b6b3042729ddfe2f2dbf7ea10#:~:text=Fractional%20Second

notice the text
"
Fractional Second - truncates (like other time fields) to the count of letters when formatting. Appends zeros if more than 3 letters specified.
'

Now, with the followng patch for v8, I can implement a BUGGY attempt to support Temporal to nanosecond precision, but this is only BUGGY attempt which prove the point it cannot be done with the current ICU API:

diff --git a/src/objects/js-date-time-format.cc b/src/objects/js-date-time-format.cc
index 6e6298e274..73269f6927 100644
--- a/src/objects/js-date-time-format.cc
+++ b/src/objects/js-date-time-format.cc
@@ -485,7 +485,7 @@ Handle<String> DateTimeStyleAsString(Isolate* isolate,
 
 int FractionalSecondDigitsFromPattern(const std::string& pattern) {
   int result = 0;
-  for (size_t i = 0; i < pattern.length() && result < 3; i++) {
+  for (size_t i = 0; i < pattern.length() && result < 9; i++) {
     if (pattern[i] == 'S') {
       result++;
     }
@@ -828,7 +828,13 @@ DateTimeValueRecord TemporalInstantToRecord(Isolate* isolate,
                      BigInt::FromInt64(isolate, 1000000))
           .ToHandleChecked()
           ->AsInt64();
-  return {milliseconds, kind};
+  double fraction =
+      BigInt::Remainder(isolate, Handle<BigInt>(instant->nanoseconds(), isolate),
+                     BigInt::FromInt64(isolate, 1000000))
+          .ToHandleChecked()
+          ->AsInt64();
+  printf("fraction %f\n", fraction);
+  return {milliseconds + fraction * 1e-6, kind};
 }
 
 Maybe<DateTimeValueRecord> TemporalPlainDateTimeToRecord(
@@ -1377,6 +1383,7 @@ icu::UnicodeString CallICUFormat(const icu::SimpleDateFormat& date_format,
   // For other Temporal objects, lazy generate a SimpleDateFormat for the kind.
   std::unique_ptr<icu::SimpleDateFormat> pattern(
       GetSimpleDateTimeForTemporal(date_format, kind));
+  printf("%.9f\n", time_in_milliseconds);
   pattern->format(time_in_milliseconds, result, fp_iter, status);
   return result;
 }
@@ -2430,7 +2437,7 @@ MaybeHandle<JSDateTimeFormat> JSDateTimeFormat::New(
       MAYBE_ASSIGN_RETURN_ON_EXCEPTION_VALUE(
           isolate, fsd,
           GetNumberOption(isolate, options,
-                          factory->fractionalSecondDigits_string(), 1, 3, 0),
+                          factory->fractionalSecondDigits_string(), 0, 9, 0),
           Handle<JSDateTimeFormat>());
       // Convert fractionalSecondDigits to skeleton.
       for (int i = 0; i < fsd; i++) {

The shortcoming is in two issues not just one

The ICU SimpleDateFormat::format only support to milisecond precision (which mean 1, 2 or 3 in fractionalSecondDigits) so there are 1,000,000 different value of nanoseconds of Instant will all formated to the same string, which they should not.
The ICU SimpleDateFormat::format take the Date (which is double) as the date type to represent millisecond, and the data type does not have the precision to encode up to nanosecond precision, even if we change ICU 73 to relax the restriction on 1 above.
Let me illustrate the issue with the following call (this is run on my local machine after I apply the patch above:

ftang@ftang4:~/v8/v8$ out/x64.release/d8 --harmony-temporal
V8 version 11.0.0 (candidate)
d8> let t = Temporal.Now.plainDateTimeISO();
undefined
d8> t
2022-11-28T14:18:35.661261056
d8> t.toLocaleString("en", {fractionalSecondDigits:9});
fraction 261056.000000
1669673915661.260986328
"11/28/2022, 2:18:35.661000000 PM"

Now, the fact it show
"11/28/2022, 2:18:35.661000000 PM"
instead of
"11/28/2022, 2:18:35.661261056 PM"
show the ICU API limitation mentioned in the API doc
but more interestingly is the debugging line I printted out

fraction 261056.000000
1669673915661.260986328
This show, my code did get the bigint as
1669673915661261056 nanosecond
but while I convert it to double, by taking
1669673915661 millisecond
and 261056 nanoseconds both as int and turn into a double, the result is
1669673915661.260986328
because double use only 53 bits to encode the digits, and
log(2^53)/log(10)= 15.9545897702
which mean we double can only encode the first 15 digits precisionly. and in this case, "1669673915661" is 13 digits, so two extra digits "26" is precious enough but it won't have the encodeing power to preiously encode the "1056", therefore what the ICU see from the double is the remaining 4 digits are "0986" instead.

Therefore, even if we fix the ICU SimpleDateFormat:format to format the rest of 6 digits from the double instead of "Appends zeros if more than 3 letters specified" , it will generate incorrect result, likely for the value fractionalSecondDigits> 5
Assuming the value is not represent not too far in the future or past. For value way in the future or in the past, the incorrect result will shown for a even smaller value in fractionalSecondDigits.

In summary, if we want to change ICU to support the formatting of nanosecond precision, not only it need to change the internal implementation of SimpleDateFomrat::format, it also need to add new API to take a new data type to encode the value which has nanosecond precision, which currently does not exist in ICU.

Accepting an behavior of 1 to 6 '0' as the format result in place with the real didigt stored in the Temporal object is a very very bad one because for anyone whe care about setting the fractionalSecondDigits > 3, we have to assume the information below millisecond is important enough for them to use such setting, and truncating it to millisecond then adding 0 will misguide the reader as a different value than what it actually is and could cause diaster for any application which care about sub millisecond precision. The users would either

• they DO NOT care about sub millisecond precision so the API do not need to support it, OR
• they DO care about sub millisecond precision so the API have to format it it precisously

and supporting an API which could set it to higher precision but allow the formatted output in lower precision (incorrect value in that precision, actually) is very dangerous. and I would argue no body in the world will need such hehavior.

[HolgerJeromin]

@FrankYFTang
Thanks for your work here.
I just want to mention that we indeed develop a hmi which should be able to handle nanosecond timestamps for sorting data but also for displaying them with high precision.
Showing all timestamps with filled zeros or wrong characters for the sub millisecond part would be a problem for us.

[FrankYFTang]

Showing all timestamps with filled zeros or wrong characters for the sub millisecond part would be a problem for us.

I understand this desire. I just want to make sure the process of making that happen conduct in a manner which could be developed properly.

Here is what I see are needed

1. if the data type is Date. the precision is only millisecond and there are no sub millisecond precision now. Is that ok?

Which means if we have

let ds = (new Date()).toLocaleString("en", { fractionalSecondDigits:x});
let df = new Intl.DateTimeFormat("en", { fractionalSecondDigits:x});
let ds2 = df.format(new Date());

because the Date() only hold the now in millisecond if x > 3, it could be either:
a. Throw RangeError in toLocaleString and Intl.DateTimeFormat constructor OR
b. padd '0' in the end

notice b. has nothing to do with ICU but what is required to be stored in Date
Currently https://tc39.es/ecma262/#sec-timeclip ensure the value stored are integer of millisecond so there are no sub millisecond information.

2. In order for Temporal toLocaleString and Intl.DateTimeFormat for Temporal types, it requires two changes in ICU based implementation (which is all engines depended on)

a. the SimpleDateFormat need to take data type as input which can represent the precision down to nanoseconds. So far we do not have this feature in ICU
b. the SimpleDateFormat need to not truncate the digits after 3 digit and not generate only '0' but based on the nanosecond value instead.

These two dependency need to be first addressed in ICU project to address the "implementation difficulity" issue.

[FrankYFTang]

File issue for ICU in https://unicode-org.atlassian.net/browse/ICU-22218