Incorrect/Misleading Type definitions for `toLocaleString`

The locales and options arguments are the same as in the constructor to Intl.DateTimeFormat.

For all toLocaleString() methods.

In this library, the following is given as the permitted type;

    toLocaleString(locales?: string | string[], options?: Intl.DateTimeFormatOptions): string;

// ns Intl in this poly;
export interface DateTimeFormatOptions extends Omit<globalThis.Intl.DateTimeFormatOptions, 'timeZone' | 'calendar'> {
  calendar?: string | Temporal.CalendarProtocol;
  timeZone?: string | Temporal.TimeZoneProtocol;
  // TODO: remove the props below after TS lib declarations are updated
  dayPeriod?: 'narrow' | 'short' | 'long';
  dateStyle?: 'full' | 'long' | 'medium' | 'short';
  timeStyle?: 'full' | 'long' | 'medium' | 'short';
}

Which according to MDN and lib.es5.d.ts is incorrect, as the actual constructor type is;

    new(locales?: string | string[], options?: DateTimeFormatOptions): DateTimeFormat;

// DateTimeFormatOptions according to lib.es5.d.ts;
interface DateTimeFormatOptions {
    localeMatcher?: "best fit" | "lookup" | undefined;
    weekday?: "long" | "short" | "narrow" | undefined;
    era?: "long" | "short" | "narrow" | undefined;
    year?: "numeric" | "2-digit" | undefined;
    month?: "numeric" | "2-digit" | "long" | "short" | "narrow" | undefined;
    day?: "numeric" | "2-digit" | undefined;
    hour?: "numeric" | "2-digit" | undefined;
    minute?: "numeric" | "2-digit" | undefined;
    second?: "numeric" | "2-digit" | undefined;
    timeZoneName?: "short" | "long" | "shortOffset" | "longOffset" | "shortGeneric" | "longGeneric" | undefined;
    formatMatcher?: "best fit" | "basic" | undefined;
    hour12?: boolean | undefined;
    timeZone?: string | undefined;
}

Behind the scenes this polyfill is doing quite a lot, but the key thing it is not doing is allowing all subtypes of Temporal.CalendarProtocol to function with the toLocaleString methods and as such shouldn't claim to do so via types; any non-builtin calendar will fail in one of several ways depending on which valid method of implementing a custom calendar is used.

The following code should work according to this library's given typescript definitions, but instead fails with RangeError: cannot format PlainDateTime with calendar custom in locale with calendar gregory. By replacing { calendar: * } at the end of the snippet with { calendar: "<valid builtin here>" }, the error changes from saying gregory to whichever builtin calendar you use.

import { Temporal } from "@js-temporal/polyfill";

class CustomCalendar implements Temporal.CalendarProtocol {
  constructor() {}

  public readonly id: string = "custom";

 /** 
 * @important lots of method not implemented, see here for full;
 * https://gist.github.com/willster277/e321d328e2892f739051b83fc50856c4 
 */

  toString(): string {
    return this.id;
  }

  toJSON?(): string {
    throw new Error("Method not implemented.");
  }
}

const customCalendarInstance = new CustomCalendar();

const custom = Temporal.Now.plainDate(customCalendarInstance);

// custom.toLocaleString(
//   one or more of undefined, [], or "custom" should work, but dont, 
//   {
//     calendar: one or both of "custom" or customCalendarInstance should work, but dont
//   },
// )
// all six of the six possible combinations above fail;

custom.toLocaleString([], { calendar: "custom" });
custom.toLocaleString([], { calendar: customCalendarInstance });
custom.toLocaleString("custom", { calendar: "custom" });
custom.toLocaleString("custom", { calendar: customCalendarInstance });
custom.toLocaleString(undefined, { calendar: "custom" });
custom.toLocaleString(undefined, { calendar: customCalendarInstance });

The same is true for plainTime plainDateTime etc.

The fix is to change the type signatures to be honest in the API they expose and only allow the types which are actually allowed and to disallow the ones which are not/are guaranteed to fail. If I've missed something and there is a non-hacky way to get locale strings for a custom calendar I'll be very happy.

Hi @willster277, thanks for reporting this issue. I think it may be helpful to clarify a few things about expected API behavior.

Which according to MDN and lib.es5.d.ts is incorrect, as the actual constructor type is

Other than calendar and timeZone (that I'll discuss below), could you clarify if there's anything else that's incorrect?

I ask because we only change calendar and timeZone to support passing Temporal objects instead of only strings in the current Intl.DateTimeFormatOptions type. Otherwise, the Temporal-enabled Intl.DateTimeFormatOptions type extends the current type (globalThis.Intl.DateTimeFormatOptions), per the declaration here:

export interface DateTimeFormatOptions extends Omit<globalThis.Intl.DateTimeFormatOptions, 'timeZone' | 'calendar'> {

Other than calendar and timeZone (discussed below), is anything else incorrect about this declaration?

the key thing it is not doing is allowing all subtypes of Temporal.CalendarProtocol to function with the toLocaleString methods and as such shouldn't claim to do so via types; any non-builtin calendar will fail in one of several ways depending on which valid method of implementing a custom calendar is used.

The challenge here is that some custom calendars actually will work with toLocaleString. For example, outside any software implementation, many real-world calendars like Chinese or Hebrew have additional metadata about years (like the Chinese zodiac) that is currently not surfaced by ECMAScript's internationalization implementation. A custom calendar could extend the Chinese calendar to add a zodiacYearType property but otherwise leave the calendar's behavior as-is. As long as this calendar's id property returned 'chinese', it would work fine with toLocaleString and Intl.DateTimeFormat.

Similarly, ECMAScript currently supports several different variations of the Islamic calendar, which AFAIK are identical except for how they calculate month boundaries. Someone could build a custom calendar with yet another different way to calculate month boundaries, and as long as that calendar returned 'islamic' for its ID, then AFAIK toLocaleString and Intl.DateTimeFormat should work with it.

However, if the ID is not one of the currently supported calendar IDs then toLocaleString and Intl.DateTimeFormat will fail, because there is currently no built-in extensibility for the localized string-formatting implementation that powers those types. Instead, if you want toLocaleString and Intl.DateTimeFormat to work with a calendar with a custom ID, then you'd need to monkeypatch toLocaleString and Intl.DateTimeFormat and handle formatting for all custom calendar IDs.

This is similar to how Temporal methods like from handle constructing Temporal objects from strings. If you want to be able to parse date and/or time strings that contain a custom calendar or time zone ID, you'd need to patch from and all other methods that accept strings, like add, subtract, etc.

Note that custom time zones are handled in the same way. toLocaleString and Intl.DateTimeFormat will output localized names of time zones in formats that include time zone names, but that will only work (without patching, that is) if a custom time zone's id field matches a built-in time zone's id. For example, if you want to replace the built-in time zone definitions with a custom implementation that uses a specific TZDB version, then you'd be using the same built-in IDs (only with different data) so toLocaleString and Intl.DateTimeFormat would work fine with those time zones.

During the design process for Temporal, we did discuss the possibility of extending the functionality of custom calendars and time zones to support a global registry of calendars and time zones, but this didn't make it into the initial Temporal release, largely because custom calendars and time zones are somewhat niche use cases, and global registries have interesting security and performance challenges that we couldn't figure out a good way to resolve to the TC39 committee's satisfaction.

During the design of Temporal, I don't believe that we discussed extending the TimeZone and Calendar types with additional methods to support custom toLocaleString output. If there's a lot of demand for this, it could potentially be added in a subsequent TC39 proposal. Note that it's not clear that Temporal would be where this extensibility would happen. Instead, a more appropriate extensibility point might be to enable users to define custom locales. Regardless, this is out of scope for the current iteration of Temporal.

Now that I've explained the context above around intended behavior of Temporal, my responses below may make more sense! I'll use your code to explain.

custom.toLocaleString("custom", { calendar: "custom" });
custom.toLocaleString("custom", { calendar: customCalendarInstance });

These are the easiest to understand, because Intl doesn't support extensibility of locale IDs (without patching toLocaleString and Intl.DateTimeFormat). The first parameter of toLocaleString is a locale, and "custom" isn't a built-in locale ID. ECMAScript has no idea how to format anything in a custom locale because this isn't a built-in locale and ECMAScript doesn't provide a way for users to add custom locales.

custom.toLocaleString([], { calendar: "custom" });
custom.toLocaleString([], { calendar: customCalendarInstance });
custom.toLocaleString(undefined, { calendar: "custom" });
custom.toLocaleString(undefined, { calendar: customCalendarInstance });

These all have a valid locale: the default locale for the user. But every locale has a default calendar. For example, if a user's default locale is en-US, then the default calendar is gregory. If the user's default locale is FA-ir then the default calendar is persian. And so on. But Temporal objects can also have a calendar. Who wins?

Temporal handles this potential conflict as follows:

If the Temporal object's calendar is iso8601 then the locale's calendar is used, under the assumption that if the calendar is ISO, then the user has not chosen a specific calendar.
Otherwise, if the Temporal object's calendar doesn't match the locale's calendar, then an exception is thrown.

This explains why you're seeing RangeError: cannot format PlainDateTime with calendar custom in locale with calendar gregory. It's the exception thrown when the locale's calendar doesn't match the Temporal object's calendar.

So the fundamental issue that you're bumping up against here is that localized string formatting depends on the locale, and locales cannot be extended in today's ECMAScript without monkeypatching Intl (and other APIs that, under the covers, use the same implementation, like Temporal's toLocaleString methods).

Anyway, back to your original question: what should be the type declaration of Intl.DateTimeFormatOptions['calendar'] and Intl.DateTimeFormatOptions['timeZone']? Currently they're this:

  calendar?: string | Temporal.CalendarProtocol;
  timeZone?: string | Temporal.TimeZoneProtocol;

I think you're suggesting that we should replace Temporal.CalendarProtocol with Temporal.Calendar and replace Temporal.TimeZoneProtocol with Temporal.TimeZone. But as I discussed above, there are many cases where a Temporal.CalendarProtocol and a Temporal.TimeZoneProtocol would not throw exceptions, as long as they re-used the IDs of built-in calendars or time zones, respectively.

Furthermore, even with built-in calendars and time zones, the runtime exceptions you saw can still happen whenever the calendar of the locale doesn't match the calendar of the Temporal instance. Changing the TS declarations won't prevent these runtime exceptions.

For this reason, we're probably going to keep the TS declarations using the *Protocol types.

Sorry for the long-winded explanation. Hopefully this was helpful!

cc @ptomato @sffc

js-temporal / temporal-polyfill

Incorrect/Misleading Type definitions for `toLocaleString` #207