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

The calendar inside Intl.DateTimeFormat and the calendar inside Temporal objects #2521

Closed
FrankYFTang opened this issue Mar 18, 2023 · 21 comments

Comments

@FrankYFTang
Copy link
Contributor

FrankYFTang commented Mar 18, 2023

Intl.DateTimeForamt has a calendar
The calendar in the Intl.DateTimeFormat could be constructed by

  1. an explicit calendar string in the option bag. For example by new Intl.DateTimeFormat("ja", {calendar: "japanese")) => "japanese"
  2. a calendar in the -u-ca- of the locale. For example by new Intl.DateTimeFormat("zh-u-ca-roc") => "roc"
  3. the default calendar of the locale, for example, the default calendar of "fa" is "persian", and the default calendar of "en" is "gregory" (not "iso8601"), the default calendar of "th" is 'buddhist', the default calendar of "ar-SA" locale is 'islamic-umalqura'
    For example by new Intl.DateTimeFormat("fa") => "persian"
  4. the default calendar of the default locale , if the users' default locale is "th" then
    new Intl.DateTimeFormat() => "buddhist"

Many Temporal objects also has a calendar

  1. it could be "iso8601" by default
  2. It could be a value pass by calendar value in the option bag
  3. It could come from the string "2022-04-14[u-ca=roc]"

So...

  1. in Temporal.*.prototype.toLocaleString() which calendar should be used? should it throw RangeError if mismatch?
  2. in Intl.DateTimeFormat.prototype.format, which calendar should be used? should it throw RangeError if mismatch?

@sffc @ryzokuken @justingrant @ptomato @pdunkel

Looks like the timeZone case scale up to more Temporal types..... isn't it?

notice while no locales passed into the Intl.DateTimeFormat constructor or toLocaleString, it eventually resolved to DefaultLocale() and the default calendar is the default calendar of that default locale.

Notice no default calendar of any locale is "iso8601" so most likey the calendar in Temporal objects will be different from the one in Intl.DateTimeFormat.
Also the old Date object Intl.DateTimeFormat has no calendar nor timeZone in it but now most of the Temporal object has a calendar in it.

@FrankYFTang
Copy link
Contributor Author

and please understand 'greogry" calendar is very different from "iso8601" calendar in Temporal

  1. 'greogry' calendar will have era and eraYar property but 'iso8601' calendar do not.
  2. 'gregory' calendar will go to different code path than the special case 'iso8601' calendar code path inside Temporal AO.
  3. default calendar of most locale (but not all) are 'groegry' but NO default calendar of any locale is "iso8601".

@sffc
Copy link
Collaborator

sffc commented Mar 18, 2023

The champions discussed this issue at length in the past and arrived at the current proposal which is based on the idea that since no locale has iso8601 as its default calendar, then we eliminate a whole class of data-driven exceptions, which is what we were trying so hard to avoid in time zones.

That said, I'm open to setting the calendar in toLocaleString to avoid awkward issues involving T.PMD and T.PYM.

@justingrant
Copy link
Collaborator

That said, I'm open to setting the calendar in toLocaleString to avoid awkward issues involving T.PMD and T.PYM.

@sffc could you explain this a bit more? What are the issues? And would fixing them require a normative change, or just editorial?

@FrankYFTang Summarizing the behavior that was added in #952:

  • If the object's calendar is iso8601, then use the calendar from the locale and/or from the options bag.
  • If the object's calendar is not iso8601, then the object's calendar must match the resolved calendar from the locale and/or the options bag. If not, throw a RangeError.

@FrankYFTang
Copy link
Contributor Author

Let me illustrate the problem as a code below

let pym = new Temporal.PlainYearMonth(2022, 3);
let str = pym.toLocaleString();

Now if you are running the code in an environment which does not have ECMA402, str will be a string
if you run the same code on an environment which support ECMA402, it will always throw RangeError by step 4a of HandleDateTimeTemporalYearMonth
same for the following code

let pmd = new Temporal.PlainMonthDay(3, 18);
let str = pmd.toLocaleString();

@justingrant
Copy link
Collaborator

Whoops, I forgot the PYM/PMD special case in the summary above. Here's the actual behavior from #952.

  • If the object's calendar is iso8601 AND if the object is a PD, PDT, or ZDT (i.e. not a PYM/PMD), then use the resolved calendar from the locale and/or from the options bag.
  • Otherwise, the object's calendar must match the resolved calendar from the locale and/or the options bag. If not, throw a RangeError.

Now if you are running the code in an environment which does not have ECMA402, str will be a string
if you run the same code on an environment which support ECMA402, it will always throw RangeError by step 4a of HandleDateTimeTemporalYearMonth

I assume that this is because in a non-402 environment, the resolved calendar is always 'iso8601' so the calendars match and it shouldn't throw. @sffc do you think this behavior is a problem?

That said, I'm open to setting the calendar in toLocaleString to avoid awkward issues involving T.PMD and T.PYM.

@sffc could you explain what you mean by "setting the calendar in toLocaleString" ?

Looks like the timeZone case scale up to more Temporal types..... isn't it?

Echoing Shane above, there's an important difference between usage of calendars and usage of time zones in Temporal. A calendar of iso8601 is not a sign of developer intent because iso8601 is the default calendar applied to Temporal objects, and (as you noted above) iso8601 isn't the default calendar for any locale.

This means that when a formatting method sees a PlainDate/PlainDateTime/ZonedDateTime with a calendar of iso8601, we can safely ignore the Temporal object's calendar (because it's almost certainly not a sign of developer intent) and instead use the calendar from the locale or options bag used to create the %DateTimeFormat%.

Sadly we can't use this ergonomic trick for PlainYearMonth/PlainMonthDay because it's not possible to convert those types from iso8601 to any other calendar. But the ergonomic benefit for PD/PDT/ZDT formatting is so large that it was judged by the champions to outweigh the cross-type inconsistency.

For time zones however, the presence of a time zone in formatting method's input is usually a clear sign of developer intent to use that time zone:

  • Unlike calendars were the vast majority of software is designed to work with one calendar only, software use cases that deal with multiple time zones are quite common.
  • Unlike calendars where there's no alternative (i.e. there's no such thing as a calendar-less PlainDate), developers who don't know the time zone or don't care about the time zone can just use a timezone-less Temporal type like PlainDate or Instant.
  • Unlike calendars where the ISO calendar is applied by default when creating new Temporal instances, a time zone is always required when creating a ZDT. There's no default. Even fetching the system time zone requires a clear opt-in decision from the developer to call Temporal.Now.zonedDateTime*() or Temporal.Now.timeZoneId, which is also a clear expression of developer intent.

Summarizing: when a time zone is present in the input to a formatting method, it almost always means "I intend to use this time zone" while the presence of the iso8601 calendar in the input almost always means "I don't know or don't care about the calendar".

This difference in intent drives the difference in API behavior.

@sffc
Copy link
Collaborator

sffc commented Mar 19, 2023

@sffc could you explain what you mean by "setting the calendar in toLocaleString" ?

I mean that Temporal could set the calendar option on the DTF in toLocaleString (not actually setting the option, but passing it to an AO) like we just decided to do for TimeZone.

@justingrant
Copy link
Collaborator

How would that "avoid awkward issues involving T.PMD and T.PYM" ? Do you mean that it'd just format all Temporal objects with the object's calendar, including if the object's calendar is iso8601? Or only T.PMD and T.PYM? Sorry for so many questions, just trying to understand what you have in mind and why it's better.

@sffc
Copy link
Collaborator

sffc commented Mar 20, 2023

I mean, the following would "just work"

Temporal.PlainYearMonth.from({
  calendar: "hebrew",
  year: 5783,
  monthCode: "M01"
}).toLocaleString()

Right now it throws an error (in my browser which is en-US)

Uncaught RangeError: cannot format PlainYearMonth with calendar hebrew in locale with calendar gregory

but there's no reason it needs to throw an error because we can set the correct calendar on the inner DTF, just like with time zone and ZDT.

Note: This applies to all Temporal types, not just PYM and PMD

More generally, it would be nice if we could say "toLocaleString always works, and Intl.DateTimeFormat works with certain conditions"

@justingrant
Copy link
Collaborator

Interesting. Would you propose any behavior difference for objects with the ISO calendar when toLocaleString is called?

@FrankYFTang
Copy link
Contributor Author

FrankYFTang commented Mar 20, 2023

Looks like the timeZone case scale up to more Temporal types..... isn't it?

Echoing Shane above, there's an important difference between usage of calendars and usage of time zones in Temporal. A calendar of iso8601 is not a sign of developer intent because iso8601 is the default calendar applied to Temporal objects, and (as you noted above) iso8601 isn't the default calendar for any locale.

but you forget a very important fact, just like the timeZone of the ZDT, calendar COULD ALSO come from string which the client received from a remote agent following https://tc39.es/proposal-temporal/#sec-temporal-iso8601grammar. grammar, any of these Temporal objects could also constrcuted by calling Temporal.*.from with a string also follow the same grammar and contains a calendar field which is not iso8601. Even worst, with your latest changes to Temporal spec, that iso8601 string may has a AnnotationCriticalFlag in that string. For example "2022-03-20[!u-ca=japanese]"

in what sense that the calendar of "japanese" is not a sign of developer intent if the Intl.DateTimeFormat is constructed with other calendar, for example, the default calendar of "ja" locale "gregory", default calendar of DTF of "th" locale "buddhist" ? Your so-called " important difference between usage of calendars and usage of time zones" is only limited in some cases but the issue still exist in the general case of that data type.

So... what will be the diffrent behavior of
A "2022-03-20[!u-ca=japanese]"
and
B "2022-03-20[u-ca=japanese]"

here. In A, the string did say u-ca=japanese is critical, right?
in what sense the calendar of A and B is less important in PD, PDT ZDT then a timeZone in a string for ZDT?

@FrankYFTang
Copy link
Contributor Author

and you may also have a Temporal.PlainDateTime constructed from "2022-03-20[!u-ca=iso8601]" and "2022-03-20[u-ca=iso8601]" to indicate it is not just " the default calendar applied to Temporal objects" right?

@FrankYFTang
Copy link
Contributor Author

  • Unlike calendars where there's no alternative (i.e. there's no such thing as a calendar-less PlainDate), developers who don't know the time zone or don't care about the time zone can just use a timezone-less Temporal type like PlainDate or Instant.

If a developer who don't know the time zone or don't care about the time zone can just use a timezone-less Temporal type like PlainDate or Instant, then why a developer don't know the calendar or don't care about the calendar can not just use a calendar-less Temporal type like Instant ? Instant is both timeZone less and calendar less, right?

@FrankYFTang
Copy link
Contributor Author

FrankYFTang commented Mar 20, 2023

  • Unlike calendars where the ISO calendar is applied by default when creating new Temporal instances, a time zone is always required when creating a ZDT. There's no default.

really?
let's look at the following code

let zdt =  Temporal.Now.zonedDateTime();

there is a default timeZone for this zdt, right?

@FrankYFTang
Copy link
Contributor Author

FrankYFTang commented Mar 20, 2023

The champions discussed this issue at length in the past

Is that discussion happen before the introduction of "AnnotationCriticalFlag" in the grammar or after? From my understanding the first presentation to TC39 2022 Sept about AnnotationCriticalFlag is in the Sept 2022 TC39 and no such mention (in details) before that. https://ptomato.name/talks/tc39-2022-09/#1 I know the Temporal champinons must know about that coming before the Sept 2022 TC39 so I wonder is your "discussed this issue at length in the past" prior to the knowledge of such rather new requirement from IETF or after that ?

@justingrant
Copy link
Collaborator

in what sense that the calendar of "japanese" is not a sign of developer intent

Using any non-ISO calendar *is* a sign of developer intent. Which is why if a Temporal object's calendar differs from the DTF's calendar, then it will throw.

My point was that we have intended the iso8601 calendar in Temporal to be a default calendar that means that the developer does not know or care about the calendar and is OK with it being overridden when formatting.

So I'm not sure I understand this specific concern in this comment. Could you briefly clarify?

Is that discussion happen before the introduction of "AnnotationCriticalFlag" in the grammar or after?

If you are asking, when implementing support for the SEDATE critical flag, whether we were aware of how it would impact our previous decisions about the rest of Temporal, then yes we were aware.

The critical flag used to create a Temporal object from a string has no impact on localized formatting of that Temporal object, because the critical flag is intentionally not part of the data model of Temporal objects. It's a parsing-and-serialization-only thing that does not affect subsequent behavior of Temporal objects. If the user wants to serialize (in toString) to include the critical flag, then there's an option to do so.

When @ryzokuken, myself, and others on the SEDATE working group designed the critical flag, our intent was to only make sure that a sending application could be sure that the receiving application would reject unknown or invalid annotations. Our intent wasn't to require what the receiving application would subsequently do with a valid annotation after it was parsed. That's up to the receiving application.

In Temporal's case, our parsing behavior of time zone and calendar annotations is identical regardless of whether the critical flag is present or not. This is also intentionally allowed by the SEDATE spec, and one reason why we wrote the SEDATE spec that way: so Temporal wouldn't have to change.

  • Unlike calendars where the ISO calendar is applied by default when creating new Temporal instances, a time zone is always required when creating a ZDT. There's no default.

really? let's look at the following code

let zdt =  Temporal.Now.zonedDateTime();

there is a default timeZone for this zdt, right?

Sorry, I didn't intend that word "create" to apply to Temporal.Now, which is why the next sentence was this: "Even fetching the system time zone requires a clear opt-in decision from the developer to call Temporal.Now.zonedDateTime*() or Temporal.Now.timeZoneId, which is also a clear expression of developer intent."

What I meant by "create" is that when users directly create ZDT instances using a SEDATE string, a property bag, via or via the ZDT constructor, then a time zone is always required. There is never an implicit time zone applied, like there would be with the iso8601 calendar that's implicitly applied while using these same APIs.

I apologize if I was unclear. I'm glad we've cleared things up.

If a developer who don't know the time zone or don't care about the time zone can just use a timezone-less Temporal type like PlainDate or Instant, then why a developer don't know the calendar or don't care about the calendar can not just use a calendar-less Temporal type like Instant ? Instant is both timeZone less and calendar less, right?

There's no way to get from an Instant to a date (with or without a calendar) unless you know the time zone to project the Instant into. Also an Instant has a time while dates do not. So using Instant is not a good substitute for a calendar-less, time-less date.

Also, it was an intentional decision in Temporal to not have a calendar-less date type, because (if I remember correctly) @sffc argued successfully that it was good to avoid a bifurcated API.

and you may also have a Temporal.PlainDateTime constructed from "2022-03-20[!u-ca=iso8601]" and "2022-03-20[u-ca=iso8601]" to indicate it is not just " the default calendar applied to Temporal objects" right?

The string used to construct a Temporal object is not retained after the object has been parsed.

It's certainly possible that someone could explicitly supply the iso8601 calendar annotation to a SEDATE string, to a property bag, or a constructor argument. But this case was viewed to be so rare and unimportant that it was not worth capturing (in the data model of Temporal objects) the difference between an explicit iso8601 calendar vs. the default calendar applied.

One reason this case is so rare and irrelevant is because it's unnecessary because ISO is the default calendar in strings. Also, no real-world country uses the iso8601 calendar, which made it irrelevant for real-world cases like localization. For those reasons and more, we decided that iso8601 was convenient to use as a default.

Could we imagine a case where this might result in unintended output? Sure. But that risk was considered to be much less than the benefits of being able to use the ISO calendar as a default which has huge ergonomic benefits throughout the Temporal API.

@FrankYFTang
Copy link
Contributor Author

ok, I probably misunderstand what the AnnotationCriticalFlag is designed for. I thought it mean it is critical to keep that date in that calendar but after reading the RFC it seems it only mean it is critical that the receiving party should reject if it does not match.

so.... if I understand correctly

  1. The toLocaleString of PlainDate, PlainDateTime, ZonedDateTime will always use the calendar of the locale (either explicitly pass in locale, or the default locale by converting from the calendar in the Temporal object to that calendar.

  2. Unless you make another PR to change the spec text, the toLocaleString of PlainYearMonth and PlainMonthDay will return a string if the env does not supprt calendar other than "iso8601" and will always throw RangeError if the env does support calendar other than "iso8601"

@sffc
Copy link
Collaborator

sffc commented Mar 21, 2023

Also, it was an intentional decision in Temporal to not have a calendar-less date type, because (if I remember correctly) @sffc argued successfully that it was good to avoid a bifurcated API.

We don't need to go off memory; we wrote up a very nice and comprehensive doc to explain how we got here. "Two separate types" is Alternative 4, and it explains the reasoning for why we didn't go with it.

https://tc39.es/proposal-temporal/docs/calendar-review.html

Interesting. Would you propose any behavior difference for objects with the ISO calendar when toLocaleString is called?

Yeah, I don't want to change any of the iso8601 behavior.

@justingrant
Copy link
Collaborator

justingrant commented Mar 21, 2023

  • The toLocaleString of PlainDate, PlainDateTime, ZonedDateTime will always use the calendar of the locale (either explicitly pass in locale, or the default locale by converting from the calendar in the Temporal object to that calendar.
  • Unless you make another PR to change the spec text, the toLocaleString of PlainYearMonth and PlainMonthDay will return a string if the env does not supprt calendar other than "iso8601" and will always throw RangeError if the env does support calendar other than "iso8601"

My understanding of the expected behavior in the current spec, which defines identical calendar-related behavior for all formatting methods (toLocaleString, DTF.p.format, etc.) is:

  1. If the object is PlainDate, PlainDateTime, or ZonedDateTime, AND if the object's calendar is iso8601, then use the resolved calendar of the %DateTimeFormat%.
  2. Otherwise, if the object's calendar matches the resolved calendar of the %DateTimeFormat%, then use that calendar.
  3. Otherwise, throw a RangeError.

@sffc is that correct? The spec is kinda convoluted for formatting, so wanted your expert eye to make sure.

after reading the RFC it seems it only mean it is critical that the receiving party should reject if it does not match.

That's correct. The RFC is focused on parsing only. If the receiver understands all critical annotations and determines that the contents of those annotations are valid, then the RFC's job is done.

Yeah, I don't want to change any of the iso8601 behavior.

@sffc Given the steps 1-3 above (and assuming they are correct), then what is your suggested toLocaleString change?

@sffc
Copy link
Collaborator

sffc commented Mar 21, 2023

As @ptomato noted:

I think it should be a separate change that we discuss, and I'd prefer to have that happen in a follow-up proposal, which should be possible since it would be changing behaviour that currently throws to be permissible.

I agree with this. We can make this as a minor change in the future, since it relaxes panicky behavior, so that we can freeze the spec now.

@justingrant
Copy link
Collaborator

Agree. I was going to suggest the same. Just wanted to know exactly what change you had in mind, just to make sure that the current behavior wasn't either a serious bug or usability issue, or something that couldn't be future-compatible.

Could you share a bit more detail about what you have in mind? Even if separate proposal, it'd be good to record the high-level suggestion here.

Also, do steps 1-3 above match your understanding of the current spec and of the champions' decision on intended behavior? If yes, then it'd be nice to be able to describe your proposal in terms of how it modifies steps 1-3.

@ptomato
Copy link
Collaborator

ptomato commented Apr 11, 2023

Does js-temporal/proposal-temporal-v2#29 capture the proposed change? I'd like to move the discussion to a temporal-v2 issue, so let's continue it there.

@ptomato ptomato closed this as completed Apr 11, 2023
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

No branches or pull requests

4 participants