Month doesn't equal itself when one is a result of adding or subtracting to a month before, but only in Node.js

[rsstiglitz]

This might be a Node bug, running the same code in Chrome DevTools for example yields true.

Months which are a result of going forward are fine:

const monthAfter1 = Temporal.PlainYearMonth.from({year: 2022, month 11}).add({month: 1});
const monthAfter2 = Temporal.PlainYearMonth.from({year: 2022, month 11}).subtract({month: -1});

monthAfter1.equals(Temporal.PlainYearMonth.from({year: 2022, month 12})) // true
monthAfter2.equals(Temporal.PlainYearMonth.from({year: 2022, month 12})) // true

while going backwards breaks equals:

const monthAfter1 = Temporal.PlainYearMonth.from({year: 2022, month 11}).add({month: -1});
const monthAfter2 = Temporal.PlainYearMonth.from({year: 2022, month 11}).subtract({month: 1});

monthAfter1.equals(Temporal.PlainYearMonth.from({year: 2022, month 10})) // false
monthAfter2.equals(Temporal.PlainYearMonth.from({year: 2022, month 10})) // false

Sorry this isn't more detailed, already wasted 2 hours on this nasty bug and now I'm tired. 😝

[rsstiglitz]

This code right here seems to be the culprit, I'm not sure though: https://github.com/fullcalendar/temporal/blob/8ddf7a709375bf1586587512968094daaa525f43/packages/temporal-polyfill/src/public/plainYearMonth.ts#L189C1-L193C5

The "official" docs state:

The overflow option has no effect in the default ISO 8601 calendar, because a year is always 12 months and therefore not ambiguous. It doesn't matter in this case that years and months can be different numbers of days, as the resolution of Temporal.PlainYearMonth does not distinguish days. However, overflow may have an effect in other calendars where years can be different numbers of months.

So it's probably not a good idea to convert the month to a PlainDate and to use daysInMonth internally.

[leonadler]

Unfortunately, TemporalPlainYearMonth.prototype.compare has the same erronous behavior 😬

const a = Temporal.PlainYearMonth.from({ year: 2023, month: 5 }).subtract({ months: 1 });
const b = Temporal.PlainYearMonth.from({ year: 2023, month: 4 });

Temporal.PlainYearMonth.compare(a, b)  // => 1 (should be 0)
a.equals(b)                            // => false (should be true)

The problem seems to be that, as you stated, PlainDate is used internally for probably this use case:

const a = Temporal.PlainYearMonth.from({ year: 2023, month: 11 });
a.subtract({ days: 29 })   // => "2023-11"
a.subtract({ days: 30 })   // => "2023-10"

which would be fine, if the resulting PlainYearMonth would have its isoDay set to 1 again after the subtraction.
Instead, the isoDay is set to the result of the substraction:

const a = Temporal.PlainYearMonth.from({ year: 2023, month: 5 });
a.getISOFields()
// => { isoYear: 2023, isoMonth: 5, isoDay: 1, calendar: isoCalendar }

a.subtract({ days: 10 }).getISOFields()
// => { isoYear: 2023, isoMonth: 5, isoDay: 21, calendar: isoCalendar }

This also seems the difference between PlainYearMonth in this polyfill and the one on the tc39 proposal page:

const thisPolyfill = await import('temporal-polyfill');
const otherPolyfill = await import('@js-temporal/polyfill');

const a = thisPolyfill.Temporal.PlainYearMonth.from({ year: 2023, month: 11 }).subtract({ days: 10 }).getISOFields();
const b = otherPolyfill.Temporal.PlainYearMonth.from({ year: 2023, month: 11 }).subtract({ days: 10 }).getISOFields();

console.log(a)  // => { isoYear: 2023, isoMonth: 11, isoDay: 20, calendar: <Temporal.Calendar> }
console.log(b)  // => { isoYear: 2023, isoMonth: 11, isoDay: 1, calendar: "iso8601" }

Fix

This should either be an easy fix in PlainYearMonth itself:

 export class PlainYearMonth extends AbstractISOObj<Temporal.PlainDateISOFields>
   implements Temporal.PlainYearMonth {
   constructor(
     isoYear: number,
     isoMonth: number,
     calendarArg: Temporal.CalendarLike = createDefaultCalendar(),
     referenceISODay = 1,
   ) {
     const constrained = constrainDateISO({
       isoYear,
       isoMonth,
       isoDay: referenceISODay,
     }, OVERFLOW_REJECT)
     const calendar = ensureObj(Calendar, calendarArg)

     validateYearMonth(constrained, calendar.toString())

     super({
       ...constrained,
+      day: 1,
       calendar,
     })
   }

Workaround / monkey-patch

or can be fixed in userland until the current in-progress-huge-refactor is done:

// https://github.com/fullcalendar/temporal/issues/21
export function monkeyPatchTemporalPolyfillIssue21() {
  const yearMonth = new Temporal.PlainYearMonth(2000, 1);
  if (!yearMonth.equals(yearMonth.subtract({ days: 1 }))) {
    const originalGetISOFields = Temporal.PlainYearMonth.prototype.getISOFields;
    Temporal.PlainYearMonth.prototype.getISOFields = function patchedGetISOFields(this) {
      return { ...originalGetISOFields.call(this), isoDay: 1 };
    };
  }
}

[leonadler]

@arshaw Would you like/merge a PR for this, or do you prefer I retest it after one refactor to rule them all is finished?

[arshaw]

This is now fixed in v0.2.0. Please confirm when you have a chance. Thanks all!

[rsstiglitz]

Thank you for the release! Was anyone able to find out why or reproduce if this was not an issue in browsers, only in Node.js, like I stated in the OP? Even if this is fixed now, I would still be very very interested how that could've happened...

[arshaw]

@rsstiglitz I'd be very curious to know myself! However I don't think I have the bandwidth right now to spin up the old code for debugging. I can't think of an obvious culprit, so this might remain a mystery.