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

Eliminate OffsetDateTime & ZonedDateTime #158

Closed
pipobscure opened this issue Sep 25, 2019 · 9 comments
Closed

Eliminate OffsetDateTime & ZonedDateTime #158

pipobscure opened this issue Sep 25, 2019 · 9 comments
Labels
behavior Relating to behavior defined in the proposal question

Comments

@pipobscure
Copy link
Collaborator

Based on #139 and #156 the second thought is to eliminate ZonedDateTime & OffsetDateTime entirely.

I've tried that experiment at https://github.com/std-proposal/temporal/tree/reduced

Pros:

  • Clear conceptual differentiation between absolute point in time and date/time-like things

( @devjgm @devbww can you elaborate on learnings from abseil)

Similar question to before:

Give me all the reasons why ZonedDateTime & OffsetDateTime should actually be a thing

( @gibson042 please comment! )

@pipobscure pipobscure added question behavior Relating to behavior defined in the proposal labels Sep 25, 2019
@devbww
Copy link

devbww commented Sep 25, 2019

I don't think we have much to add beyond what @devjgm said in #139.

One thing I can say is that I don't believe we have ever had a request to add a type beyond our absolute-time, absolute-duration, and time-zone ones (absl::Time, absl::Duration, and absl::TimeZone), and our set and civil-time ones (absl::CivilSecond and friends).

People may have created their own hybrids, I guess, but they have never risen to the level of noticeability.

@jorroll
Copy link

jorroll commented Oct 1, 2019

FWIW, I think this is a good idea. I maintain rSchedule, a typescript recurring events library. Through some trial and error, I arrived at a similar abstraction for implementing the recurrence logic.

When a user provides a date, it is converted into a custom absolute time object (which mostly relies on the standard Date in UTC). All logic is processed using these absolute times. Then, before yielding a date back to the user, the absolute time is re-combined with the original time zone.

The rrulejs recurrence library uses a similar (albeit, more crudely implemented) strategy where the user is required to convert provided dates into an absolute time format (basically, the user needs to handle the conversion rather than the library doing it for them -- but the idea is the same).

Clear conceptual differentiation between absolute point in time and date/time-like things

^ Agreed. This format is much easier to reason over while also avoiding timezone issues.

This decision also leaves open the possibility of adding a ZonedDateTime object to the temporal module in a future update. In the meantime, if a ZonedDateTime object is found to be desirable, users will be able to create their own ZonedDateTime object relatively easily using the tools in the 1.0 temporal module.

@pipobscure
Copy link
Collaborator Author

Just fyi this is what I'm presenting as an update at TC39

@jorroll
Copy link

jorroll commented Oct 1, 2019

Give me all the reasons why ZonedDateTime & OffsetDateTime should actually be a thing

@pipobscure maybe this should be a separate issue, but one area that is unclear when looking at the code for the current polyfill (which has eliminated ZonedDateTime & OffsetDateTime), is what happens when the user attempts to combine a zoneless DateTime with a time zone and the result is invalid? E.g. if a DST shift results in 3am immediately becoming 4am, so 3:30am doesn't exist, what happens if you combine a DateTime at 3:30am with that time zone?

  • Personally, as a library implementor, what I would find most useful is for a special error to be thrown indicating that the current time is invalid because of DST. The error would contain a property indicating when the DST time-skip started (e.g. 3am) and a separate property indicating how much time was skipped (e.g. 1 hour). This would be enough information for a developer to easily handle the issue (e.g. they could prompt the user for a new date, or they could add 1 hour to the time (producing 4:30am), or they could get the next valid time (4am) or the most recently valid time in the past (3am - 1 nanosecond).

@pipobscure
Copy link
Collaborator Author

Depends on how you do it.

TZ.getAbsoluteFor(DT) => []
DT.withZone(TZ) => throws

for the case a datetime exists twice (DST)

TZ.getAbsoluteFor(DT) => [earlier, later]
DT.withZone(TZ, Temporal.EARLIER) => earlier
DT.withZone(TZ, Temporal.LATER) => later
DT.withZone(TZ, offset) => the one with that offset or throw otherwise
default second prameter is Temporal.EARLIER

@devbww
Copy link

devbww commented Oct 1, 2019 via email

@pipobscure
Copy link
Collaborator Author

Thanks! That sounds very much like what we’re doing adjusted for different idioms.

Working with a state flag like that sounds just right for C/C++ but would be weird in JS. But being able to get 0, 1 or 2 results sounds right and exactly like what we’re thinking for TZ. getAbsoluteFor(DT)

For the DT.withZone(TZ) where a single result is expected, throwing seems idiomatic for JS.

@gilmoreorless
Copy link
Contributor

I'm in two minds about this. On one hand, there was a large discussion in #84 about how useful the concept of ZonedDateTime was, so it seems strange that it's now getting dumped. On the other hand, I agree that it's better to have no ZonedDateTime at all, and be able to add it in later, than to lock in a version that people don't agree on.

What I'm most concerned about here, though, is the process by which it was dropped. The removal of 2 API objects was proposed with just a single "pro" and no "cons" as justification, which seems pretty weak from where I sit. Then the burden of proof for keeping them was put on to everyone else. That's the inverse of how it should go.

Unfortunately, many of the large API changes are being done with an almost complete absence of practical examples (something I raised in #26 (comment), and will continue to harp on about). It would be far easier to make decisions about the impacts of a proposed API change if there were before/after code samples for real-world use cases.

@devbww
Copy link

devbww commented May 18, 2020 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
behavior Relating to behavior defined in the proposal question
Projects
None yet
Development

No branches or pull requests

4 participants