Skip to content

Commit

Permalink
Normative: Make ZonedDateTime.toLocaleString work without DateTimeFormat
Browse files Browse the repository at this point in the history 
See PR #2479 about which a consensus was not reached. This change allows
Temporal.ZonedDateTime.prototype.toLocaleString to work by overriding the
time zone at the time of creating an Intl.DateTimeFormat object and
formatting the corresponding Temporal.Instant, but disallows calling any
of the Intl.DateTimeFormat methods on a Temporal.ZonedDateTime.

NOTE: The reference code does not implement the spec exactly as written.
It observably modifies the options before passing them to the real
Intl.DateTimeFormat constructor. The behaviour described in the spec is
the correct behaviour.

UPSTREAM_COMMIT=b8e56c21cefbdb0ea68c3380a9d83c702461e0b9
  • Loading branch information
@ptomato @justingrant
ptomato authored and justingrant committed Apr 26, 2023
1 parent c8c41cf commit 12071fb
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 125 deletions.
116 changes: 12 additions & 104 deletions lib/intl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as ES from './ecmascript';
import { GetIntrinsic } from './intrinsicclass';
import {
GetSlot,
INSTANT,
ISO_YEAR,
ISO_MONTH,
ISO_DAY,
Expand All @@ -12,8 +11,7 @@ import {
ISO_MILLISECOND,
ISO_MICROSECOND,
ISO_NANOSECOND,
CALENDAR,
TIME_ZONE
CALENDAR
} from './slots';
import type { Temporal, Intl } from '..';
import type { DateTimeFormatParams as Params, DateTimeFormatReturn as Return } from './internaltypes';
Expand All @@ -23,11 +21,9 @@ const YM = Symbol('ym');
const MD = Symbol('md');
const TIME = Symbol('time');
const DATETIME = Symbol('datetime');
const ZONED = Symbol('zoneddatetime');
const INST = Symbol('instant');
const ORIGINAL = Symbol('original');
const TZ_RESOLVED = Symbol('timezone');
const TZ_GIVEN = Symbol('timezone-id-given');
const CAL_ID = Symbol('calendar-id');
const LOCALE = Symbol('locale');
const OPTIONS = Symbol('options');
Expand All @@ -52,14 +48,12 @@ interface CustomFormatters {
[MD]: typeof monthDayAmend | typeof globalThis.Intl.DateTimeFormat;
[TIME]: typeof timeAmend | typeof globalThis.Intl.DateTimeFormat;
[DATETIME]: typeof datetimeAmend | typeof globalThis.Intl.DateTimeFormat;
[ZONED]: typeof zonedDateTimeAmend | typeof globalThis.Intl.DateTimeFormat;
[INST]: typeof instantAmend | typeof globalThis.Intl.DateTimeFormat;
}

interface PrivateProps extends CustomFormatters {
[ORIGINAL]: globalThis.Intl.DateTimeFormat;
[TZ_RESOLVED]: string | Temporal.TimeZoneProtocol;
[TZ_GIVEN]: string | Temporal.TimeZoneProtocol | null;
[CAL_ID]: globalThis.Intl.ResolvedDateTimeFormatOptions['calendar'];
[LOCALE]: globalThis.Intl.ResolvedDateTimeFormatOptions['locale'];
[OPTIONS]: Intl.DateTimeFormatOptions;
Expand Down Expand Up @@ -135,7 +129,6 @@ function DateTimeFormatImpl(
this[OPTIONS] = options;
}

this[TZ_GIVEN] = options.timeZone ? options.timeZone : null;
this[LOCALE] = ro.locale;
this[ORIGINAL] = original;
this[TZ_RESOLVED] = ro.timeZone;
Expand All @@ -145,7 +138,6 @@ function DateTimeFormatImpl(
this[MD] = monthDayAmend;
this[TIME] = timeAmend;
this[DATETIME] = datetimeAmend;
this[ZONED] = zonedDateTimeAmend;
this[INST] = instantAmend;
return undefined; // TODO: I couldn't satisfy TS without adding this. Is there another way?
}
Expand Down Expand Up @@ -191,51 +183,15 @@ function resolvedOptions(this: DateTimeFormatImpl): Return['resolvedOptions'] {
return this[ORIGINAL].resolvedOptions();
}

function adjustFormatterTimeZone(
formatter: globalThis.Intl.DateTimeFormat,
timeZone?: string
): globalThis.Intl.DateTimeFormat {
if (!timeZone) return formatter;
const options = formatter.resolvedOptions();
if (options.timeZone === timeZone) return formatter;
// Existing Intl isn't typed to accept Temporal-specific options and the lib
// types for resolved options are less restrictive than the types for options.
// For example, `weekday` is
// `'long' | 'short' | 'narrow'` in options but `string` in resolved options.
// TODO: investigate why, and file an issue against TS if it's a bug.
if ((options as any)['dateStyle'] || (options as any)['timeStyle']) {
// Unfortunately, Safari's resolvedOptions include parameters that will
// cause errors at runtime if passed along with
// dateStyle or timeStyle options as per
// https://tc39.es/proposal-intl-datetime-style/#table-datetimeformat-components.
// This has been fixed in newer versions of Safari:
// https://bugs.webkit.org/show_bug.cgi?id=231041
delete options['weekday'];
delete options['era'];
delete options['year'];
delete options['month'];
delete options['day'];
delete options['hour'];
delete options['minute'];
delete options['second'];
delete options['timeZoneName'];
delete (options as any)['hourCycle'];
delete options['hour12'];
delete (options as any)['dayPeriod'];
}
return new IntlDateTimeFormat(options.locale, { ...(options as globalThis.Intl.DateTimeFormatOptions), timeZone });
}

// TODO: investigate why there's a rest parameter here. Does this function really need to accept extra params?
// And if so, why doesn't formatRange also accept extra params?
function format<P extends readonly unknown[]>(
this: DateTimeFormatImpl,
datetime: Params['format'][0],
...rest: P
): Return['format'] {
let { instant, formatter, timeZone } = extractOverrides(datetime, this);
let { instant, formatter } = extractOverrides(datetime, this);
if (instant && formatter) {
formatter = adjustFormatterTimeZone(formatter, timeZone);
return formatter.format(instant.epochMilliseconds);
}
// Support spreading additional args for future expansion of this Intl method
Expand All @@ -248,9 +204,8 @@ function formatToParts<P extends readonly unknown[]>(
datetime: Params['formatToParts'][0],
...rest: P
): Return['formatToParts'] {
let { instant, formatter, timeZone } = extractOverrides(datetime, this);
let { instant, formatter } = extractOverrides(datetime, this);
if (instant && formatter) {
formatter = adjustFormatterTimeZone(formatter, timeZone);
return formatter.formatToParts(instant.epochMilliseconds);
}
// Support spreading additional args for future expansion of this Intl method
Expand All @@ -266,23 +221,11 @@ function formatRange(this: DateTimeFormatImpl, a: Params['formatRange'][0], b: P
if (!sameTemporalType(a, b)) {
throw new TypeError('Intl.DateTimeFormat.formatRange accepts two values of the same type');
}
const {
instant: aa,
formatter: aformatter,
timeZone: atz
} = extractOverrides(a as unknown as TypesWithToLocaleString, this);
const {
instant: bb,
formatter: bformatter,
timeZone: btz
} = extractOverrides(b as unknown as TypesWithToLocaleString, this);
if (atz && btz && atz !== btz) {
throw new RangeError('cannot format range between different time zones');
}
const { instant: aa, formatter: aformatter } = extractOverrides(a as unknown as TypesWithToLocaleString, this);
const { instant: bb, formatter: bformatter } = extractOverrides(b as unknown as TypesWithToLocaleString, this);
if (aa && bb && aformatter && bformatter && aformatter === bformatter) {
const formatter = adjustFormatterTimeZone(aformatter, atz);
// TODO: Remove type assertion after this method lands in TS lib types
return (formatter as Intl.DateTimeFormat).formatRange(aa.epochMilliseconds, bb.epochMilliseconds);
return (aformatter as Intl.DateTimeFormat).formatRange(aa.epochMilliseconds, bb.epochMilliseconds);
}
}
// TODO: Remove type assertion after this method lands in TS lib types
Expand All @@ -298,15 +241,11 @@ function formatRangeToParts(
if (!sameTemporalType(a, b)) {
throw new TypeError('Intl.DateTimeFormat.formatRangeToParts accepts two values of the same type');
}
const { instant: aa, formatter: aformatter, timeZone: atz } = extractOverrides(a, this);
const { instant: bb, formatter: bformatter, timeZone: btz } = extractOverrides(b, this);
if (atz && btz && atz !== btz) {
throw new RangeError('cannot format range between different time zones');
}
const { instant: aa, formatter: aformatter } = extractOverrides(a, this);
const { instant: bb, formatter: bformatter } = extractOverrides(b, this);
if (aa && bb && aformatter && bformatter && aformatter === bformatter) {
const formatter = adjustFormatterTimeZone(aformatter, atz);
// TODO: Remove type assertion after this method lands in TS lib types
return (formatter as Intl.DateTimeFormat).formatRangeToParts(aa.epochMilliseconds, bb.epochMilliseconds);
return (aformatter as Intl.DateTimeFormat).formatRangeToParts(aa.epochMilliseconds, bb.epochMilliseconds);
}
}
// TODO: Remove type assertion after this method lands in TS lib types
Expand Down Expand Up @@ -432,22 +371,6 @@ function datetimeAmend(optionsParam: OptionsType<Temporal.PlainDateTime>) {
return options;
}

function zonedDateTimeAmend(optionsParam: OptionsType<Temporal.PlainTime>) {
let options = optionsParam;
if (!hasTimeOptions(options) && !hasDateOptions(options)) {
options = ObjectAssign({}, options, {
year: 'numeric',
month: 'numeric',
day: 'numeric',
hour: 'numeric',
minute: 'numeric',
second: 'numeric'
});
if (options.timeZoneName === undefined) options.timeZoneName = 'short';
}
return options;
}

function instantAmend(optionsParam: OptionsType<Temporal.Instant>) {
let options = optionsParam;
if (!hasTimeOptions(options) && !hasDateOptions(options)) {
Expand Down Expand Up @@ -619,24 +542,9 @@ function extractOverrides(temporalObj: Params['format'][0], main: DateTimeFormat
}

if (ES.IsTemporalZonedDateTime(temporalObj)) {
const calendar = ES.ToTemporalCalendarIdentifier(GetSlot(temporalObj, CALENDAR));
if (calendar !== 'iso8601' && calendar !== main[CAL_ID]) {
throw new RangeError(
`cannot format ZonedDateTime with calendar ${calendar} in locale with calendar ${main[CAL_ID]}`
);
}

const timeZone = GetSlot(temporalObj, TIME_ZONE);
const objTimeZone = ES.ToTemporalTimeZoneIdentifier(timeZone);
if (main[TZ_GIVEN] && main[TZ_GIVEN] !== objTimeZone) {
throw new RangeError(`timeZone option ${main[TZ_GIVEN]} doesn't match actual time zone ${objTimeZone}`);
}

return {
instant: GetSlot(temporalObj, INSTANT),
formatter: getPropLazy(main, ZONED),
timeZone: objTimeZone
};
throw new TypeError(
'Temporal.ZonedDateTime not supported in DateTimeFormat methods. Use toLocaleString() instead.'
);
}

if (ES.IsTemporalInstant(temporalObj)) {
Expand Down
60 changes: 58 additions & 2 deletions lib/zoneddatetime.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import type { ZonedDateTimeParams as Params, ZonedDateTimeReturn as Return } fro
import JSBI from 'jsbi';
import { BILLION, MILLION, THOUSAND, ZERO, HOUR_NANOS } from './ecmascript';

const customResolvedOptions = DateTimeFormat.prototype.resolvedOptions as Intl.DateTimeFormat['resolvedOptions'];
const ObjectCreate = Object.create;

export class ZonedDateTime implements Temporal.ZonedDateTime {
constructor(
epochNanosecondsParam: bigint | JSBI,
Expand Down Expand Up @@ -446,10 +449,63 @@ export class ZonedDateTime implements Temporal.ZonedDateTime {
}
toLocaleString(
locales: Params['toLocaleString'][0] = undefined,
options: Params['toLocaleString'][1] = undefined
optionsParam: Params['toLocaleString'][1] = undefined
): string {
if (!ES.IsTemporalZonedDateTime(this)) throw new TypeError('invalid receiver');
return new DateTimeFormat(locales, options).format(this);
const options = ES.GetOptionsObject(optionsParam);

const optionsCopy = ObjectCreate(null);
// This is not quite per specification, but this polyfill's DateTimeFormat
// already doesn't match the InitializeDateTimeFormat operation, and the
// access order might change anyway;
// see https://github.com/tc39/ecma402/issues/747
ES.CopyDataProperties(optionsCopy, options, ['timeZone']);

if (options.timeZone !== undefined) {
throw new TypeError('ZonedDateTime toLocaleString does not accept a timeZone option');
}

if (
optionsCopy.year === undefined &&
optionsCopy.month === undefined &&
optionsCopy.day === undefined &&
optionsCopy.weekday === undefined &&
optionsCopy.dateStyle === undefined &&
optionsCopy.hour === undefined &&
optionsCopy.minute === undefined &&
optionsCopy.second === undefined &&
optionsCopy.timeStyle === undefined &&
optionsCopy.dayPeriod === undefined &&
optionsCopy.timeZoneName === undefined
) {
optionsCopy.timeZoneName = 'short';
// The rest of the defaults will be filled in by formatting the Instant
}

let timeZone = ES.ToTemporalTimeZoneIdentifier(GetSlot(this, TIME_ZONE));
if (ES.IsTimeZoneOffsetString(timeZone)) {
// Note: https://github.com/tc39/ecma402/issues/683 will remove this
throw new RangeError('toLocaleString does not support offset string time zones');
}
timeZone = ES.GetCanonicalTimeZoneIdentifier(timeZone);
optionsCopy.timeZone = timeZone;

const formatter = new DateTimeFormat(locales, optionsCopy);

const localeCalendarIdentifier = ES.Call(customResolvedOptions, formatter, []).calendar;
const calendarIdentifier = ES.ToTemporalCalendarIdentifier(GetSlot(this, CALENDAR));
if (
calendarIdentifier !== 'iso8601' &&
localeCalendarIdentifier !== 'iso8601' &&
localeCalendarIdentifier !== calendarIdentifier
) {
throw new RangeError(
`cannot format ZonedDateTime with calendar ${calendarIdentifier}` +
` in locale with calendar ${localeCalendarIdentifier}`
);
}

return formatter.format(GetSlot(this, INSTANT));
}
toJSON(): Return['toJSON'] {
if (!ES.IsTemporalZonedDateTime(this)) throw new TypeError('invalid receiver');
Expand Down
20 changes: 1 addition & 19 deletions test/expected-failures-todo-migrated-code.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,4 @@ built-ins/Temporal/Calendar/prototype/mergeFields/non-string-properties.js
built-ins/Temporal/Calendar/prototype/mergeFields/order-of-operations.js
intl402/Temporal/TimeZone/prototype/getNextTransition/transition-at-instant-boundaries.js
intl402/Temporal/TimeZone/prototype/getPreviousTransition/transition-at-instant-boundaries.js
# Failures below here started failing when test262 was advanced to
# 2df6c7d29a18cd33120e791bdbe2043980a893fd (which was its HEAD as of 2022-04-22).
# They'll be reduced as we start porting the latest batch of commits from upstream.
# Failures above here, however, were failing on the previous test262
# (daefac08146f6a179aa273802878a87b6eca4fb8) and continued to fail after test262
# was updated. So there's a chance that these are real porting bugs. But none of them
# are in critical functionality and AFAICT all of them are in codepaths that will
# be heavily modified in upstream commits. If they're still failing after we catch
# up with upstream, then we'll investigate and fix them.
built-ins/Temporal/Duration/prototype/round/calendar-dateadd-called-with-options-undefined.js
built-ins/Temporal/Duration/prototype/round/calendar-dateuntil-called-with-singular-largestunit.js
built-ins/Temporal/Duration/prototype/round/largestunit-correct-rebalancing.js
built-ins/Temporal/Duration/prototype/round/timezone-getpossibleinstantsfor-iterable.js
intl402/Temporal/Duration/prototype/round/relativeto-string-datetime.js
intl402/Temporal/ZonedDateTime/prototype/toLocaleString/options-timeZone.js
intl402/DateTimeFormat/prototype/format/temporal-zoneddatetime-not-supported.js
intl402/DateTimeFormat/prototype/formatRange/temporal-zoneddatetime-not-supported.js
intl402/DateTimeFormat/prototype/formatRangeToParts/temporal-zoneddatetime-not-supported.js
intl402/DateTimeFormat/prototype/formatToParts/temporal-zoneddatetime-not-supported.js

0 comments on commit 12071fb

Please sign in to comment.