Skip to content

Polyfill: Fix extreme PlainYearMonth crash in non-ISO calendars#3279

Merged
ptomato merged 2 commits intotc39:mainfrom
jessealama:fix-extreme-plainyearmonth
Feb 23, 2026
Merged

Polyfill: Fix extreme PlainYearMonth crash in non-ISO calendars#3279
ptomato merged 2 commits intotc39:mainfrom
jessealama:fix-extreme-plainyearmonth

Conversation

@jessealama
Copy link
Collaborator

@jessealama jessealama commented Feb 19, 2026

calendarToIsoDate()'s bisection loop can produce intermediate ISO dates beyond the legacy Date boundary, causing isoToCalendarDate() to fail via Intl.DateTimeFormat.formatToParts().

The fix is to override isoToCalendarDate in affected calendar helpers to (1) shift out-of-range ISO dates by a calendar-appropriate cycle, (2) convert, then (3) adjust the year back. Another wrinkle is that Coptic/Ethiopian and Islamic calendars use day-based shifting with exact cycle lengths to avoid Julian–Gregorian drift; Indian uses year-based shifting since its months start at fixed Gregorian dates; Hebrew doesn't need an override since its extreme dates stay within range during bisection, though I'm not fully sure this holds in all cases.

Fixes #3251

@jessealama jessealama requested a review from ptomato February 19, 2026 15:28
@jessealama jessealama marked this pull request as draft February 19, 2026 15:29
@jessealama jessealama marked this pull request as ready for review February 19, 2026 15:33
@codecov
Copy link

codecov bot commented Feb 19, 2026

Codecov Report

❌ Patch coverage is 93.42105% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 97.93%. Comparing base (8827087) to head (9b223e5).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
polyfill/lib/calendar.mjs 93.42% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3279      +/-   ##
==========================================
- Coverage   97.96%   97.93%   -0.04%     
==========================================
  Files          22       22              
  Lines       10519    10586      +67     
  Branches     1814     1827      +13     
==========================================
+ Hits        10305    10367      +62     
- Misses        197      201       +4     
- Partials       17       18       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@jessealama jessealama force-pushed the fix-extreme-plainyearmonth branch 2 times, most recently from 5be3fd4 to 875ce03 Compare February 19, 2026 15:59
@ptomato ptomato force-pushed the fix-extreme-plainyearmonth branch from 875ce03 to 74d2751 Compare February 19, 2026 23:23
Copy link
Collaborator

@ptomato ptomato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems reasonable. But the approximate nature of the shifts for some calendars makes me uneasy, and I find it hard to reason about.

One alternative idea that I had, would be to extend the range beyond the legacy Date range more conservatively, staying only within the same month. e.g., if new Temporal.PlainDate(-271821, 4, 19, 'islamic-civil') gives { year: -280804, month: 3, day: 21 }, then we can manually calculate the calendar date for ISO dates between -271821-03-30 and -271821-04-18.

No idea if this would be sufficient to not crash in the bisection loop; maybe we need to bisect with dates outside of that, in which case we should probably go with the solution currently in this PR.

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.
@jessealama jessealama requested a review from ptomato February 20, 2026 07:59
Copy link
Collaborator

@ptomato ptomato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd still like to try the approach I suggested, but given that we don't have a lot of time, better to merge this since it works and iterate later. Thanks again!

@ptomato ptomato merged commit af0cb4b into tc39:main Feb 23, 2026
10 checks passed
@jessealama jessealama deleted the fix-extreme-plainyearmonth branch February 24, 2026 07:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Polyfill: Exception incorrectly thrown with maximum allowed PlainYearMonth in some non-ISO calendars

2 participants