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

Design of Temporal.Calendar and Temporal.TimeZone classes #300

Closed
littledan opened this issue Dec 11, 2019 · 7 comments
Closed

Design of Temporal.Calendar and Temporal.TimeZone classes #300

littledan opened this issue Dec 11, 2019 · 7 comments
Labels
calendar Part of the effort for Temporal Calendar API

Comments

@littledan
Copy link
Member

In other issues and documents, we've been talking about protocols for Temporal.Calendar and Temporal.TimeZone, so users could put in their own behavior for these. I didn't understand it at first, but the design is a really clean separation: ownership of time-related calculations simply moves from Temporal.Time and Temporal.DateTime to the timezone objects, and similarly, date-related calculations move from Temporal.Date to Temporal.DateTime.

In this issue, I want to talk about how built-in instances work. How, when you pass "America/New_York" to Temporal.DateTime.prototype.inTimeZone, or { calendar: "gregory" } as part of the parameter to Temporal.DateTime.from, this turns into an object which respects the Temporal.TimeZone or Temporal.Calendar protocol.

The rest of post describes details for Temporal.Calendar, just because there's more written up about the protocol and less about the class. I suspect that the same sorts of logic will make sense for Temporal.TimeZone as well. Some people have alluded to alternatives, but I can't figure out what they would be concretely, so I'm just focusing on my ideas to start things off. I encourage you to post any other ideas to this thread for discussion.


The Temporal Calendar protocol is described here. One class whose instances conform to this protocol is Temporal.Calendar. This class is used for both the "iso" calendar and calendars defined in ECMA-402.

Instances have a [[CalendarName]] internal slot, which is a string like "iso" or "chinese". All methods (such as Temporal.Calendar.prototype[Temporal.Calendar.plus]) start by checking for the presence of an [[InitializedTemporalCalendar]] internal slot, throwing a TypeError otherwise, and if it's fine, using the [[CalendarName]] internal slot by doing the appropriate calculation.

The Temporal Calendar protocol operates by simply calling methods; there is no reference to the [[CalendarName]] internal slot by users of the Temporal Calendar protocol (such as Temporal.Date.prototype.plus). The internal slot is only referenced by the methods within Temporal.Calendar.prototype.

The layering of ECMA-402 is explained by the definitions of the methods and constructor for Calendar to be patched/replaced by ECMA-402; there's separate class for them. This is analogous to the toLocaleString() methods.

The Temporal.Calendar constructor (which requires explicit new) takes a string as an argument and looks up if there's a built-in calendar with this name (using the same logic as Intl with respect to normalization). If there is, it constructs a fresh Temporal.Calendar instance with the [[CalendarName]] internal slot set appropriately. If no calendar is found, then a RangeError is thrown. The Temporal.Calendar.from method will call new Temporal.Calendar if passed a string, return the object if passed an object, and throw a TypeError otherwise.

Specification-wise, when creating an object that has a calendar (such as a Temporal.Date instance), a new instance is created immediately, as if by Temporal.Calendar.from. In the implementation, though, if a string is used, then the calendar could unobservably be stored in the Temporal.Date as its corresponding string, with the allocation delayed until someone actually accesses the calendar with the get Temporal.Date.prototype.calendar accessor (and then, in that case, cached in place).

@ljharb
Copy link
Member

ljharb commented Dec 12, 2019

This seems right to me; it enables non-subclasses to participate in the protocol, but the "happy path" is to subclass the builtin objects, and override methods.

This assumes that class extends Temporal.Calendar would have its methods called directly, so it could shadow the builtin ones - if not, then it seems like this design wouldn't be subclassable. Can you elaborate?

@littledan
Copy link
Member Author

@ljharb The proposed Temporal Calendar protocol is based on calling the methods, not the built-in ones. So a subclass of Temporal.Calendar could shadow built-in methods, and do super method calls if it wants to. Is this what you mean?

@ljharb
Copy link
Member

ljharb commented Dec 13, 2019

Yes, exactly right - but if they didnt shadow them they’d hit the builtin brand-checking ones.

@ptomato ptomato added the calendar Part of the effort for Temporal Calendar API label Jan 22, 2020
@gibson042
Copy link
Collaborator

Carrying forward Temporal.TimeZone-specific requirements,

time zone objects consist fundamentally of a collection of rules describing when and how local time transitions between (UTC) offsets. Although a time zone can be looked up from a common database by name, it's not a name—it's a data structure, and constructing a time zone from scratch is an important use case (cf. getTimeZoneObjectFromRules, among others).

Some of those other use cases include denying or overriding lookup of host-provided time zones.

@kaizhu256

This comment has been minimized.

@kaizhu256

This comment has been minimized.

@littledan
Copy link
Member Author

Closing this per the design documents that we've agreed on which capture it well, e.g., #487 and #498, which @ptomato is implementing.

ptomato added a commit that referenced this issue May 29, 2020
The other half of the custom time zones and calendars protocol discussed
in #300 was allowing calendars and time zones to be plain objects with
the appropriate methods on them.

This adds a test verifying that it's possible to use a plain object with
getOffsetNanosecondsFor(), getPossibleAbsolutesFor(), and toString() as
a time zone, and makes the changes needed to allow that: removing brand
checks from time zone methods, and calling methods on
Temporal.TimeZone.prototype if they are not present on the plain object.
ptomato added a commit that referenced this issue Jun 1, 2020
The other half of the custom time zones and calendars protocol discussed
in #300 was allowing calendars and time zones to be plain objects with
the appropriate methods on them.

This adds a test verifying that it's possible to use a plain object with
getOffsetNanosecondsFor(), getPossibleAbsolutesFor(), and toString() as
a time zone, and makes the changes needed to allow that: removing brand
checks from time zone methods, and calling methods on
Temporal.TimeZone.prototype if they are not present on the plain object.
ptomato added a commit that referenced this issue Jun 1, 2020
The other half of the custom time zones and calendars protocol discussed
in #300 was allowing calendars and time zones to be plain objects with
the appropriate methods on them.

This adds a test verifying that it's possible to use a plain object with
getOffsetNanosecondsFor(), getPossibleAbsolutesFor(), and toString() as
a time zone, and makes the changes needed to allow that: removing brand
checks from time zone methods, and calling methods on
Temporal.TimeZone.prototype if they are not present on the plain object.
ptomato added a commit that referenced this issue Jun 1, 2020
The other half of the custom time zones and calendars protocol discussed
in #300 was allowing calendars and time zones to be plain objects with
the appropriate methods on them.

This adds a test verifying that it's possible to use a plain object with
getOffsetNanosecondsFor(), getPossibleAbsolutesFor(), and toString() as
a time zone, and makes the changes needed to allow that: removing brand
checks from time zone methods, and calling methods on
Temporal.TimeZone.prototype if they are not present on the plain object.
ptomato added a commit that referenced this issue Jun 5, 2020
The other half of the custom time zones and calendars protocol discussed
in #300 was allowing calendars and time zones to be plain objects with
the appropriate methods on them.

This adds a test verifying that it's possible to use a plain object with
calendar methods as a time zone, and makes the changes needed to allow
that.
ptomato added a commit that referenced this issue Jun 5, 2020
FIXME: Test fails passing custom time zone ID in to Intl
FIXME: Need to remove ES.ToTemporalTimeZone and use
  Temporal.TimeZone.from throughout

The other half of the custom time zones and calendars protocol discussed
in #300 was allowing calendars and time zones to be plain objects with
the appropriate methods on them.

This adds a test verifying that it's possible to use a plain object with
getOffsetNanosecondsFor(), getPossibleAbsolutesFor(), and toString() as
a time zone, and makes the changes needed to allow that: removing brand
checks from time zone methods, and calling methods on
Temporal.TimeZone.prototype if they are not present on the plain object.
ptomato added a commit that referenced this issue Jun 8, 2020
The other half of the custom time zones and calendars protocol discussed
in #300 was allowing calendars and time zones to be plain objects with
the appropriate methods on them.

This adds a test verifying that it's possible to use a plain object with
getOffsetNanosecondsFor(), getPossibleAbsolutesFor(), and toString() as
a time zone, and makes the changes needed to allow that: removing brand
checks from time zone methods, and calling methods on
Temporal.TimeZone.prototype if they are not present on the plain object.
ptomato added a commit that referenced this issue Jun 9, 2020
The other half of the custom time zones and calendars protocol discussed
in #300 was allowing calendars and time zones to be plain objects with
the appropriate methods on them.

This adds a test verifying that it's possible to use a plain object with
calendar methods as a time zone, and makes the changes needed to allow
that.
ryzokuken pushed a commit that referenced this issue Jun 9, 2020
The other half of the custom time zones and calendars protocol discussed
in #300 was allowing calendars and time zones to be plain objects with
the appropriate methods on them.

This adds a test verifying that it's possible to use a plain object with
getOffsetNanosecondsFor(), getPossibleAbsolutesFor(), and toString() as
a time zone, and makes the changes needed to allow that: removing brand
checks from time zone methods, and calling methods on
Temporal.TimeZone.prototype if they are not present on the plain object.
ptomato added a commit that referenced this issue Jun 9, 2020
The other half of the custom time zones and calendars protocol discussed
in #300 was allowing calendars and time zones to be plain objects with
the appropriate methods on them.

This adds a test verifying that it's possible to use a plain object with
calendar methods as a time zone, and makes the changes needed to allow
that.
ptomato added a commit that referenced this issue Jun 10, 2020
The other half of the custom time zones and calendars protocol discussed
in #300 was allowing calendars and time zones to be plain objects with
the appropriate methods on them.

This adds a test verifying that it's possible to use a plain object with
calendar methods as a time zone, and makes the changes needed to allow
that.
ptomato added a commit that referenced this issue Jun 12, 2020
The other half of the custom time zones and calendars protocol discussed
in #300 was allowing calendars and time zones to be plain objects with
the appropriate methods on them.

This adds a test verifying that it's possible to use a plain object with
calendar methods as a time zone, and makes the changes needed to allow
that.
ptomato added a commit that referenced this issue Nov 15, 2020
We had originally decided to go with a scheme where the value of the
[[Identifier]] internal slot determines the behaviour of the calendar
methods, and there are not individual parent classes for each built-in
calendar. This implements that scheme.

Merges most of the spec text of ISO8601Calendar into Calendar.

Closes: #847
See: #300
ptomato added a commit that referenced this issue Nov 16, 2020
This makes a small change to the constructor of Temporal.TimeZone so that
it doesn't accept any arbitrary ID, only a recognized built-in one, to
bring it in line with calendars.

See: #847
See: #300
ptomato added a commit that referenced this issue Nov 16, 2020
We had originally decided to go with a scheme where the value of the
[[Identifier]] internal slot determines the behaviour of the calendar
methods, and there are not individual parent classes for each built-in
calendar. This implements that scheme.

Merges most of the spec text of ISO8601Calendar into Calendar.

Closes: #847
See: #300
ptomato added a commit that referenced this issue Nov 16, 2020
This makes a small change to the constructor of Temporal.TimeZone so that
it doesn't accept any arbitrary ID, only a recognized built-in one, to
bring it in line with calendars.

See: #847
See: #300
ptomato added a commit that referenced this issue Nov 16, 2020
We had originally decided to go with a scheme where the value of the
[[Identifier]] internal slot determines the behaviour of the calendar
methods, and there are not individual parent classes for each built-in
calendar. This implements that scheme.

Merges most of the spec text of ISO8601Calendar into Calendar.

Closes: #847
See: #300
ptomato added a commit that referenced this issue Nov 16, 2020
This makes a small change to the constructor of Temporal.TimeZone so that
it doesn't accept any arbitrary ID, only a recognized built-in one, to
bring it in line with calendars.

See: #847
See: #300
ptomato added a commit that referenced this issue Nov 16, 2020
We had originally decided to go with a scheme where the value of the
[[Identifier]] internal slot determines the behaviour of the calendar
methods, and there are not individual parent classes for each built-in
calendar. This implements that scheme.

Merges most of the spec text of ISO8601Calendar into Calendar.

Closes: #847
See: #300
ptomato added a commit that referenced this issue Nov 16, 2020
This makes a small change to the constructor of Temporal.TimeZone so that
it doesn't accept any arbitrary ID, only a recognized built-in one, to
bring it in line with calendars.

See: #847
See: #300
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
calendar Part of the effort for Temporal Calendar API
Projects
None yet
Development

No branches or pull requests

6 participants