tc39 / proposal-temporal

Provides standard objects and functions for working with dates and times.
https://tc39.es/proposal-temporal/docs/
Other
3.29k stars 149 forks source link

ISOMonthDayFromFields ignores year when monthCode is present #2497

Closed gibson042 closed 1 year ago

gibson042 commented 1 year ago

While working on #2466, I discovered this surprising and inconsistent behavior:

const isoCal = Temporal.Calendar.from("iso8601");
isoCal.monthDayFromFields({ monthCode: "M02", day: 29, year: 2023 }, { overflow: "reject" });
// => Temporal.PlainMonthDay <02-29>
isoCal.monthDayFromFields({ month: 2, day: 29, year: 2023 }, { overflow: "reject" });
// => RangeError (value out of range: 1 <= 29 <= 28)

Since 2023 is not a leap year, both cases should result in a RangeError.

I'm expecting to fix this along with #2466, but it still merits a distinct issue.

justingrant commented 1 year ago

We may choose to change this behavior. But I think the current behavior may actually be consistent with other parts of Temporal, which also follow the following rules:

For example, the following code also doesn't throw:

Temporal.PlainTime.from({hour: 12, year: 2023, month: 2, day: 29})

What makes PlainMonthDay a bit weird is that "fields that contribute to the result" can vary more than other types. If there's a monthCode present in the input, then monthCode and day are sufficient to calculate the result. But if there's no monthCode, then month can mean a different real-world month in lunisolar calendars if the month is later in the year than a leap month.

We could have designed the algorithm to only require a year for only lunisolar calendars, or even more narrowly only in lunisolar calendars where month is larger than the earliest possible leap month. But we chose to design it more restrictively so that code written for one calendar was more likely to work with code written for another calendar.

I wouldn't object to also validating the full date whether or not monthCode is present, because prohibiting having an invalid date doesn't seem like it will hurt any valid use cases. But I also don't think we must make this change.