Polyfill: Update per #2500

[gibson042]

This PR removes some dead steps from PlainMonthDay-related spec algorithms to hopefully clarify that year is optional only for the ISO 8601 calendar—and that in that calendar it is always optional (regardless of entry point or presence vs. absence of month and/or monthCode)—and then updates the polyfill accordingly, fixing a gap from #2500 that supported Temporal.PlainMonthDay.from({ monthCode, day }) and Temporal.PlainMonthDay.from({ month, day }) but not Temporal.PlainMonthDay.from({ month, day, calendar: "iso8601" }) or Temporal.Calendar.from("iso8601").monthDayFromFields({ month, day })).

Note that polyfilled non–ISO-8601 calendars, including "gregory", continue to reject input that fails to include at least one of year and monthCode.

Related test262 PR: tc39/test262#3899

Fixes #2664

[codecov]

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (ee2ddeb) 96.30% compared to head (a58a0f4) 96.35%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2663      +/-   ##
==========================================
+ Coverage   96.30%   96.35%   +0.05%     
==========================================
  Files          20       20              
  Lines       12204    12196       -8     
  Branches     2276     2274       -2     
==========================================
- Hits        11753    11752       -1     
+ Misses        393      389       -4     
+ Partials       58       55       -3     

Files	Coverage Δ
polyfill/lib/calendar.mjs	87.30% <100.00%> (+0.12%)	⬆️
polyfill/lib/ecmascript.mjs	98.56% <25.00%> (+0.06%)	⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[ptomato]

OK to merge when the review comments on the tests are addressed. Thanks for following up!

[justingrant]

My naive memory of this (happy to be corrected) was that the deviations here were intentional, because we wanted the shape of arguments (not their values) to determine the exceptions thrown.

Temporal.PlainMonthDay.from({ month, day }) - because there's no calendar, we know by the shape that it must be ISO8601 calendar, so we don't need to throw.

Temporal.PlainMonthDay.from({ month, day, calendar }) - because there's a calendar, we throw because we don't want a data-driven exception. monthCode or year (or era/eraYear) is required

@sffc - do you remember our decisions here, and does the above represent those decisions correctly?

[sffc]

I believe what @justingrant said is correct.

The behavior should be:

// Success: numeric month and day explicitly in an options bag means ISO-8601
Temporal.PlainMonthDay.from({ month: 8, day: 22 })

// Reject: calendar with numeric month and day (no data-driven exception)
Temporal.PlainMonthDay.from({ month: 8, day: 22, calendar: "iso8601" })
Temporal.PlainMonthDay.from({ month: 8, day: 22, calendar: "buddhist" })

// Success: monthCode, day, and calendar all explicit
Temporal.PlainMonthDay.from({ monthCode: "M08", day: 22 })
Temporal.PlainMonthDay.from({ monthCode: "M08", day: 22, calendar: "iso8601" })
Temporal.PlainMonthDay.from({ monthCode: "M08", day: 22, calendar: "buddhist" })

This behavior is what is currently in the polyfill.

[justingrant]

I'm not sure that this is a change we want to make. See #2663 (comment)

[gibson042]

The current state of the polyfill is inconsistent with both the pre-#2500 spec and the post-#2500 spec, so that definitely needs updating. As for the spec, it is content to accept month without year or monthCode for the ISO 8601 calendar whether implicit (e.g., Temporal.PlainMonthDay.from(`${zeroPad(month, 2)}-${zeroPad(day, 2)}`) or Temporal.PlainMonthDay.from({ month, day })) or explicit (e.g., Temporal.PlainMonthDay.from(`${zeroPad(month, 2)}-${zeroPad(day, 2)}[u-ca=iso8601]`) or Temporal.PlainMonthDay.from({ month, day, calendar: "iso8601" })), although the latter of those explicit examples was just merged in #2500 and per this conversation might be reverted.

But on that point, the previous "only shape matters" state was inconsistent—not just in the trivial sense of e.g. "M13" being an acceptable monthCode in some calendars but not others, but more relevantly because whether a month without year or monthCode is acceptable is data-dependent in both string form (e.g., "08-23[u-ca=iso8601]" is accepted but "08-23[u-ca=gregory]" is not) and more importantly in object form (e.g., even pre-#2500, Temporal.PlainMonthDay.from({ month, day, calendar }) was acceptable with a non-string calendar, although the built-in Temporal.Calendar.prototype.monthDayFromFields bizarrely rejected { month, day } even in the ISO 8601 calendar).

So if we truly want a normative change that restores rejection of Temporal.PlainMonthDay.from({ month, day, calendar: "iso8601" }), then it would seem to be appropriate in ToTemporalMonthDay rather than in Temporal.Calendar.prototype.monthDayFromFields—and IMO should extend rather than block/replace this PR and tc39/test262#3899 . I've opened #2664 for that discussion.

[justingrant]

Looks good! Thanks so much for your patience while we collectively figured out the right behavior here.

[justingrant]

I assume tests are coming? Also needs a rebase and autosquash.

[gibson042]

I assume tests are coming?

Related test262 PR: tc39/test262#3899

Also needs a rebase and autosquash.

Done.

[ptomato]

Good to go, when the tests are.

[ptomato]

Don't know if we want this, but maybe:

Suggested change

When _type_ is ~month-day~ and *"month"* is provided without *"monthCode"*, it is recommended that all built-in calendars other than the ISO 8601 calendar require a disambiguating year (e.g., either *"year"* or *"era"* and *"eraYear"*), including those that always use exactly the same months as the ISO 8601 calendar (which receives special handling in this specification as a default calendar that is permanently stable for automated processing).

When _type_ is ~month-day~ and *"month"* is provided without *"monthCode"*, all built-in calendars other than the ISO 8601 calendar must require a disambiguating year (e.g., either *"year"* or *"era"* and *"eraYear"*), including those that always use exactly the same months as the ISO 8601 calendar (which receives special handling in this specification as a default calendar that is permanently stable for automated processing).

(in order to make it clear that there should not be implementation divergence)

[gibson042]

I definitely don't want to introduce such a strong statement in this PR, and I'm not sure it belongs in the text at all (it's true that built-in calendars are constrained to CLDR, but that source itself can change over time). Even CalendarDateMonthCode, which I consider to be much more strict regarding "M##[L]" formatting, uses "should" rather than "must".

[ptomato]

While working on the tests I found an additional misalignment between the polyfill and the spec. CalendarResolveFields says "The operation throws a TypeError exception if the properties of fields are internally inconsistent within the calendar or insufficient to identify a unique instance of type in the calendar." But the polyfill would throw RangeError in this case.

Normally I'd open a separate PR to correct this, but since it also requires updating the test262 tests which were incorrect, that would cause another sync-and-rebase dance. So for convenience I've just appended the fix to this PR.