Skip to content

April 2026 rebase, part 3 - #361

Merged
ptomato merged 20 commits into
mainfrom
rebase-part3
Sep 21, 2026
Merged

ptomato merged 20 commits into
mainfrom
rebase-part3

Conversation

@ptomato

@ptomato ptomato commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

This batch of commits mainly deals with working around weirdnesses in newer ICU versions, and bugfixes to pass the test262 tests dealing with calendar calculations.

There is one normative change, from the November 2025 TC39 plenary. It deals with rounding durations in edge cases and is unlikely to break any working code.

Closes: #360

ptomato and others added 20 commits April 22, 2026 10:32
These don't have defaults, so undefined isn't the same as passing 'auto'

UPSTREAM_COMMIT=ba168d7ab6f91a9015c97708ed8741ea323d3232
This is defined in the Intl Era Monthcode proposal.

See tc39/ecma402#534 and
tc39/proposal-intl-era-monthcode#30

UPSTREAM_COMMIT=25c6205a572a188fbbd02d6bff9e9be2da743ad0
If 'era' is used as a standalone option, it should still trigger
default formatting for PlainTime and PlainMonthDay.

See tc39/proposal-temporal#3049 for more details.

UPSTREAM_COMMIT=0ecf1572df1083e319e01eda5e89b5bec3f83ae6
This was causing the overflow: "reject" to be ignored in leap month
coercion.

UPSTREAM_COMMIT=6f947a70a5332b223860c6082fa04eb6305653f5
These error messages refer to "Chinese year" because they are in the
helperChinese object, but since helperDangi inherits from that it
results in odd error messages. Use ${this.id} instead.

UPSTREAM_COMMIT=af87126b42856ee814ae2e0f99d143586219e7da
Latest versions of Node have this "Mo" prefix in their ICU data, which
completely broke the Chinese and Dangi calendars.

UPSTREAM_COMMIT=835c195b65cba8a836e8806c1243bc819bbc9c79
The check for the Indian calendar BCE bug included the literal string
"Saka", which has been changed to "Śaka" in the latest ICU data. Ignore
the formatted era name and just check the formatted date to see if the
host is vulnerable to the bug.

UPSTREAM_COMMIT=314b112888f679156142ac15135131959ec32ff2
CLDR has removed the "ERA0" code from the Coptic calendar but it looks
like they neglected to actually remove the era - if you try to format a
date with a negative era year, it prints a positive era year and omits
the era code (the final part of formatToParts() is a literal " " part,
which normally should be followed by an era part.) Handle this
eventuality with a special case.

UPSTREAM_COMMIT=49e2d1d385a05c4402d954b517105cfd609ac362
We need to do the non-negative modulo here, otherwise all negative years
are leap years.

UPSTREAM_COMMIT=0df570c70ef9d6b97f4bdd204ac753ad7fa3cd93
…ring durations

See tc39/proposal-temporal#3168

UPSTREAM_COMMIT=17e12be0af42a02b3389d22417923f29f93c16c5
The spec mandates that the shape of the fields property bag must be
checked (and a TypeError thrown if incorrect) before any RangeError
conditions are checked.

See #3166

UPSTREAM_COMMIT=98991268f1f06a1865f136c196383076a50fdcde
As per tc39/proposal-temporal#2940 this belongs
in NonISOResolveFields, before any RangeError conditions are checked.

UPSTREAM_COMMIT=99e43310d04cff672ccd38cd8470596f984287eb
I have gotten confused by the lack of resolveLunisolarMonth here several
times, so add a comment explaining that it's intentional.

UPSTREAM_COMMIT=66268cb2cc2297228e416179b0d922a86e0e18ed
UPSTREAM_COMMIT=264900c8312711c000db1b70e574284e2fd319a4
This fixes a bug where adding 1 year 0 months would skip applying the
overflow option. Brings the polyfill calendar code in line with the
spec text of proposal-intl-era-monthcode.

(For example, in 2020-02-29[u-ca=gregory] + P1Y with overflow: "reject")

UPSTREAM_COMMIT=8a51e7e8cd849d83db912c42523eb1802cb3799e
UPSTREAM_COMMIT=419764273a68b77edd6254a975666c7d9413a7de
In the tabular Hijri calendars, the months have a set number of days
with only one month varying between 29 and 30 days in leap years. In
the observational Hijri calendar (islamic-umalqura) any month can have
30 days.

This applies that information to make maxLengthOfMonthCodeInAnyYear() in
helperIslamic more accurate. It prevents a slow search roundtripping
through 20 years when code like this is executed:

    Temporal.PlainMonthDay.from({
      calendar: 'islamic-tbla',
      monthCode: 'M10',
      day: 30
    })

This PlainMonthDay shouldn't exist. The change makes it fail immediately
rather than exhausting the search space.

This is just a polyfill optimization. The spec text remains correct.

UPSTREAM_COMMIT=b7d96ecbc24fe1f284b9454550ac76b1df677ae2
In hindsight, when I wrote the maxLengthOfMonthCodeInAnyYear() code in
e977eff0 I put it in a slightly odd place. There is no need to check
overflow in each iteration of the search loop, we can check it at the
beginning.

And in fact this code was buggy: PlainMonthDays created from
out-of-range days with overflow: 'constrain' would sometimes get wrong
reference years. For example:

   Temporal.PlainMonthDay.from({
     calendar: 'gregory',
     monthCode: 'M12',
     day: 32
   })

This would get the incorrect reference year 1971, and would actually
produce a different PlainMonthDay instance from the same call with day:
31, which would get the correct reference year 1972.

This was a polyfill-only bug. No change needed to the spec text.

UPSTREAM_COMMIT=ef85a85f791085b4b0b58083a4e9e6890638a74b
We need to add some new tests to the expected failures file: they expose
a bug in ICU4C where the wrong leap month was calculated for the Chinese
calendar in 1987.

UPSTREAM_COMMIT=2cee76afbb27bda99126d0b7e69fc606499c0b2b
@craig-o-curtis

Copy link
Copy Markdown

The normative change in this PR (50d66d2, porting proposal-temporal 5dd0b0d97ee1 / tc39/proposal-temporal#3168) fixes a user-visible wrong answer on main today. Sharing the evidence in case it helps prioritise the review, since the PR description understandably describes it as "unlikely to break any working code".

const from = Temporal.PlainDateTime.from("2024-01-31T00:00:00");
const to   = Temporal.PlainDateTime.from("2024-02-29T12:00:00");

from.until(to, { largestUnit: "month" }).toString();
// "P29DT12H" on both — correct, and the reason r1 is 0 months

from.until(to, { largestUnit: "month", smallestUnit: "month", roundingMode: "trunc" }).toString();
// main: "PT0S"                after 50d66d2: "P1M"

from.until(to, { largestUnit: "month" }).total({ unit: "month", relativeTo: from });
// main: 1.0172413793103448    after 50d66d2: 1.0161290322580645

Temporal.Duration.from("P29DT12H").round({
  smallestUnit: "month", roundingMode: "floor",
  relativeTo: Temporal.PlainDate.from("2024-01-31"),
}).toString();
// main: "PT0S"                after 50d66d2: "P1M"

Both columns are measured here, from two NODE_ENV=production builds — main at c8f344c, and the same tree with 50d66d2 applied on top. The post-fix values agree with Chromium 153.0.8010.12; on #3168 ptomato recorded that SpiderMonkey, temporal_rs and the FullCalendar polyfill agree with V8 too.

Jan 31 + 1 month is Feb 29 only by constraining, so until doesn't count it as a month and r1 is 0. The first window is then Jan 31 → Feb 29, which ends before the Feb 29T12:00 target, so progress exceeds 1 and the result is taken against bounds that don't contain it. In production builds the ${unit} was 0 days long assert is stripped, so it returns a wrong value instead of throwing. Only a relativeTo on the 29th, 30th or 31st can reach it.

Scanning Chromium 153.0.8010.12 against published 0.5.1: total 16 mismatches in 2,016 rows, round 15 in 12,960, until/since with a calendar smallestUnit 15 in 9,940 — all unit: "month", and only in the directed rounding modes (ceil/floor/trunc). The half modes agree either way, because with progress > 1 they still select r2. until without rounding agrees on every row, and Duration.compare reaches no nudge window at all (256 rows, 0 mismatches).

One note for whoever reviews it: #3168's own reproducer (new Temporal.Duration(1, 0, 0, 0, 1) relative to 2020-02-29, unit years) passes on 0.5.1 — there r1 is already 1 from the duration's own years field, so no retry is needed. The month cases above are what expose it, and test262 doesn't cover this yet.

Verification of 50d66d2 applied alone on top of main, Node 24.21.0 / ICU 78.3:

  • npm test — 607 passed, 4 suites, 0 failed
  • npm run test262 (pinned ac3035f0, production build) — 4855 finished, 4828 passed, 0 failed, 0 passed unexpectedly, 27 expected failures, 6 missing
  • npm run lint — clean

It applies cleanly on main by itself and touches only lib/ecmascript.ts, so if it would help to have this one commit split out as a small standalone PR while the rest of the rebase waits for review, I'm happy to open it — just say the word. Otherwise I'll leave it here where it belongs.

Found while testing @northguild/gmt against the specification; it carries a dormant workaround until a release lands.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

The updated rounding nudging logic can select an incorrect nudgedEpochNs after a retry, and era is no longer treated as a date option in Intl-style defaulting, both of which can produce incorrect observable behavior.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 2 Medium severity

Open (2)
What changed in this PR

This PR continues the April 2026 rebase work by adding/adjusting ICU workarounds for non-ISO calendars (notably Chinese/Dangi/Coptic/Ethiopic) and updating internal rounding logic to match the November 2025 TC39 normative change, with the goal of improving test262 pass rates across newer ICU versions.

Changes:

  • Updated calendar parsing/validation to handle newer ICU month/era quirks (e.g., Chinese/Dangi “Mo…” month parts, Coptic era omissions).
  • Refactored and extended relative-duration rounding (“nudge”) logic for calendar-unit rounding edge cases.
  • Updated test expected-failure lists to reflect known ICU behavior regressions/bugs.
File Description
test/​expected-failures.txt Adds new expected failures for Chinese calendar monthCode tests due to ICU4C leap-month issues.
test/​expected-failures-cldr48.txt Removes CLDR48-specific expected failures and adjusts the remaining set for current behavior.
lib/​intl.ts Changes option-amend/defaulting logic for Temporal toLocaleString, including new handling around era.
lib/​ecmascript.ts Refactors calendar-unit nudging window computation and rounding behavior for relative durations.
lib/​calendar.ts Implements multiple ICU workarounds and strengthens calendar field resolution/validation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/intl.ts
Comment thread lib/intl.ts

@justingrant justingrant left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks great, thanks for porting these commits, we're getting close to present day!

Copilot had some feedback you may want to look at, but I didn't see anything obvious to worry about.

@ptomato
ptomato merged commit e49705c into main Sep 21, 2026
32 checks passed
@ptomato
ptomato deleted the rebase-part3 branch September 21, 2026 23:57
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.

Bug: Malformed leap month suffix while implementing Chinese calendar

5 participants