Skip to content

Commit

Permalink
Throw TypeError if no overlap between options and Temporal object
Browse files Browse the repository at this point in the history
Somewhere along the line the spec text got out of sync with the consensus,
probably in one of the rebases on newer ECMA-402. When attempting to
format a Temporal object, if the DateTimeFormat has options that do not
overlap with the data model of the Temporal object, toLocaleString() and
format() are supposed to throw a TypeError.

Previously, this worked correctly with dateStyle and timeStyle, but not
with other options. We would treat the options as if the DateTimeFormat
had been created with only default options, which was incorrect.

Closes: #2795
  • Loading branch information
ptomato committed Oct 4, 2024
1 parent 53d2b63 commit b21ed2d
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 17 deletions.
58 changes: 42 additions & 16 deletions polyfill/lib/intl.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@ import {
ObjectAssign,
ObjectCreate,
ObjectDefineProperties,
ObjectDefineProperty
ObjectDefineProperty,
ObjectKeys
} from './primordials.mjs';
import { assert } from './assert.mjs';
import * as ES from './ecmascript.mjs';
Expand Down Expand Up @@ -319,8 +320,8 @@ function amend(options = {}, amended = {}) {
return options;
}

function timeAmend(options) {
options = amend(options, {
function timeAmend(originalOptions) {
const options = amend(originalOptions, {
year: false,
month: false,
day: false,
Expand All @@ -329,7 +330,10 @@ function timeAmend(options) {
dateStyle: false
});
if (!hasTimeOptions(options)) {
options = ObjectAssign({}, options, {
if (hasAnyDateTimeOptions(originalOptions)) {
throw new TypeError(`cannot format Temporal.PlainTime with options [${ObjectKeys(originalOptions)}]`);
}
ObjectAssign(options, {
hour: 'numeric',
minute: 'numeric',
second: 'numeric'
Expand All @@ -338,7 +342,7 @@ function timeAmend(options) {
return options;
}

function yearMonthAmend(options) {
function yearMonthAmend(originalOptions) {
// Try to fake what dateStyle should do for dates without a day. This is not
// accurate for locales that always print the era
const dateStyleHacks = {
Expand All @@ -347,7 +351,7 @@ function yearMonthAmend(options) {
long: { year: 'numeric', month: 'long' },
full: { year: 'numeric', month: 'long' }
};
options = amend(options, {
const options = amend(originalOptions, {
day: false,
hour: false,
minute: false,
Expand All @@ -363,20 +367,23 @@ function yearMonthAmend(options) {
ObjectAssign(options, dateStyleHacks[style]);
}
if (!('year' in options || 'month' in options || 'era' in options)) {
options = ObjectAssign(options, { year: 'numeric', month: 'numeric' });
if (hasAnyDateTimeOptions(originalOptions)) {
throw new TypeError(`cannot format PlainYearMonth with options [${ObjectKeys(originalOptions)}]`);
}
ObjectAssign(options, { year: 'numeric', month: 'numeric' });
}
return options;
}

function monthDayAmend(options) {
function monthDayAmend(originalOptions) {
// Try to fake what dateStyle should do for dates without a day
const dateStyleHacks = {
short: { month: 'numeric', day: 'numeric' },
medium: { month: 'short', day: 'numeric' },
long: { month: 'long', day: 'numeric' },
full: { month: 'long', day: 'numeric' }
};
options = amend(options, {
const options = amend(originalOptions, {
year: false,
hour: false,
minute: false,
Expand All @@ -392,13 +399,16 @@ function monthDayAmend(options) {
ObjectAssign(options, dateStyleHacks[style]);
}
if (!('month' in options || 'day' in options)) {
options = ObjectAssign({}, options, { month: 'numeric', day: 'numeric' });
if (hasAnyDateTimeOptions(originalOptions)) {
throw new TypeError(`cannot format PlainMonthDay with options [${ObjectKeys(originalOptions)}]`);
}
ObjectAssign(options, { month: 'numeric', day: 'numeric' });
}
return options;
}

function dateAmend(options) {
options = amend(options, {
function dateAmend(originalOptions) {
const options = amend(originalOptions, {
hour: false,
minute: false,
second: false,
Expand All @@ -407,7 +417,10 @@ function dateAmend(options) {
timeStyle: false
});
if (!hasDateOptions(options)) {
options = ObjectAssign({}, options, {
if (hasAnyDateTimeOptions(originalOptions)) {
throw new TypeError(`cannot format PlainDate with options [${ObjectKeys(originalOptions)}]`);
}
ObjectAssign(options, {
year: 'numeric',
month: 'numeric',
day: 'numeric'
Expand All @@ -416,10 +429,13 @@ function dateAmend(options) {
return options;
}

function datetimeAmend(options) {
options = amend(options, { timeZoneName: false });
function datetimeAmend(originalOptions) {
const options = amend(originalOptions, { timeZoneName: false });
if (!hasTimeOptions(options) && !hasDateOptions(options)) {
options = ObjectAssign({}, options, {
if (hasAnyDateTimeOptions(originalOptions)) {
throw new TypeError(`cannot format PlainDateTime with options [${ObjectKeys(originalOptions)}]`);
}
ObjectAssign(options, {
year: 'numeric',
month: 'numeric',
day: 'numeric',
Expand Down Expand Up @@ -467,6 +483,16 @@ function hasTimeOptions(options) {
);
}

function hasAnyDateTimeOptions(originalOptions) {
return (
hasDateOptions(originalOptions) ||
hasTimeOptions(originalOptions) ||
'dateStyle' in originalOptions ||
'timeStyle' in originalOptions ||
'timeZoneName' in originalOptions
);
}

function isTemporalObject(obj) {
return (
ES.IsTemporalDate(obj) ||
Expand Down
3 changes: 2 additions & 1 deletion polyfill/lib/primordials.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ export const {
getOwnPropertyNames: ObjectGetOwnPropertyNames,
defineProperty: ObjectDefineProperty,
defineProperties: ObjectDefineProperties,
entries: ObjectEntries
entries: ObjectEntries,
keys: ObjectKeys
} = Object;

export const {
Expand Down
3 changes: 3 additions & 0 deletions spec/intl.html
Original file line number Diff line number Diff line change
Expand Up @@ -855,11 +855,14 @@ <h1>
1. If _required_ is ~time~ or ~any~, then
1. Set _formatOptions_.[[hourCycle]] to _options_.[[hourCycle]].
1. Let _needDefaults_ be *true*.
1. Let _anyPresent_ be *false*.
1. For each property name _prop_ of _requiredOptions_, do
1. Let _value_ be _options_.[[&lt;_prop_>]].
1. If _value_ is not *undefined*, then
1. Set _formatOptions_.[[&lt;_prop_>]] to _value_.
1. Set _needDefaults_ to *false*.
1. Set _anyPresent_ to *true*.
1. If _anyPresent_ is *false*, return *null*.
1. If _needDefaults_ is *true*, then
1. For each property name _prop_ of _defaultOptions_, do
1. Set _formatOptions_.[[&lt;_prop_>]] to *"numeric"*.
Expand Down

0 comments on commit b21ed2d

Please sign in to comment.