Normative: Changes to AddDurationToYearMonth #3208

catamorphism · 2025-12-12T00:06:22Z

Add years/months first, then skip to the end of the month if the duration sign is negative, then add the other duration fields. This is to address cases with overflow: reject where previously, an exception was thrown due to constraining the internally-constructed end-of-the-month date.

In the spec, new abstract operations CalendarDateLastDayOfMonth and NonISODateLastDayOfMonth are added to make the algorithm simpler to specify.

Closes #3197

Add years/months first, then skip to the end of the month if the duration sign is negative, then add the other duration fields. This is to address cases with overflow: reject where previously, an exception was thrown due to constraining the internally-constructed end-of-the-month date. In the spec, new abstract operations CalendarDateLastDayOfMonth and NonISODateLastDayOfMonth are added to make the algorithm simpler to specify.

catamorphism · 2025-12-12T00:12:24Z

The test262 failures are expected; see tc39/test262#4783 .

catamorphism · 2025-12-12T00:49:35Z

The NonISODateLastDayOfMonth AO also needs to be defined in proposal-intl-era-monthcode, but I'll wait for feedback on this PR before I do that.

ptomato

This is much nicer than the previous approach with two overflow parameters!

(Comments on the polyfill code also apply to spec text.)

ptomato · 2025-12-12T17:44:53Z

polyfill/lib/ecmascript.mjs

+  let previousIsoDate = isoDate;
+  let nextIsoDate = isoDate;
+  let calendarDate = calendarImplForID(calendar).isoToDate(nextIsoDate, { daysInMonth: true });
+  while (calendarDate.day !== calendarDate.daysInMonth) {
+    previousIsoDate = nextIsoDate;
+    calendarDate = calendarImplForID(calendar).isoToDate(nextIsoDate, { daysInMonth: true });
+    try {
+      nextIsoDate = CalendarDateAdd(calendar, nextIsoDate, { days: 1 }, 'constrain');
+    } catch (e) {
+      // Date overflowed maximum range => consider this the last day
+      break;
+    }
+  }
+  return previousIsoDate;


The previous approach for jumping to the last day of the month (jump to day 1, add 1 month, subtract 1 day) was chosen mainly because it made the fewest calls into user code. Since we dropped user-defined calendars, we will always have zero calls into user code, so I'm not opposed to rewriting this to be more efficient.

But, daysInMonth is the count of days in the month, not the day number of the last day of the month so although this rewrite is correct for all currently supported calendars, it'll break when ICU adds a calendar that skips days (which has been a plan for some time now.)

You could add days one at a time until the month number changes? or just add the number of days in the month minus 1, I think?

I'm also not (yet) convinced about stopping the loop early when we get to the end of the range. Although it would allow Temporal.PlainYearMonth.from('+275760-09').subtract({ months: 1 }) which previously wouldn't work, it would also allow oddities like Temporal.PlainYearMonth.from('+275760-09').subtract({ weeks: 2 }) resulting in +275760-08.

One thing we could do to not error out on the end of the range, is just skip the second addition if the weeks and days are zero.

Changed in cf79f59 to not assume there are no skipped days in the month.

Although it would allow Temporal.PlainYearMonth.from('+275760-09').subtract({ months: 1 }) which previously wouldn't work, it would also allow oddities like Temporal.PlainYearMonth.from('+275760-09').subtract({ weeks: 2 }) resulting in +275760-08.

If we don't stop the loop early, we get Temporal.PlainYearMonth.from('+275760-09').subtract({ weeks: 2 }) throwing because 275760-09-16 is out of range. I find that just as surprising as the result being 275760-08. I'm keeping it as it is for now, but could be convinced otherwise.

What kind of evidence would convince you otherwise? 😄 Personally I'd prefer the throwing result for Temporal.PlainYearMonth.from('+275760-09').subtract({ weeks: 2 }) because the alternative is having the types behave weirdly:

> Temporal.PlainYearMonth.from('+275760-09').subtract({ weeks: 2 }) // → +275760-08 > Temporal.PlainYearMonth.from('+275760-08').subtract({ weeks: 2 }) // → also +275760-08

In other words I'd prefer not to shoehorn handling this case into the algorithm if it makes the overall design less predictable.

polyfill/lib/ecmascript.mjs

ptomato · 2025-12-12T17:55:29Z

spec/calendar.html

+    <emu-clause id="sec-temporal-nonisodatelastdayofmonth" type="implementation-defined abstract operation">
+      <h1>
+        NonISODateLastDayOfMonth (
+          _calendar_: a calendar type that is not *"iso8601"*,
+          _isoYear_: an integer,
+          _isoMonth_: an integer between 1 and 12 (inclusive),
+        ): an ISO Date Record
+      </h1>
+      <dl class="header">
+        <dt>description</dt>
+        <dd>
+          The operation performs implementation-defined processing to determine the last day of the month denoted by _isoDate_. It returns an ISO Date Record with the same year and month as _isoYear_ and _isoMonth_, and the day set to the last day of _isoMonth_ in _isoYear_ according to the years, months and weeks reckoning of _calendar_.
+        </dd>
+      </dl>
+    </emu-clause>


I think it'd be better to specify CalendarDateLastDayOfMonth in terms of existing implementation-defined operations if possible, then we wouldn't have to have another implementation-defined operation.

The problem is that if we specify it in terms of NonISODateAdd, there's no way to "catch" the exception if the result of the addition is outside the representable range and we want to back off to the last representable day. (Or is there a way to catch exceptions in spec language?)

I think the way to do it would be to check ISODateWithinLimits after we call CalendarDateLastDayOfMonth.

ptomato · 2026-01-08T23:22:44Z

spec/calendar.html

+          _isoYear_: an integer,
+          _isoMonth_: an integer between 1 and 12 (inclusive),
+        ): an ISO Date Record
+      </h1>
+      <dl class="header">
+        <dt>description</dt>
+        <dd>
+          The operation performs implementation-defined processing to determine the last day of the month denoted by _isoDate_. It returns an ISO Date Record with the same year and month as _isoYear_ and _isoMonth_, and the day set to the last day of _isoMonth_ in _isoYear_ according to the years, months and weeks reckoning of _calendar_.


I think either we want to pass an ISO Date Record here, or pass the calendar year and month. E.g.:

Suggested change

_isoYear_: an integer,

_isoMonth_: an integer between 1 and 12 (inclusive),

): an ISO Date Record

</h1>

<dl class="header">

<dt>description</dt>

<dd>

The operation performs implementation-defined processing to determine the last day of the month denoted by _isoDate_. It returns an ISO Date Record with the same year and month as _isoYear_ and _isoMonth_, and the day set to the last day of _isoMonth_ in _isoYear_ according to the years, months and weeks reckoning of _calendar_.

_year_: an integer,

_month_: an integer between 1 and 13 (inclusive),

): an ISO Date Record

</h1>

<dl class="header">

<dt>description</dt>

<dd>

The operation performs implementation-defined processing to determine the last day of the month denoted by _year_ and _month_. It returns an ISO Date Record representing a date in the reckoning of _calendar_ that has the the same year and month as _year_ and _month_, and the day set to the last day of _month_.

Passing the ISO year and month seems like it's not enough info to determine the last day of the calendar month.

ptomato · 2026-01-08T23:27:11Z

spec/calendar.html

+            1. Let _maxDay_ be ISODaysInMonth(_isoDate_.[[Year]], _isoDate_.[[Month]]).
+          1. Return CreateISODateRecord(_isoDate_.[[Year]], _isoDate_.[[Month]], _maxDay_).
+        1. Else,
+          1. Return NonISODateLastDayOfMonth(_calendar_, _isoYear_, _isoMonth_).


isoYear and isoMonth don't exist yet here.

catamorphism added 2 commits December 11, 2025 16:04

Fix formatting error

a7d112f

ptomato requested changes Dec 12, 2025

View reviewed changes

catamorphism added 4 commits December 12, 2025 18:40

Improve CalendarDateLastDayOfMonth

cf79f59

Use ToDateDurationRecordWithoutTime earlier

07015a3

Fix spec errors

5223614

Fix lint error

771534b

ptomato reviewed Jan 8, 2026

View reviewed changes

Normative: Changes to AddDurationToYearMonth #3208

Are you sure you want to change the base?

Normative: Changes to AddDurationToYearMonth #3208

Conversation

catamorphism commented Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

catamorphism commented Dec 12, 2025

Uh oh!

catamorphism commented Dec 12, 2025

Uh oh!

ptomato left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

catamorphism commented Dec 12, 2025 •

edited

Loading