Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CalendarMonthDayToISOReferenceDate: Require "eraYear + year" and "month + monthCode" to be consistent? #2864

Open
anba opened this issue May 27, 2024 · 5 comments · May be fixed by #2940
Open
Milestone

Comments

@anba
Copy link
Contributor

anba commented May 27, 2024

It'd be good to have more detailed information when built-in calendars need to check for consistent inputs:

// Which of these are errors?

// eraYear and year inconsistent when month is present.
Temporal.PlainMonthDay.from({calendar: "gregory", era: "ce", eraYear: 2023, year: 2022, month: 2, day: 30});

// eraYear and year inconsistent when monthCode is present.
Temporal.PlainMonthDay.from({calendar: "gregory", era: "ce", eraYear: 2023, year: 2022, monthCode: "M02", day: 30});

// month and monthCode inconsistent.
Temporal.PlainMonthDay.from({calendar: "gregory", month: 3, monthCode: "M02", day: 30});
@justingrant
Copy link
Collaborator

Meeting 2024-05-30: @gibson042 will PR a change to align behavior of non-ISO calendars to match behavior of ISO calendar, which would throw if month/monthCode or year/era/eraYear are present but inconsistent.

@ptomato
Copy link
Collaborator

ptomato commented Sep 9, 2024

@gibson042 Will you have time to pick this up in the near future or should I pick it up?

@ptomato ptomato added this to the Stage "3.5" milestone Sep 9, 2024
gibson042 added a commit to gibson042/proposal-temporal that referenced this issue Sep 16, 2024
…levant) monthCode vs. month in non-ISO calendars

Fixes tc39#2864
@gibson042
Copy link
Collaborator

gibson042 commented Sep 16, 2024

I think this is already covered in the spec: CalendarResolveFields (emphasis mine)

The operation throws a TypeError exception if the fields of fields are internally inconsistent within the calendar or insufficient to identify a unique instance of type in the calendar. For example:

  • If type is DATE or MONTH-DAY and DAY in the calendar has an interpretation analogous to ISO 8601 and fields.[[Day]] is UNSET.
  • If MONTH and MONTH-CODE in the calendar have interpretations analogous to ISO 8601 and either the corresponding values for both are UNSET or neither value is UNSET but they do not identify the same month.
  • If type is MONTH-DAY and fields.[[MonthCode]] is UNSET and a year cannot be determined from fields.
  • If type is DATE or YEAR-MONTH and the calendar supports the usual partitioning of years into eras with their own year counting as represented by YEAR, ERA, and ERA-YEAR (as in the Gregorian or traditional Japanese calendars) and any of the following cases apply:
    • Each of fields.[[Year]], fields.[[Era]], and fields.[[EraYear]] is UNSET.
    • fields.[[Era]] is UNSET but fields.[[EraYear]] is not.
    • fields.[[EraYear]] is UNSET but fields.[[Era]] is not.
    • None of the three values are UNSET but fields.[[Era]] and fields.[[EraYear]] do not together identify the same year as fields.[[Year]].

But I've updated the polyfill accordingly in #2940. It's code that I haven't touched before, so I'd appreciate a thorough review for correctness and coverage.

@anba
Copy link
Contributor Author

anba commented Sep 17, 2024

This part:

If type is DATE or YEAR-MONTH [...]

doesn't apply to the example, because Temporal.PlainMonthDay calls CalendarResolveFields with type = MONTH-DAY.

@gibson042
Copy link
Collaborator

Updated #2940 to correct that.

gibson042 added a commit to gibson042/proposal-temporal that referenced this issue Sep 18, 2024
…levant) monthCode vs. month in non-ISO calendars

Fixes tc39#2864
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants