Polyfill: Fix extreme PlainYearMonth crash in non-ISO calendars - #369
Open
craig-o-curtis wants to merge 2 commits into
Open
craig-o-curtis wants to merge 2 commits into
craig-o-curtis wants to merge 2 commits into
Conversation
…sion In the iterative guessing process of converting a calendar date to an ISO date, we can run afoul of the supported range when either of two things happen: - A calendar helper's estimateISODate() returns an estimate of 01-01 of whatever year, which may be -271821 - diffTotalDaysEstimate is positive and takes us past the end of the range In the places where either of these two things happen, clamp the ISO date to the supported range. It's possible there are other places that could error, but I haven't been able to find cases that would make them do so. UPSTREAM_COMMIT=a41eb67481d51cc5e0fe3f95136c978126a6c9c6
* Polyfill: Fix extreme PlainYearMonth crash in non-ISO calendars Fixes #3251. * Deduplicate legacy Date range boundary logic Replace isOutOfLegacyDateRange() with compareISODateToLegacyDateRange(), a three-way comparison returning -1/0/1. Rewrite clampISODate() to use it, removing the duplicated boundary conditions. Thanks to ptomato for the suggestion. NOTE: (this info below wasn't in the upstream commit) "Fixes #3251" above refers to tc39/proposal-temporal#3251, not to an issue in this repository. What this changes: converting a calendar date to an ISO date searches by trial ISO dates, and near the ends of the supported range some of those trial dates are outside the range of the JavaScript Date object. Intl.DateTimeFormat cannot format those, so the conversion threw "RangeError: Invalid ISO date". In the islamic calendars, the coptic, ethiopic and ethioaa calendars and the indian calendar, such a date is now moved into range by a whole number of calendar cycles (islamic: 10631 days = 30 years; coptic/ethiopic/ethioaa: 1461 days = 4 years; indian: 4 years), converted, and the calendar year moved back by the same number of years. Dates inside the range are converted as before. Still broken after this commit (unchanged, not part of the port): - hebrew and persian: until()/since() and Duration round() with relativeTo can still throw "Invalid ISO date" near the end of the range, e.g. persian +275760-07-01 until +275760-09-13 by months. - chinese and dangi: years far from the present (e.g. year 250000) throw "TypeError: Internal error. Icu error." from ICU. That is fixed in proposal-temporal by tc39/proposal-temporal#3277, which is not ported here. TypeScript port: the shifted functions call HelperBase.prototype.isoToCalendarDate where upstream calls nonIsoHelperBase.isoToCalendarDate, are installed as override class fields, and use this file's addDaysISO for ES.AddDaysToISODate. UPSTREAM_COMMIT=af0cb4b17fad3bafbe905892abda5fb91054a7ea
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ports two tc39/proposal-temporal commits, one commit each:
What was wrong
To convert a non-ISO calendar date to an ISO date,
calendarToIsoDate()guesses an ISO date, converts the guess back to the calendar withIntl.DateTimeFormat, and corrects it. Near either end of the supported range, a guess can fall outside the range of the JavaScriptDateobject.Intl.DateTimeFormatcan't format such a date, so the conversion threwRangeError: Invalid ISO date, even though the requested date itself is in range. Onmain, all of these throw:What this changes
Daterange is moved into range by whole calendar cycles (islamic 10631 days = 30 years, coptic/ethiopic/ethioaa 1461 days = 4 years, indian 4 years), converted, and the year moved back. This fixes the last three lines. It also fixes islamic-civiluntil()near both ends of the range, which threw onmain. Dates inside the range take the same code path as before.Not fixed by this PR
These behave the same on
mainand on this branch:until()/since()andDuration.prototype.round()withrelativeTocan still throwInvalid ISO date. For example, persian+275760-07-01until+275760-09-13withlargestUnit: 'months', orTemporal.Duration.from('P40D').round({ largestUnit: 'months', relativeTo: { calendar: 'hebrew', year: 279517, month: 8, day: 15 } }). proposal-temporal's polyfill has the same problem, so I'll report it there first.TypeError: Internal error. Icu error.from ICU. proposal-temporal fixed this in Polyfill: fix Chinese/Dangi calendar at extreme date ranges tc39/proposal-temporal#3277 (for Coverage gap: Incorrect behaviour at end of range for Chinese calendar tc39/proposal-temporal#3081), which isn't ported here. Also, under ICU 78 these calendars currently fail even for ordinary years (Bug: Malformed leap month suffix while implementing Chinese calendar #360); April 2026 rebase, part 3 #361 fixes that.Testing
Node 24.21.0 (ICU 78.3, CLDR 48): jest passes, and test262 at the pinned version passes with no new failures or unexpected passes (intl402/Temporal 330 passed, 8 expected failures; built-ins/Temporal 4330 passed).
The test262 tests for this fix,
intl402/Temporal/{PlainDate,PlainDateTime,PlainYearMonth,ZonedDateTime}/from/extreme-dates.jsand.../prototype/withCalendar/extreme-dates.js, are newer than our pinned test262. I ran them by hand, at the test262 commit proposal-temporal used for af0cb4b, and counted each calendar row separately:mainWith this PR, every remaining failure is one of the "Not fixed by this PR" items or is fixed by #361: indian before year 1, hebrew negative leap years, the coptic/ethiopic eras, and chinese/dangi under ICU 78. With #361 on top (it applies cleanly), only the chinese/dangi far-future and far-past rows fail. Upstream's removal of
PlainYearMonth/from/extreme-dates.jsfrom its expected failures needs no change here, because that test isn't in our pinned test262.Differences from upstream (TypeScript)
isoToCalendarDatefunctions callHelperBase.prototype.isoToCalendarDate, where upstream callsnonIsoHelperBase.isoToCalendarDate.overrideclass fields, the same wayOrthodoxBaseHelperalready installsOrthodoxOps.ES.AddDaysToISODateis this file'saddDaysISO.