Skip to content

Conversation

@ptomato
Copy link
Collaborator

@ptomato ptomato commented Aug 21, 2025

  • Make some of the changes mandated by the Intl Era Monthcode proposal
  • Some cleanups in lib/calendar.mjs
  • Some refactoring in lib/calendar.mjs to make the calculations work more like the spec text of the Intl Era Monthcode proposal, e.g., in terms of abstract ops such as CalendarSupportsEra and IsValidMonthCodeForCalendar (I didn't refactor everything though; this PR just contains the easy parts)

Best reviewed commit by commit.

ptomato added 13 commits August 21, 2025 15:35
Error messages are not defined in the spec text, but should not cause
observable effects.
See https://tc39.es/proposal-intl-era-monthcode/#sup-availablecalendars

These IDs correspond to calendars that in practice are not being used in
real life, and have already been removed in Firefox.
The era/monthCode proposal went through some changes in the era codes in
the past while. Update the calendar definitions to reflect these
changes.

This allows removing makeHelperGregorianFixedEpoch, as there are now no
longer any solar calendars without eras. We can also move
completeEraYear into the nonISOBaseHelper's adjustCalendarDate method,
since the only calendars without eras are now lunisolar calendars that
have to override adjustCalendarDate anyway.
This is needed for dealing with the coptic era that
proposal-intl-era-monthcode drops, but is still present in old versions
of ICU as ERA0. We give it a Symbol era code so that it can never be
matched by user code.
…c IDs

The calendar IDs 'islamic' and 'islamic-rgsa' don't actually correspond
to calendars in use and don't specify a calendar algorithm. The era-
monthCode proposal makes implementations define fallback behaviour for
them.

If the JS engine already performs the fallback behaviour, this code
should have no effect.
There is another comment not too far below this one that says basically
the same thing.
The calendar impl object doesn't have an `id` property, only the helper
object does.
This makes the reference implementation slightly more like the spec
text, which should help make sure the spec text is correct. More commits
like this to follow.
To eventually break the circular dependency between calendar.mjs and
ecmascript.mjs, we will need to stop accessing the ES object in the
latter file. Since we already import Call directly from es-abstract,
this is a trivial step down that path.
This makes the reference implementation slightly more like the spec
text, which should help make sure the spec text is correct.
This makes the reference implementation slightly more like the spec
text, which should help make sure the spec text is correct.

The derivation of monthsPerYear from the table for non-lunisolar
calendars would ideally be written differently, but this is temporary
until we can refactor the implementation to be even more like the spec.
Adding days to an ISO date does not have any calendar-specific behaviour
so extract it out of the calendar helper objects. There were also some
calls with a leftover cache parameter which was unused.
This folder has been removed from test262 and the tests moved into
intl402/DateTimeFormat. Now that we've updated test262 past that point,
we don't need this line anymore.
@codecov
Copy link

codecov bot commented Aug 21, 2025

Codecov Report

❌ Patch coverage is 93.46591% with 23 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.86%. Comparing base (9c1a2bc) to head (df82fd6).
⚠️ Report is 14 commits behind head on main.

Files with missing lines Patch % Lines
polyfill/lib/calendar.mjs 92.85% 18 Missing and 5 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3145      +/-   ##
==========================================
+ Coverage   96.82%   96.86%   +0.03%     
==========================================
  Files          22       22              
  Lines        9995    10081      +86     
  Branches     1819     1824       +5     
==========================================
+ Hits         9678     9765      +87     
+ Misses        270      267       -3     
- Partials       47       49       +2     

☔ 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.

@ptomato ptomato merged commit a48c297 into main Aug 22, 2025
10 checks passed
@ptomato ptomato deleted the calendar-stuff branch August 22, 2025 19:31
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.

2 participants