Skip to content

Commit 2ce2e23

Browse files
ptomato12wrigja
authored andcommitted
Fix inconsistency in order of observable operations
Previously, the monthDayFromFields() method of non-ISO calendars behaved differently from dateFromFields() and yearMonthFromFields(), as well as monthDayFromFields() of the ISO calendar. The latter checked the overflow option before accessing the fields, and the former checked the overflow option afterwards. This normative change fixes this inconsistency. The general principle is that arguments to a function should be validated in order, so we go with accessing the fields before checking the overflow option. Requires an update to the reference code, which previously consistently checked the overflow option first. UPSTREAM_COMMIT=a3a82377045895940f1278b19edfc9ae2e62e136
1 parent b63dd3a commit 2ce2e23

File tree

2 files changed

+6
-12
lines changed

2 files changed

+6
-12
lines changed

lib/calendar.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -353,24 +353,24 @@ DefineIntrinsic('Temporal.Calendar.from', Calendar.from);
353353
*/
354354
impl['iso8601'] = {
355355
dateFromFields(fieldsParam, options, calendar) {
356-
const overflow = ES.ToTemporalOverflow(options);
357356
let fields = ES.PrepareTemporalFields(fieldsParam, ['day', 'month', 'monthCode', 'year'], ['year', 'day']);
357+
const overflow = ES.ToTemporalOverflow(options);
358358
fields = resolveNonLunisolarMonth(fields);
359359
let { year, month, day } = fields;
360360
({ year, month, day } = ES.RegulateISODate(year, month, day, overflow));
361361
return ES.CreateTemporalDate(year, month, day, calendar);
362362
},
363363
yearMonthFromFields(fieldsParam, options, calendar) {
364-
const overflow = ES.ToTemporalOverflow(options);
365364
let fields = ES.PrepareTemporalFields(fieldsParam, ['month', 'monthCode', 'year'], ['year']);
365+
const overflow = ES.ToTemporalOverflow(options);
366366
fields = resolveNonLunisolarMonth(fields);
367367
let { year, month } = fields;
368368
({ year, month } = ES.RegulateISOYearMonth(year, month, overflow));
369369
return ES.CreateTemporalYearMonth(year, month, calendar, /* referenceISODay = */ 1);
370370
},
371371
monthDayFromFields(fieldsParam, options, calendar) {
372-
const overflow = ES.ToTemporalOverflow(options);
373372
let fields = ES.PrepareTemporalFields(fieldsParam, ['day', 'month', 'monthCode', 'year'], ['day']);
373+
const overflow = ES.ToTemporalOverflow(options);
374374
if (fields.month !== undefined && fields.year === undefined && fields.monthCode === undefined) {
375375
throw new TypeError('either year or monthCode required with month');
376376
}
@@ -2287,11 +2287,11 @@ class NonIsoCalendar implements CalendarImpl {
22872287
options: NonNullable<Params['dateFromFields'][1]>,
22882288
calendar: Temporal.Calendar
22892289
): Temporal.PlainDate {
2290-
const overflow = ES.ToTemporalOverflow(options);
22912290
const cache = new OneObjectCache();
22922291
const fieldNames = this.fields(['day', 'month', 'monthCode', 'year']) as readonly AnyTemporalKey[];
22932292
ArrayPrototypeSort.call(fieldNames);
22942293
const fields = ES.PrepareTemporalFields(fieldsParam, fieldNames, []);
2294+
const overflow = ES.ToTemporalOverflow(options);
22952295
const { year, month, day } = this.helper.calendarToIsoDate(fields, overflow, cache);
22962296
const result = ES.CreateTemporalDate(year, month, day, calendar);
22972297
cache.setObject(result);
@@ -2302,11 +2302,11 @@ class NonIsoCalendar implements CalendarImpl {
23022302
options: NonNullable<Params['yearMonthFromFields'][1]>,
23032303
calendar: Temporal.Calendar
23042304
): Temporal.PlainYearMonth {
2305-
const overflow = ES.ToTemporalOverflow(options);
23062305
const cache = new OneObjectCache();
23072306
const fieldNames = this.fields(['month', 'monthCode', 'year']) as readonly AnyTemporalKey[];
23082307
ArrayPrototypeSort.call(fieldNames);
23092308
const fields = ES.PrepareTemporalFields(fieldsParam, fieldNames, []);
2309+
const overflow = ES.ToTemporalOverflow(options);
23102310
const { year, month, day } = this.helper.calendarToIsoDate({ ...fields, day: 1 }, overflow, cache);
23112311
const result = ES.CreateTemporalYearMonth(year, month, calendar, /* referenceISODay = */ day);
23122312
cache.setObject(result);
@@ -2317,13 +2317,13 @@ class NonIsoCalendar implements CalendarImpl {
23172317
options: NonNullable<Params['monthDayFromFields'][1]>,
23182318
calendar: Temporal.Calendar
23192319
): Temporal.PlainMonthDay {
2320-
const overflow = ES.ToTemporalOverflow(options);
23212320
const cache = new OneObjectCache();
23222321
// For lunisolar calendars, either `monthCode` or `year` must be provided
23232322
// because `month` is ambiguous without a year or a code.
23242323
const fieldNames = this.fields(['day', 'month', 'monthCode', 'year']) as readonly AnyTemporalKey[];
23252324
ArrayPrototypeSort.call(fieldNames);
23262325
const fields = ES.PrepareTemporalFields(fieldsParam, fieldNames, []);
2326+
const overflow = ES.ToTemporalOverflow(options);
23272327
const { year, month, day } = this.helper.monthDayFromFields(fields, overflow, cache);
23282328
// `year` is a reference year where this month/day exists in this calendar
23292329
const result = ES.CreateTemporalMonthDay(month, day, calendar, /* referenceISOYear = */ year);

test/expected-failures-todo-migrated-code.txt

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ built-ins/Temporal/Calendar/prototype/dateAdd/argument-string-time-zone-annotati
1313
built-ins/Temporal/Calendar/prototype/dateAdd/argument-string-unknown-annotation.js
1414
built-ins/Temporal/Calendar/prototype/dateAdd/argument-zoneddatetime-timezone-getoffsetnanosecondsfor-out-of-range.js
1515
built-ins/Temporal/Calendar/prototype/dateAdd/order-of-operations.js
16-
built-ins/Temporal/Calendar/prototype/dateFromFields/order-of-operations.js
1716
built-ins/Temporal/Calendar/prototype/dateUntil/argument-calendar-fields-undefined.js
1817
built-ins/Temporal/Calendar/prototype/dateUntil/argument-propertybag-calendar-instance-does-not-get-calendar-property.js
1918
built-ins/Temporal/Calendar/prototype/dateUntil/argument-propertybag-calendar-wrong-type.js
@@ -98,7 +97,6 @@ built-ins/Temporal/Calendar/prototype/monthCode/argument-string-date-with-utc-of
9897
built-ins/Temporal/Calendar/prototype/monthCode/argument-string-time-zone-annotation.js
9998
built-ins/Temporal/Calendar/prototype/monthCode/argument-string-unknown-annotation.js
10099
built-ins/Temporal/Calendar/prototype/monthCode/argument-zoneddatetime-timezone-getoffsetnanosecondsfor-out-of-range.js
101-
built-ins/Temporal/Calendar/prototype/monthDayFromFields/order-of-operations.js
102100
built-ins/Temporal/Calendar/prototype/monthsInYear/argument-calendar-fields-undefined.js
103101
built-ins/Temporal/Calendar/prototype/monthsInYear/argument-propertybag-calendar-case-insensitive.js
104102
built-ins/Temporal/Calendar/prototype/monthsInYear/argument-propertybag-calendar-instance-does-not-get-calendar-property.js
@@ -123,7 +121,6 @@ built-ins/Temporal/Calendar/prototype/year/argument-string-date-with-utc-offset.
123121
built-ins/Temporal/Calendar/prototype/year/argument-string-time-zone-annotation.js
124122
built-ins/Temporal/Calendar/prototype/year/argument-string-unknown-annotation.js
125123
built-ins/Temporal/Calendar/prototype/year/argument-zoneddatetime-timezone-getoffsetnanosecondsfor-out-of-range.js
126-
built-ins/Temporal/Calendar/prototype/yearMonthFromFields/order-of-operations.js
127124
built-ins/Temporal/Calendar/prototype/yearOfWeek/argument-calendar-datefromfields-called-with-null-prototype-fields.js
128125
built-ins/Temporal/Calendar/prototype/yearOfWeek/argument-calendar-fields-undefined.js
129126
built-ins/Temporal/Calendar/prototype/yearOfWeek/argument-leap-second.js
@@ -870,7 +867,6 @@ built-ins/Temporal/ZonedDateTime/prototype/yearOfWeek/validate-calendar-value.js
870867
built-ins/Temporal/ZonedDateTime/timezone-instance-does-not-get-timeZone-property.js
871868
intl402/Temporal/Calendar/calendar-case-insensitive.js
872869
intl402/Temporal/Calendar/from/calendar-case-insensitive.js
873-
intl402/Temporal/Calendar/prototype/dateFromFields/order-of-operations.js
874870
intl402/Temporal/Calendar/prototype/era/argument-calendar-fields-undefined.js
875871
intl402/Temporal/Calendar/prototype/era/argument-propertybag-calendar-case-insensitive.js
876872
intl402/Temporal/Calendar/prototype/era/argument-propertybag-calendar-instance-does-not-get-calendar-property.js
@@ -887,8 +883,6 @@ intl402/Temporal/Calendar/prototype/eraYear/argument-string-date-with-utc-offset
887883
intl402/Temporal/Calendar/prototype/eraYear/argument-string-time-zone-annotation.js
888884
intl402/Temporal/Calendar/prototype/eraYear/argument-string-unknown-annotation.js
889885
intl402/Temporal/Calendar/prototype/eraYear/argument-zoneddatetime-timezone-getoffsetnanosecondsfor-out-of-range.js
890-
intl402/Temporal/Calendar/prototype/monthDayFromFields/order-of-operations.js
891-
intl402/Temporal/Calendar/prototype/yearMonthFromFields/order-of-operations.js
892886
intl402/Temporal/Calendar/prototype/yearOfWeek/infinity-throws-rangeerror.js
893887
intl402/Temporal/TimeZone/prototype/getNextTransition/transition-at-instant-boundaries.js
894888
intl402/Temporal/TimeZone/prototype/getPreviousTransition/transition-at-instant-boundaries.js

0 commit comments

Comments
 (0)