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

How should LocalDateTime expose its Absolute, TimeZone, Calendar, and (calculated) DateTime? #742

Closed
justingrant opened this issue Jul 8, 2020 · 2 comments
Labels
zoned-type Part of the effort for a type with timestamp+timezone

Comments

@justingrant
Copy link
Collaborator

How should LocalDateTIme store and expose the Absolute, TimeZone, Calendar, and (calculated) DateTime instances that logically make up each LocalDateTime instance?

There are four basic questions:

  1. Should the user-facing syntax be .foo or .toFoo()? The former is preferred as it's better ergonomics and aligns with getFields(), from, and with which accept absolute, calendar, and timeZone properties (and maybe dateTime too, per Easier way to combine multiple Temporal objects in one from or with call ? #720).
  2. Should these values be stored as objects? Or as three primitives (epochNanoseconds, calendarId, and timeZoneId) and re-create types as needed during each method call? Or as the union of primitives of all 4 types?
  3. Should these props be stored in slots or own data properties?
  4. Should any of the answers above vary for DateTime, given that DateTime can be a calculated value? Note that if we do store a DateTime instance, then we won't need to store a Calendar too, because we can just delegate the calendar storage and retrieval to the DateTime instance.

Some answers above imply other answers. For example, if a .foo syntax is used then the answer to (2) is "store objects" because we'd want to retain === identity across calls to the same property.

Answering these questions is probably dependent on @ptomato's research on how to handle the calendar property of Date, DateTime, etc. which brings up many of the same issues.

@justingrant
Copy link
Collaborator Author

Tentative decisions from 2020-07-31 meeting:

  1. timeZone and calendar will stay as property getters, will continue being returned by getFields, and will continue being accepted by from and with.
  2. toDateTime will stay a function.
  3. absolute getter will change to toAbsolute function, and absolute will not be accepted anymore by from and with.

Before making the changes in (3), I'll review the cookbook use of LocalDateTime to make sure that this change won't break any important use cases.

@justingrant
Copy link
Collaborator Author

I reviewed cookbook samples and none of them were problematic for making the .absolute->toAsbolute() change. Change is checked in here: a39c10f.

Closing this issue as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
zoned-type Part of the effort for a type with timestamp+timezone
Projects
None yet
Development

No branches or pull requests

1 participant