js-temporal/temporal-polyfill

Removed month number on PlainMonthDay

Closed this issue · 4 comments

Hello,

In a recent update, monthCode (a string) was added next to month (a number), cf. #1203.
In the associated changes, for PlainMonthDay the month number was removed while other types kept it, cf. here.

Was this done on purpose? I know it is still possible to get it via getISOFields().isoMonth, but I was wondering if it was meant to be this way or if it was a slight mistake in the commit.

Note: great work on this much needed "improvement" (if you can still call it that given the scope) to good old Date by the way

Note: will create a PR soon

Thanks for taking the time to report the issue!

This was actually done on purpose. In many non-Gregorian calendars, the numeric month index is meaningless in the context of PlainMonthDay, because the sequence of months may not be the same every year. So PlainMonthDay only has a month identifier (monthCode).

(The closest equivalent I can think of in the Gregorian calendar, is that we don't have a dayOfYear property on PlainMonthDay, because without knowing whether the year is a leap year, we wouldn't know whether Temporal.PlainMonthDay.from('07-26').dayOfYear should be 207 or 208.)

.getISOFields().isoMonth will not necessarily give you the same value as .month would have, because the PlainMonthDay may be in a non-Gregorian calendar. The correct way to get the month index would be to choose a year in order to resolve the ambiguity: .toPlainDate({ year: ... }).month.

Hello,

I was actually told it was an issue in tc39/proposal-temporal#1675 but if it's not that's also fine 😁.

I think I understand your explanation that a month cannot always be determined without having the year information, the dayOfYear example works well for that. For my specific use-case I can still get by with getISOFields because I'm only going to be using an ISO calendar (using toPlainDate to solve the issue in a more general scenario is also a good tip).

I was actually told it was an issue in tc39/proposal-temporal#1675 but if it's not that's also fine 😁.

I updated that issue with a link to @ptomato's comment above which is correct.

I think I understand your explanation that a month cannot always be determined without having the year information, the dayOfYear example works well for that.

Here's a more specific example: in the Chinese calendar, 2020 is a leap year. The Chinese calendar is lunisolar, which means that months are intended to track the cycles of the moon but still should land near the same part of the solar year. Because there are more than 12 lunar cycles in each solar year, every few years a "leap month" is added to keep lunar months from moving around the seasons. (Lunar calendars, like the Islamic calendar, have months that move through the seasons. This is why Ramadan sometimes lands in the cooler part of the year, which is easier for fasters, and sometimes lands in the summer which is harder!)

Anyway, in lunisolar calendars like Chinese or Hebrew, the month number of a birthday or holiday may be different in a year with a leap month vs. in a normal year. For example:

// Birthday in the first day of the last month of the Chinese year
birthday = Temporal.PlainMonthDay.from({monthCode: 'M12', day: 1, calendar: 'chinese'});

// leap year
birthday.toPlainDate({year: 2020}).month;
// => 13

// normal year
birthday.toPlainDate({year: 2019}).month;
// => 12