Repository navigation
Rebase part 4 - #374
Rebase part 4#374
Conversation
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It is a large, wide-ranging rebase touching core date arithmetic, string parsing, calendar/era logic, and Intl formatting, including a sign-convention flip and many guard-to-assert conversions that warrant final human review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
This is the fourth in a series of "rebase" PRs that port upstream changes from tc39/proposal-temporal (Dec 2025 – Jan 2026) into this polyfill. The dominant theme is replacing user-facing guard clauses that were determined to be unreachable with internal assert(...) calls, plus several core refactors: a new AddDaysToISODate helper replacing BalanceISODate, a rewrite of the ISO-string parsing regexes to use named capture groups, calendar era handling switching from hasYearZero to a numeric startingYear, and new dateStyle/timeStyle handling in the Intl.DateTimeFormat wrapper.
Changes:
- Convert many unreachable
throwguards intoassert(...)acrossecmascript.tsandcalendar.ts; drop redundant=== undefined ? 0param defaults in constructors. - Rewrite
lib/regex.tsparsing regexes to use named capture groups (relies on ES2025 duplicate named groups) and updateParseISODateTime/ParseTemporalTimeString/offset parsing to readmatch.groups. - Refactor calendar difference logic (
untilCalendar,ISODateSurpasses→CompareSurpasses,NonIsoCalendar.dateUntilweek/day fast path), flip thesignconvention inDifferenceZonedDateTime, and update IntldateStyle/timeStyleslot handling; update test expected-failure lists.
| File | Description |
|---|---|
| lib/ecmascript.ts | Guard→assert conversions, AddDaysToISODate adoption, named-group parsing, DifferenceZonedDateTime sign flip, equal-date short-circuit in CalendarDateUntil. |
| lib/calendar.ts | untilCalendar/ISODateSurpasses rewrite, startingYear era model, clampISODate, non-ISO week/day diff fast path, guard→assert conversions. |
| lib/regex.ts | Regexes rebuilt with named capture groups for date/time/offset/duration parsing. |
| lib/intl.ts | CreateDateTimeFormat export, dateStyle/timeStyle slot handling with nullable formatters, formatRange type check simplification. |
| lib/slots.ts | DATE/YM/MD/TIME_FMT slots widened to `FormatterOrAmender |
| lib/plain*.ts, duration.ts, zoneddatetime.ts | Dropped redundant undefined param defaults; switched toLocaleString to CreateDateTimeFormat; direction guard→assert. |
| lib/math.ts | Removed dead ` |
| index.d.ts | Removed Temporal.ZonedDateTime from Intl.Formattable. |
| test/*.txt, runtest262.mjs, test/validStrings.mjs | Updated expected-failure lists, removed cldr48 handling, year ?? 1972 fallback. |
I verified the most correctness-sensitive parts: the DifferenceZonedDateTime sign flip is internally consistent across all its sign uses; the next.day = calendarOne.day in-place mutation and the assertion-based refactors match the upstream reference implementation; the reworked regexes construct and parse correctly on Node 24; the era startingYear migration leaves no stray hasYearZero; and impl.dateUntil is only reached via CalendarDateUntil, so the new equal-date asserts are safe.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| throw new RangeError(`property '${property}' cannot be a a number less than one`); | ||
| } | ||
| throw new RangeError('Cannot convert a number less than one to a positive integer'); | ||
| throw new RangeError(`property '${property}' cannot be a a number less than one`); |
There was a problem hiding this comment.
Pre-existing in the spec polyfill, will fix it there and pull it in here in the next rebase.
8169463 to
802fbf3
Compare
|
I'll have to do a bit of refactoring to remove the regex duplicate named capture groups. Will push a new version shortly. |
5dea532 to
bab5877
Compare
justingrant
left a comment
There was a problem hiding this comment.
Looks great, only a few minor comments/questions.
This offset is not allowed because the separator for hours/minutes and the separate for minutes/second must be consistent. To make this change, I refactored most of the regexps used for time/date parsing to use named groups. This was to make the code easier to follow The change in semantics is found in the `offsetWithParts` and `offset` regexes, which previously made the ':' separator optional in both of these cases, allowing +00:0000 or +0000:00. The fix is to use a disjunction (introducing new regexps optionalMinSec and optionalMinSecNoSep), to enforce that the separator or lack thereof is consistent. UPSTREAM_COMMIT=bc64d40d8a42b81a3d1cb21a3bc291da9d6d0b8d
…onds The `IsValidEpochNanoseconds` check was previously omitted when the time zone was UTC. UPSTREAM_COMMIT=d90d432b2ff10ffc4149dd0ba8f0aece21150adc
This affects something like:
(new Temporal.PlainDate(1970, 1, 1)).withCalendar("T11:30[u-ca=buddhist]").calendarId
Previously, the calendar annotation was being ignored and the
result would be 'iso8601'.
UPSTREAM_COMMIT=589edc109ded23ba87e5af28eccc8a6eaf40c112
Previously, strings like "00:000" or "0000:00" were being accepted by PlainTime.from. UPSTREAM_COMMIT=251d2316d3042f185c9e9b94cbabe674623050e4
…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
This was based on erroneous data in CLDR. See https://unicode-org.atlassian.net/browse/CLDR-11375 UPSTREAM_COMMIT=977d11e0d0a47896cb43124c57b6364058e017a8
See the change in tc39/proposal-temporal#3175 for details. `era` should not be added to the default options for PlainMonthDay, because [[era]] isn't copied in step 12.b of GetDateTimeFormat when defaults is MONTH-DAY. The spec is correct here, this brings the reference code into alignment with it. UPSTREAM_COMMIT=0512c498552b07d3898cf2fa41acb360f4a004c6
This is easier to debug when we know which date is out of range. UPSTREAM_COMMIT=0eb86b4d757e6c91729e0ab439b2dcd928f4b773
6aefa42 to
b6b28c3
Compare
UPSTREAM_COMMIT=48063a97dbf32636a0a174f91dfd6451082c63db
(lastRelDow < 0) can never be true, so replace the check with an assertion. UPSTREAM_COMMIT=902f86192e3aede5589966707bd8c3d2cc8691ba
The polyfill always calls ISODateSurpasses with weeks and days equal to 0, and the arguments are only used when they're not equal to 0, so remove them altogether. (In the spec, ISODateSurpasses is called with non-zero weeks and days, but the polyfill does an optimization that means it's not called with non-zero weeks or days.) UPSTREAM_COMMIT=6cb1c252ee0061efbe9402fe356c12d2209856a2
Some checks in eraFromYear must always be true; change exception-throwing code to assertions and document why the assertions are always true. UPSTREAM_COMMIT=cc6406dcf6ad74739f6ba0a9a72684e715fafaf7
This code is unreachable because it's inside an `else` clause attached to `if (year !== undefined)`, and `year` equals `calendarDate.year`. UPSTREAM_COMMIT=199923d59e99d3e9dbc29c2ac52e312ce5e1079e
This method is never called. UPSTREAM_COMMIT=f55d4665fd988b43b99d24e35edb71d3f984d474
The arguments all default to 0 already. UPSTREAM_COMMIT=87492bc662e7a2babd824a35148e5563764c234b
Month and day must not be undefined if the zoneddatetime regex matched. UPSTREAM_COMMIT=e80bed16dc83db2316630048b2a1411b566243f1
UPSTREAM_COMMIT=93124cdafd2569bac2dfd1717021475628473c8b
The previous line of code returns if x is 0, so MathSign(x) || MathSign(z) can just be MathSign(x). UPSTREAM_COMMIT=d21b95a9a9090d705c07545e16c27f2338bd4162
Several constructors had redundant code that checked a value for undefined and set it to 0 if so. This was redundant because the constructor already initializes these values. UPSTREAM_COMMIT=451af1f9fad323e45ddc0482688e609acd54c9e9
…ring tc39/ecma402#683 has been resolved, so this check can be removed. UPSTREAM_COMMIT=6bb83a6b1d5c9ad83ea4e3d5a56cea6191966b8c
UPSTREAM_COMMIT=9e63759316a5c90838e40deea84033006863d07b
…eISOYearMonth I don't see a case where these would be called on Infinity. UPSTREAM_COMMIT=315c8903adf71ea61a0a7624062d249c0e6e25b8
This method is only called on valid dates, so monthInfo should never be undefined. Also includes a similar change for the Chinese calendar. UPSTREAM_COMMIT=9f8bd77412a8397939d2099685a6400e3213fd60
This is only called on valid dates, so year should always be defined. UPSTREAM_COMMIT=dd78b2bfa0bde3c7d4a55c782e08c2c8f50e2af7
…o a assertions (#3225) UPSTREAM_COMMIT=3c8b91677052c9d91c49d683df4a4d0dc27f5f26
UPSTREAM_COMMIT=0f3d3d45754015702b400a6ed807d2f7803a18d5
UPSTREAM_COMMIT=3a3349b220ddf9a290f6a2e31acb12a1354caa03
This addresses issue tc39/proposal-temporal#3210 The regex parser falls into a corner case for strings with separators that have fractional minutes and no seconds. It ends up parsing `undefined` minutes and interprets the fraction as seconds. Instead of complicating the regex parser, change ParseISODateTime and ParseTemporalTimeString to throw a RangeError if there a fraction is defined, and either: seconds are undefined (safety measure, the regex parser actually takes care of this) or: minutes are undefined and hours are defined. This takes care of the corner case without complicating the regex parser, for readability. UPSTREAM_COMMIT=72ec688b0ced0126469ae931bc1652cde614f201
UPSTREAM_COMMIT=5363772c0b64ce2e52e6c407fbef6b8ef61e164b
…d formatRangeToParts (squashed with) Polyfill: Change check to assertion in sameTemporalType This function is only called when isTemporalObject(x) is true and isTemporalObject(x) === isTemporalObject(y). Therefore, change the check for isTemporalObject(x) && isTemporalObject(y) to an assertion. UPSTREAM_COMMIT=876bb19e0450774eb5bbe6f075975c2da2093c99
As far as I can see, this function shouldn't be called on undefined. UPSTREAM_COMMIT=92f0ba9816ff56cc74d3c4b69ee404b55448d944
This shouldn't be called with infinity as the value of any field, as far as I can tell. UPSTREAM_COMMIT=e65f411f1cfceb3a54c30139a3d734f7efa7f04b
…3230) The TemporalInstantString production requires a Time, so `time` will never be `start-of-day` in this code. UPSTREAM_COMMIT=23220c07ea45b6efcf10e21c6ae13a8c5af7c987
This change makes it possible to change a check to an assertion in the polyfill. UPSTREAM_COMMIT=264e4b8fae582faf2f3f558f1d72ae29cae5607f
…alendar This is called on dates that have already been validated, so it should be an internal error if there is no length for a valid month code. UPSTREAM_COMMIT=5d15d2da934af505db333c5b0a311d30b1ffb1f6
intl402/Temporal/PlainYearMonth/from/extreme-dates.js was already in the expected-failures file, but add a note that it will also fail as long as tc39/proposal-temporal#3251 is an issue. UPSTREAM_COMMIT=b2e0b7bf6e025da681b8a9ee1a8121c0501ca7e0
As of tc39/proposal-temporal#3218 , this check is always done before calling dateUntil. UPSTREAM_COMMIT=a91494f8a4ea272d9f24c4d90952707ba1afa956
UPSTREAM_COMMIT=9291822b284e9b705ecf3d302a95d4f6d8517e0e
Remove 'timeZoneName' from hasAnyDateTimeOptions. This matches the spec (https://tc39.es/proposal-temporal/#sec-getdatetimeformat step 14). hasDateOptions and hasTimeOptions already check for dateStyle and timeStyle respectively, so those can be deleted as well. UPSTREAM_COMMIT=082b679214ecd1820cb6190c3b58c95ed5f550fb
This fixes a polyfill bug that was previously not covered by tests. When calling the toLocaleString methods of PlainDate, PlainMonthDay, PlainTime, or PlainYearMonth, the formatter should be created using the 'required' parameter, so that at least one date or time option is required (depending on the type being formatted). This parameter does not correspond to any constructor argument of Intl.DateTimeFormat, so we need to export a CreateDateTimeFormat() function from intl.mjs to expose the functionality to other files. Rename the existing createDateTimeFormat() to internalCreateDateTimeFormat(). UPSTREAM_COMMIT=2e11131cfb489a8d8860f63e6402c3ee4bf8109e
UPSTREAM_COMMIT=1a46f871a3915689645e66253cd533867c10c452
Update internal era data to change hasYearZero to startingYear, so that we can accommodate the starting year of 6. UPSTREAM_COMMIT=993e6322d56971b68e539d007a32c62bf94a4032
UPSTREAM_COMMIT=1fec0fb347c1578a41503bf5bed8bd39523715c7
These can be calculated calendar-independently, so we can save the cost of converting an ISO date to calendar space and back for no reason. UPSTREAM_COMMIT=a5a4fd977dace1b32ba8379a4f9231eb2029a5c9
See tc39/proposal-intl-era-monthcode#101. This changes the general outline of ISODateSurpasses so that it parallels the outline of NonISODateSurpasses in the Intl Era/Month Code proposal as proposed in the PR, and adds the CompareSurpasses operation. Also refactors the polyfill code, but keeps the optimizations we already had regarding not passing weeks and days to ISODateSurpasses. UPSTREAM_COMMIT=196a31919d2997b66a3c4171f94baf9fc3f5dd4a
UPSTREAM_COMMIT=137d270a503def2ab06612151a33c119fb037d90
b6b28c3 to
64e750f
Compare
64e750f to
1f34f12
Compare

This rebase PR covers changes from December 2025 and January 2026. A large part of it consists of deleting code that was determined to be unreachable.
LLMs or human reviewers can look at the SHAs in each commit (e.g.
UPSTREAM_COMMIT=bc64d40d8a42b81a3d1cb21a3bc291da9d6d0b8d) to see the corresponding commit in https://github.com/tc39/proposal-temporal.