tc39 / proposal-temporal

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

size-suggestion: Remove `Temporal.Calendar` class and protocol #2854

Open justingrant opened 2 months ago

justingrant commented 2 months ago

This issue was split from #2826. This issue is solely focused on removing the Calendar class and protocol. Please keep discussions on-topic.

Conclusion in the champions meeting of 2024-04-18. We've been asked to reduce the size and complexity of the proposal. The callable hooks in the time zone and calendar protocols are the part that implementations have been most uncomfortable with. They also seem to be where we get the biggest reduction in complexity for the least loss of use cases. The TimeZone and Calendar classes themselves make less sense to keep if the protocols are gone. So our conclusion is to remove them. This issue is focused on calendars only. A separate issue #2853 discusses time zones.

Motivations for removing custom calendars

Use cases that disappear, and their replacements

Work around incomplete/outdated support in CLDR calendars

For example:

Replacement: Previously, you could implement a custom calendar by creating an object that implemented the protocol, and monkeypatching or wrapping some of Temporal so that it would deserialize dates with your custom calendar ID into a Temporal object with the correct protocol object. You would now have to monkeypatch or wrap more of Temporal. As part of moving this issue forward, we'll create a proof of concept for doing this, to make sure that it remains possible.

Scope of issue

ptomato commented 2 months ago

While working on the test262 tests for this, I realized that if we only have builtin calendars, it would be possible to remove the 4th "reference ISO day" argument from the PlainYearMonth constructor, and the 4th "reference ISO year" argument from the PlainMonthDay constructors.

So Temporal.PlainYearMonth.from({year: 5780, month: 1, day: 1, calendar: 'hebrew'}) would still have the ISO year, month, and reference day of 2019-09-30. However, you would no longer be able to create that object using new Temporal.PlainYearMonth(2019, 9, 'hebrew', 30); but you would also no longer be able to create an object with its internal slots in an inconsistent state, using new Temporal.PlainYearMonth(2019, 9, 'hebrew', 22). (We might want to just forbid non-ISO calendars in the PlainYearMonth and PlainMonthDay constructors altogether.)

These 4th arguments have always been a bit weird, as we have to basically say in the documentation "Don't use this argument, it's only for custom calendar implementations." Removing it would be a minor win for learnability/comprehensibility.

On the other hand, you'd have a piece of the data model (reference day / year) which was not directly settable in the constructor. (You'd still be able to read the value using .getISOFields().year or .getISOFields().day.) Not sure if that would be an obstacle. FWIW, if there are no custom calendars then I don't see any benefit in allowing the creation of objects with arbitrary and possibly inconsistent reference days / years.

Any thoughts on this? I'm fairly neutral on it.

khawarizmus commented 2 months ago

How would Hijri adjustment days work with and without custom calendars? I am curious as I will eventually be dealing with that use case.

As for the second use case. It's very important when dealing with rrules and calendar information. But I am curious to see what the proposed solution would be to have a better understanding of the pros and cons as well as the limitations.

As long as there is a garentee that the functionality would still be achievable it might not matter much at the end.

With the current state of the polyfill I managed to build an rrule library (not open sourced yet) using Temporal and custom calendars allows me to support custom week starts and non-Gregorian recurrence rules. My library is currently fully compliant with RFC 5545 and RFC 7529.

I am sharing this to give the champions an idea of how custom calendars would be used.

justingrant commented 2 months ago

if we only have builtin calendars, it would be possible to remove the 4th "reference ISO day" argument from the PlainYearMonth constructor, and the 4th "reference ISO year" argument from the PlainMonthDay constructors.

@ptomato If, in a later proposal, we go with the "eagerly calculate properties" (#2852) replacement for the current custom calendar API, then could the ISO reference day or year be supplied by the calendar inside the PMD/PYM constructor execution? If so, then we'd never need to add those constructor arguments back.

ptomato commented 2 months ago

@khawarizmus I'm fairly confident that even if it's not possible to do Hijri adjustment days in the form of a pluggable calendar that you can pass to the PlainDate constructor, it'll still be possible to achieve the same functionality. I'd like to learn more about this use case. The way I imagine it'd work would be that you'd create a class something like

class HijriAdjustmentPlainDate {
  #underlyingPlainDate;
}

with a Temporal.PlainDate used internally to implement all the operations while applying the adjustment days (which I don't know the data model of, so that's what I'd like to learn more about.)

I'd also like to learn more about the other use case. If it would be possible to share your library (even privately) that would be really helpful.

ptomato commented 2 months ago

@justingrant Unfortunately no. (And the more I think about this, the more I think we should probably not remove the 4th argument; it's weird either way and it's probably not worth it to swap one kind of weirdness for the other.) It's because the constructor arguments are ISO.

Taking PlainYearMonth as an example, if you did new Temporal.PlainYearMonth(2025, 1, 'hebrew') — this could mean either 5785-04 (starts 2025-01-01 in ISO) or 5785-05 (starts 2025-01-30 in ISO). So you could just never use the constructor for non-ISO calendars if we didn't have the 4th argument. Or, we'd have to redesign the constructor args to be the year and month numbers in the non-ISO calendar, which would be difficult from a developer education point of view. (And I'd consider a redesign out of scope of V1.)

FrankYFTang commented 2 months ago

Shu-yu, Shane and I have an internal meeting to sync our position. I agree with Shu-yu that as long as we still allow the implementation to support the calendar the Intl.DateTimeFormat supported via a build-in calendar route, I am fine we strip off the 'user defined" calendar support from Temporal.

justingrant commented 2 months ago

Meeting 2024-05-23: At the upcoming June 2024 TC39 plenary meeting, we'll propose removing the Temporal.Calendar class and protocol. This removes 33 functions from Temporal (more than 10% of Temporal's total surface area!), removes a lot of complexity from the spec and from implementations, and removes the need for over 1000 Test262 tests.

Also, removing this expensive design now enables a follow-up proposal to implement custom calendars in a simpler and more space-efficient way, for example @FrankYFTang's idea in #2852 to compute properties in a single user-code call or to handle object construction using fewer methods like what's proposed in #2851. These ideas are very promising, but changes that large are well beyond the scope of Temporal V1.

khawarizmus commented 2 months ago

@ptomato For the first use case, the available CLDR Hijri calendars are typically used for civil and administrative purposes, aiding in long-term planning and day-to-day scheduling. However, many Muslim countries practice moon sighting for religious purposes, which can cause Hijri dates to shift by a day or two (up to a maximum of three days if I am not mistaken). Since moon sighting is region-based, there is no one-size-fits-all solution. Therefore, it's essential to accommodate this variability when building systems that handle Hijri calendars.

There are two ways to address this:

  1. User-Managed Adjustments:
    • Allow users to manually adjust their calendars by adding or subtracting a specific number of days.
  2. Centralized Moon Sighting Database:
    • Develop a central database that tracks moon sighting across different regions and sync with it daily or at month edges (to my knowledge there in no standard implementation of this similar to TZDB and while this is something I am interested to solve in a standardised manner at some point, implementers can now have a proprietary system that stores some of this data and update periodically)

A custom calendar would help with these adjustments from the perspective of centralising the logic where you would initiate that calendar once and use it with all the Temporal objects (PlainTime, PlainDateTime, ZonedDateTime) across the code base. otherwise you would be using .add() or substract() everywhere or wrapping your Temporal objects as you shared above although I would prefer the wrapper to only contain adjustment logic and generate Temporal objects as needed with the correct adjustments.

As for the second use case. I have given you (write) access to the library I am working on and I will reach out to chat about it a bit. but you can check the rrule part of the code and see the tests we have. It is worth noting that the custom calendar logic used actually comes from the week-dates library that I open-sourced and there I have a PlainWeekDate object that supports on custom calendars to do custom week starts for both ISO and Hijri week calendar (HWC)

kabaros commented 1 month ago

trying to better understand the impact of this change - With this change, implementing a custom calendar like we did for Nepali calendar here will not be possible once Temporal.Calendar is removed, right? Will there be a different way to implement such a calendar?

As an aside - how do implementers decide which calendars to support typically (and why Nepali for example is not one of them)?

ptomato commented 1 month ago

@kabaros It won't be possible to create a PlainDate object that transparently uses the Nepali calendar as if it was a built-in calendar, but it will still be possible to achieve the same result; just a bit less convenient because you will have to define a Nepali-specific PlainDate class that extends PlainDate.

It's on my to-do list to prepare a proof of concept for this. I haven't gotten around to it yet, but the principle would be similar to the custom time zone example I'm working on in PR #2894.

Longer-term, your best course of action (regardless of whether Temporal would support custom calendar objects or not) is to work to get the Nepali calendar supported in the Common Locale Data Repository (CLDR). JS implementations support exactly the calendars that CLDR supports. If the calendar is supported in CLDR, you'll also get proper localization, which would not have been possible with Temporal's custom calendars.

I don't know why the Nepali calendar is not included in CLDR, but I'm sure it's not from lack of will. More likely they have just never been approached by any experts on the Nepali calendar and would love to work with you on it. @sffc can tell you more about this process. (This is also what we advised @khawarizmus about the week numbering for Hijri calendars.)

sffc commented 1 month ago

There are CLDR tracking issues to add additional south Asian calendars:

You could add one for Nepalese.

These types of contributions would be welcome into CLDR/Unicode, and by doing so, not only do you get the calendar in Temporal, you also get it in Intl.DateTimeFormat, and you also get it everywhere else CLDR is used, including Android, iOS, Windows, and most other platforms.