From d4589388e4e413eefc041eb028b4eb48b47bdcbf Mon Sep 17 00:00:00 2001 From: Justin Grant Date: Wed, 15 Dec 2021 13:48:39 -0800 Subject: [PATCH 1/4] Update documentation comment for era metadata --- polyfill/lib/calendar.mjs | 30 ++++++++++++++++-------------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/polyfill/lib/calendar.mjs b/polyfill/lib/calendar.mjs index b2b68efee2..b00d0812e0 100644 --- a/polyfill/lib/calendar.mjs +++ b/polyfill/lib/calendar.mjs @@ -1298,10 +1298,8 @@ const helperIndian = ObjectAssign({}, nonIsoHelperBase, { * eras. Note that it mutates and normalizes the original era objects, which is * OK because this is non-observable, internal-only metadata. * - * The result is an array of eras with this shape: - * ``` - * interface Era { - * // name of the era + * interface Era { + * /** name of the era * name: string; * * // alternate name of the era used in old versions of ICU data @@ -1309,25 +1307,29 @@ const helperIndian = ObjectAssign({}, nonIsoHelperBase, { * // with the oldest era being 0. * genericName: string; * - * // Signed calendar year where this era begins. Will be - * // 1 (or 0 for zero-based eras) for the anchor era assuming that `year` - * // numbering starts at the beginning of the anchor era, which is true - * // for all ICU calendars except Japanese. If an era starts mid-year - * // then a calendar month and day are included. Otherwise - * // `{ month: 1, day: 1 }` is assumed. - * anchorEpoch: { year: number } | { year: number, month: number, day: number } + * // Signed calendar year where this era begins. Will be 1 (or 0 for zero-based + * // eras) for the anchor era assuming that `year` numbering starts at the + * // beginning of the anchor era, which is true for all ICU calendars except + * // Japanese. For input, the month and day are optional. If an era starts + * // mid-year then a calendar month and day are included. + * // Otherwise `{ month: 1, day: 1 }` is assumed. + * anchorEpoch: { year: number; month: number; day: number }; * * // ISO date of the first day of this era - * isoEpoch: { year: number, month: number, day: number} + * isoEpoch: { year: number; month: number; day: number }; * * // If present, then this era counts years backwards like BC * // and this property points to the forward era. This must be * // the last (oldest) era in the array. - * reverseOf: Era; + * reverseOf?: Era; * * // If true, the era's years are 0-based. If omitted or false, * // then the era's years are 1-based. - * hasYearZero: boolean = false; + * hasYearZero?: boolean; + * + * // Override if this era is the anchor. Not normally used because + * // anchor eras are inferred. + * isAnchor?: boolean; * } * ``` * */ From efe4498219b0ba851d68601656cf9063c9f2311c Mon Sep 17 00:00:00 2001 From: Justin Grant Date: Wed, 15 Dec 2021 14:16:26 -0800 Subject: [PATCH 2/4] Call maximumMonthLength with correct args `year` might have been mutated in the first line of this method. So it's not OK to pass `calendarDate` to `maximumMonthLength`. Need to pass current year/month values instead. --- polyfill/lib/calendar.mjs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/polyfill/lib/calendar.mjs b/polyfill/lib/calendar.mjs index b00d0812e0..56f1934520 100644 --- a/polyfill/lib/calendar.mjs +++ b/polyfill/lib/calendar.mjs @@ -1146,10 +1146,10 @@ const helperHebrew = ObjectAssign({}, nonIsoHelperBase, { } else { if (overflow === 'reject') { ES.RejectToRange(month, 1, this.monthsInYear({ year })); - ES.RejectToRange(day, 1, this.maximumMonthLength(calendarDate)); + ES.RejectToRange(day, 1, this.maximumMonthLength({ year, month })); } else { month = ES.ConstrainToRange(month, 1, this.monthsInYear({ year })); - day = ES.ConstrainToRange(day, 1, this.maximumMonthLength({ ...calendarDate, month })); + day = ES.ConstrainToRange(day, 1, this.maximumMonthLength({ year, month })); } if (monthCode === undefined) { monthCode = this.getMonthCode(year, month); From 592b1000065fd6b877837a4faf3d4aabba3253fb Mon Sep 17 00:00:00 2001 From: Justin Grant Date: Wed, 15 Dec 2021 14:21:24 -0800 Subject: [PATCH 3/4] Additonal validation for hard-coded era data This adds an additional check to make sure that the (hard-coded) era definitions for each calendar aren't missing required `isoEpoch` properties. Note that this code is only run once at load time, not each time calendars are accessed, so there's no runtime impact. --- polyfill/lib/calendar.mjs | 1 + 1 file changed, 1 insertion(+) diff --git a/polyfill/lib/calendar.mjs b/polyfill/lib/calendar.mjs index 56f1934520..7d858953e2 100644 --- a/polyfill/lib/calendar.mjs +++ b/polyfill/lib/calendar.mjs @@ -1389,6 +1389,7 @@ function adjustEras(eras) { ArraySort.call(eras, (e1, e2) => { if (e1.reverseOf) return 1; if (e2.reverseOf) return -1; + if (!e1.isoEpoch || !e2.isoEpoch) throw new RangeError('Invalid era data: missing ISO epoch'); return e2.isoEpoch.year - e1.isoEpoch.year; }); From ba9661587210d375acc41a4611aa88190fed2b19 Mon Sep 17 00:00:00 2001 From: Justin Grant Date: Wed, 15 Dec 2021 15:01:25 -0800 Subject: [PATCH 4/4] Fix bad code ordering in `adjustCalendarDate` Re-order code to guarantee that `calendarDate.year` is set before calculating the number of months in that year. The old code would break for any calendar that a) used a constant `era` like `islamic` and b) had a variable number of months in each year like `hebrew`. Today no ICU calendars fit in that Venn diagram, but if any are added in the future this code will break when initializing any calendar using an eraYear/era pair in a property bag. Better to fix it now! --- polyfill/lib/calendar.mjs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/polyfill/lib/calendar.mjs b/polyfill/lib/calendar.mjs index 7d858953e2..4ac2f2d8e4 100644 --- a/polyfill/lib/calendar.mjs +++ b/polyfill/lib/calendar.mjs @@ -633,18 +633,20 @@ const nonIsoHelperBase = { adjustCalendarDate(calendarDate, cache, overflow /*, fromLegacyDate = false */) { if (this.calendarType === 'lunisolar') throw new RangeError('Override required for lunisolar calendars'); this.validateCalendarDate(calendarDate); - const largestMonth = this.monthsInYear(calendarDate, cache); - let { month, year, eraYear, monthCode } = calendarDate; - // For calendars that always use the same era, set it here so that derived // calendars won't need to implement this method simply to set the era. if (this.constantEra) { // year and eraYear always match when there's only one possible era - if (year === undefined) year = eraYear; - if (eraYear === undefined) eraYear = year; - calendarDate = { ...calendarDate, era: this.constantEra, year, eraYear }; + const { year, eraYear } = calendarDate; + calendarDate = { + ...calendarDate, + era: this.constantEra, + year: year !== undefined ? year : eraYear, + eraYear: eraYear !== undefined ? eraYear : year + }; } - + const largestMonth = this.monthsInYear(calendarDate, cache); + let { month, monthCode } = calendarDate; ({ month, monthCode } = resolveNonLunisolarMonth(calendarDate, overflow, largestMonth)); return { ...calendarDate, month, monthCode }; },