Skip to content

Commit

Permalink
Support Node 20 and work around ICU calendar bug
Browse files Browse the repository at this point in the history
Node 20 (ICU 73.1, actually) introduced a bug that broke some tests.
This PR ensures that our tests will continue to run after other users
update their CI to Node 20. See
https://bugs.chromium.org/p/chromium/issues/detail?id=1451943 for
the bug. https://bugs.chromium.org/p/chromium/issues/detail?id=1173158
is the root cause issue that was made worse in ICU 73.1.

The fix was to avoid using Intl.DateTimeFormat in the polyfill
for Gregorian-based calendars. A nice side effect of this is that
these calendars now run tests faster and no longer fail for dates
before 1582-10-15.
  • Loading branch information
justingrant committed Jun 14, 2023
1 parent 8b94fd6 commit 08d214e
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 59 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ jobs:
- uses: actions/checkout@v3
with:
persist-credentials: false
- name: use node.js v19.x
- name: use node.js v20.x
uses: actions/setup-node@v3
with:
node-version: 19.x
node-version: 20.x
- run: npm ci
- run: npm run build
- uses: JamesIves/[email protected]
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: use node.js v19.x
- name: use node.js v20.x
uses: actions/setup-node@v3
with:
node-version: 19.x
node-version: 20.x
- run: npm ci
- run: npm run lint
- run: npm run build:spec
16 changes: 8 additions & 8 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: use node.js v19.x
- name: use node.js v20.x
uses: actions/setup-node@v3
with:
node-version: 19.x
node-version: 20.x
- run: npm ci
- run: npm run test-demitasse
env:
Expand All @@ -23,10 +23,10 @@ jobs:
- uses: actions/checkout@v3
with:
submodules: true
- name: use node.js v19.x
- name: use node.js v20.x
uses: actions/setup-node@v3
with:
node-version: 19.x
node-version: 20.x
- run: npm ci
- run: npm run codecov:test262
env:
Expand All @@ -35,20 +35,20 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: use node.js v19.x
- name: use node.js v20.x
uses: actions/setup-node@v3
with:
node-version: 19.x
node-version: 20.x
- run: npm ci
- run: npm run test-cookbook
test-validstrings:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v3
- name: use node.js v19.x
- name: use node.js v20.x
uses: actions/setup-node@v3
with:
node-version: 19.x
node-version: 20.x
- run: npm ci
- run: |
cd polyfill
Expand Down
85 changes: 39 additions & 46 deletions polyfill/lib/calendar.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -544,7 +544,7 @@ const nonIsoHelperBase = {
day: 'numeric',
month: 'numeric',
year: 'numeric',
era: this.eraLength,
era: 'short',
timeZone: 'UTC'
});
}
Expand Down Expand Up @@ -763,11 +763,12 @@ const nonIsoHelperBase = {
// If the estimate is in the same year & month as the target, then we can
// calculate the result exactly and short-circuit any additional logic.
// This optimization assumes that months are continuous. It would break if
// a calendar skipped days, like the Julian->Gregorian switchover. But the
// only ICU calendars that currently skip days (japanese/roc/buddhist) is
// a calendar skipped days, like the Julian->Gregorian switchover. But
// current ICU calendars only skip days (japanese/roc/buddhist) because of
// a bug (https://bugs.chromium.org/p/chromium/issues/detail?id=1173158)
// that's currently detected by `checkIcuBugs()` which will throw. So
// this optimization should be safe for all ICU calendars.
// that's currently worked around by a custom calendarToIsoDate
// implementation in those calendars. So this optimization should be safe
// for all ICU calendars.
let testIsoEstimate = this.addDaysIso(isoEstimate, diffDays);
if (date.day > this.minimumMonthLength(date)) {
// There's a chance that the calendar date is out of range. Throw or
Expand Down Expand Up @@ -981,11 +982,11 @@ const nonIsoHelperBase = {
// Add enough days to roll over to the next month. One we're in the next
// month, we can calculate the length of the current month. NOTE: This
// algorithm assumes that months are continuous. It would break if a
// calendar skipped days, like the Julian->Gregorian switchover. But the
// only ICU calendars that currently skip days (japanese/roc/buddhist) is a
// bug (https://bugs.chromium.org/p/chromium/issues/detail?id=1173158)
// that's currently detected by `checkIcuBugs()` which will throw. So this
// code should be safe for all ICU calendars.
// calendar skipped days, like the Julian->Gregorian switchover. But current
// ICU calendars only skip days (japanese/roc/buddhist) because of a bug
// (https://bugs.chromium.org/p/chromium/issues/detail?id=1173158) that's
// currently worked around by a custom calendarToIsoDate implementation in
// those calendars. So this code should be safe for all ICU calendars.
const { day } = calendarDate;
const max = this.maximumMonthLength(calendarDate);
const min = this.minimumMonthLength(calendarDate);
Expand Down Expand Up @@ -1043,9 +1044,6 @@ const nonIsoHelperBase = {
);
return duration.days;
},
// The short era format works for all calendars except Japanese, which will
// override.
eraLength: 'short',
// All built-in calendars except Chinese/Dangi and Hebrew use an era
hasEra: true,
monthDayFromFields(fields, overflow, cache) {
Expand Down Expand Up @@ -1575,24 +1573,27 @@ const makeHelperGregorian = (id, originalEras) => {
const { anchorEra } = this;
const isoYearEstimate = year + anchorEra.isoEpoch.year - (anchorEra.hasYearZero ? 0 : 1);
return ES.RegulateISODate(isoYearEstimate, month, day, 'constrain');
},
// Several calendars based on the Gregorian calendar use Julian dates (not
// proleptic Gregorian dates) before the Julian switchover in Oct 1582. See
// https://bugs.chromium.org/p/chromium/issues/detail?id=1173158.
v8IsVulnerableToJulianBug: new Date('+001001-01-01T00:00Z')
.toLocaleDateString('en-US-u-ca-japanese', { timeZone: 'UTC' })
.startsWith('12'),
calendarIsVulnerableToJulianBug: false,
checkIcuBugs(isoDate) {
if (this.calendarIsVulnerableToJulianBug && this.v8IsVulnerableToJulianBug) {
const beforeJulianSwitch = ES.CompareISODate(isoDate.year, isoDate.month, isoDate.day, 1582, 10, 15) < 0;
if (beforeJulianSwitch) {
throw new RangeError(
`calendar '${this.id}' is broken for ISO dates before 1582-10-15` +
' (see https://bugs.chromium.org/p/chromium/issues/detail?id=1173158)'
);
}
}
}
});
};

/**
* Some calendars are identical to Gregorian except era and year. For these
* calendars, we can avoid using Intl.DateTimeFormat and just calculate the
* year, era, and eraYear. This is faster (because Intl.DateTimeFormat is slow
* and uses a huge amount of RAM), and it avoids ICU bugs like
* https://bugs.chromium.org/p/chromium/issues/detail?id=1173158.
*/
const makeHelperSameMonthDayAsGregorian = (id, originalEras) => {
const base = makeHelperGregorian(id, originalEras);
return ObjectAssign(base, {
isoToCalendarDate(isoDate) {
// Month and day are same as ISO, so bypass Intl.DateTimeFormat and
// calculate the year, era, and eraYear here.
const { year: isoYear, month, day } = isoDate;
const monthCode = buildMonthCode(month);
const year = isoYear - this.anchorEra.isoEpoch.year + 1;
return this.completeEraYear({ year, month, monthCode, day });
}
});
};
Expand Down Expand Up @@ -1648,26 +1649,22 @@ const helperEthiopic = makeHelperOrthodox('ethiopic', [

const helperRoc = ObjectAssign(
{},
makeHelperGregorian('roc', [
makeHelperSameMonthDayAsGregorian('roc', [
{ name: 'minguo', isoEpoch: { year: 1912, month: 1, day: 1 } },
{ name: 'before-roc', reverseOf: 'minguo' }
]),
{
calendarIsVulnerableToJulianBug: true
}
])
);

const helperBuddhist = ObjectAssign(
{},
makeHelperGregorian('buddhist', [{ name: 'be', hasYearZero: true, isoEpoch: { year: -543, month: 1, day: 1 } }]),
{
calendarIsVulnerableToJulianBug: true
}
makeHelperSameMonthDayAsGregorian('buddhist', [
{ name: 'be', hasYearZero: true, isoEpoch: { year: -543, month: 1, day: 1 } }
])
);

const helperGregory = ObjectAssign(
{},
makeHelperGregorian('gregory', [
makeHelperSameMonthDayAsGregorian('gregory', [
{ name: 'ce', isoEpoch: { year: 1, month: 1, day: 1 } },
{ name: 'bce', reverseOf: 'ce' }
]),
Expand Down Expand Up @@ -1716,7 +1713,7 @@ const helperJapanese = ObjectAssign(
// '1 1, 6 Meiji, 12:00:00 PM'
// > new Date('1872-12-31T12:00').toLocaleString(...args)
// '12 31, 5 Meiji, 12:00:00 PM'
makeHelperGregorian('japanese', [
makeHelperSameMonthDayAsGregorian('japanese', [
// The Japanese calendar `year` is just the ISO year, because (unlike other
// ICU calendars) there's no obvious "default era", we use the ISO year.
{ name: 'reiwa', isoEpoch: { year: 2019, month: 5, day: 1 }, anchorEpoch: { year: 2019, month: 5, day: 1 } },
Expand All @@ -1728,11 +1725,7 @@ const helperJapanese = ObjectAssign(
{ name: 'bce', reverseOf: 'ce' }
]),
{
// The last 3 Japanese eras confusingly return only one character in the
// default "short" era, so need to use the long format.
eraLength: 'long',
erasBeginMidYear: true,
calendarIsVulnerableToJulianBug: true,
reviseIntlEra(calendarDate, isoDate) {
const { era, eraYear } = calendarDate;
const { year: isoYear } = isoDate;
Expand Down
2 changes: 1 addition & 1 deletion polyfill/test262
Submodule test262 updated 88 files
+1 −1 features.txt
+2 −2 harness/temporalHelpers.js
+0 −32 test/built-ins/Array/prototype/group/array-like.js
+0 −35 test/built-ins/Array/prototype/group/callback-arg.js
+0 −24 test/built-ins/Array/prototype/group/callback-throws.js
+0 −28 test/built-ins/Array/prototype/group/emptyList.js
+0 −30 test/built-ins/Array/prototype/group/evenOdd.js
+0 −31 test/built-ins/Array/prototype/group/get-throws.js
+0 −29 test/built-ins/Array/prototype/group/invalid-callback.js
+0 −28 test/built-ins/Array/prototype/group/invalid-object.js
+0 −28 test/built-ins/Array/prototype/group/invalid-property-key.js
+0 −36 test/built-ins/Array/prototype/group/length-throw.js
+0 −26 test/built-ins/Array/prototype/group/length.js
+0 −25 test/built-ins/Array/prototype/group/name.js
+0 −27 test/built-ins/Array/prototype/group/null-prototype.js
+0 −36 test/built-ins/Array/prototype/group/sparse-array.js
+0 −57 test/built-ins/Array/prototype/group/this-arg-strict.js
+0 −60 test/built-ins/Array/prototype/group/this-arg.js
+0 −37 test/built-ins/Array/prototype/group/toPropertyKey.js
+0 −35 test/built-ins/Array/prototype/groupToMap/array-like.js
+0 −33 test/built-ins/Array/prototype/groupToMap/callback-arg.js
+0 −24 test/built-ins/Array/prototype/groupToMap/callback-throws.js
+0 −28 test/built-ins/Array/prototype/groupToMap/emptyList.js
+0 −30 test/built-ins/Array/prototype/groupToMap/evenOdd.js
+0 −31 test/built-ins/Array/prototype/groupToMap/get-throws.js
+0 −28 test/built-ins/Array/prototype/groupToMap/groupLength.js
+0 −29 test/built-ins/Array/prototype/groupToMap/invalid-callback.js
+0 −28 test/built-ins/Array/prototype/groupToMap/invalid-object.js
+0 −31 test/built-ins/Array/prototype/groupToMap/invalid-property-key.js
+0 −36 test/built-ins/Array/prototype/groupToMap/length-throw.js
+0 −26 test/built-ins/Array/prototype/groupToMap/length.js
+0 −26 test/built-ins/Array/prototype/groupToMap/map-instance.js
+0 −25 test/built-ins/Array/prototype/groupToMap/name.js
+0 −25 test/built-ins/Array/prototype/groupToMap/negativeZero.js
+0 −37 test/built-ins/Array/prototype/groupToMap/sparse-array.js
+0 −57 test/built-ins/Array/prototype/groupToMap/this-arg-strict.js
+0 −60 test/built-ins/Array/prototype/groupToMap/this-arg.js
+0 −39 test/built-ins/Array/prototype/groupToMap/toPropertyKey.js
+32 −0 test/built-ins/Map/groupBy/callback-arg.js
+25 −0 test/built-ins/Map/groupBy/callback-throws.js
+27 −0 test/built-ins/Map/groupBy/emptyList.js
+22 −0 test/built-ins/Map/groupBy/evenOdd.js
+21 −0 test/built-ins/Map/groupBy/groupLength.js
+29 −0 test/built-ins/Map/groupBy/invalid-callback.js
+34 −0 test/built-ins/Map/groupBy/invalid-iterable.js
+34 −0 test/built-ins/Map/groupBy/iterator-next-throws.js
+25 −0 test/built-ins/Map/groupBy/length.js
+26 −0 test/built-ins/Map/groupBy/map-instance.js
+25 −0 test/built-ins/Map/groupBy/name.js
+30 −0 test/built-ins/Map/groupBy/negativeZero.js
+29 −0 test/built-ins/Map/groupBy/toPropertyKey.js
+32 −0 test/built-ins/Object/groupBy/callback-arg.js
+25 −0 test/built-ins/Object/groupBy/callback-throws.js
+27 −0 test/built-ins/Object/groupBy/emptyList.js
+22 −0 test/built-ins/Object/groupBy/evenOdd.js
+8 −9 test/built-ins/Object/groupBy/groupLength.js
+29 −0 test/built-ins/Object/groupBy/invalid-callback.js
+34 −0 test/built-ins/Object/groupBy/invalid-iterable.js
+31 −0 test/built-ins/Object/groupBy/invalid-property-key.js
+34 −0 test/built-ins/Object/groupBy/iterator-next-throws.js
+25 −0 test/built-ins/Object/groupBy/length.js
+25 −0 test/built-ins/Object/groupBy/name.js
+27 −0 test/built-ins/Object/groupBy/null-prototype.js
+36 −0 test/built-ins/Object/groupBy/toPropertyKey.js
+40 −0 test/built-ins/Temporal/Duration/prototype/round/zero-day-length-1.js
+53 −0 test/built-ins/Temporal/Duration/prototype/round/zero-day-length-2.js
+40 −0 test/built-ins/Temporal/Duration/prototype/total/zero-day-length-1.js
+53 −0 test/built-ins/Temporal/Duration/prototype/total/zero-day-length-2.js
+1 −0 test/intl402/Locale/constructor-non-iana-canon.js
+2 −2 test/intl402/Locale/prototype/minimize/removing-likely-subtags-first-adds-likely-subtags.js
+3 −1 test/language/module-code/instn-iee-err-ambiguous-as.js
+3 −1 test/language/module-code/instn-iee-err-ambiguous.js
+3 −1 test/language/module-code/instn-iee-err-circular-as.js
+3 −1 test/language/module-code/instn-iee-err-circular.js
+3 −1 test/language/module-code/instn-iee-err-dflt-thru-star-as.js
+3 −1 test/language/module-code/instn-iee-err-dflt-thru-star.js
+3 −1 test/language/module-code/instn-iee-err-not-found-as.js
+3 −1 test/language/module-code/instn-iee-err-not-found.js
+3 −1 test/language/module-code/instn-named-err-ambiguous-as.js
+3 −1 test/language/module-code/instn-named-err-ambiguous.js
+3 −1 test/language/module-code/instn-named-err-dflt-thru-star-as.js
+3 −1 test/language/module-code/instn-named-err-dflt-thru-star-dflt.js
+3 −1 test/language/module-code/instn-named-err-not-found-as.js
+3 −1 test/language/module-code/instn-named-err-not-found-dflt.js
+3 −1 test/language/module-code/instn-named-err-not-found.js
+3 −1 test/language/module-code/instn-star-err-not-found.js
+21 −2 test/staging/Intl402/Temporal/old/japanese-era.js
+33 −1 test/staging/Intl402/Temporal/old/non-iso-calendars.js

0 comments on commit 08d214e

Please sign in to comment.