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

Possibly combine (Instant + ZonedDateTime => Absolute) #156

Closed
pipobscure opened this issue Sep 12, 2019 · 6 comments
Closed

Possibly combine (Instant + ZonedDateTime => Absolute) #156

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

Comments

@pipobscure
Copy link
Collaborator

I've been thinking hard about the number of classes we have and whether all of them are strictly necessary. Part of this was an exploration of whether there is any real value in having distinct classes for Instant and ZonedDateTime.

I've explored what this would mean in this branch of the polyfill and actually quite like it.

The rationale for having Instant was to have a clear mental model and distinction between absolute timeline things and just day/month/year/hour/min/sec/...

However that left ZonedDateTime being an unholy exception to everything. By opting for combining them, it becomes a bit simpler. And appears a bit more user-friendly, while still giving the full-spectrum of capabilities.

This is to gather opinions to check if this is a direction of thought worth continuing down. So please throw all the arguments at me for keeping them separate!

@gibson042
Copy link
Collaborator

gibson042 commented Sep 12, 2019

Some people argued against me pretty strongly when I made a similar suggestion. How do you propose addressing the serialization variance ("2019-09-12T21:19:00.000000000Z" for Instant vs. "2019-09-12T21:19:00.000000000+00:00[Etc/UTC]" for ZonedInstant)? And how does everyone feel about val.plus({days: 1}) being fundamentally different from val.plus({seconds: 86400}) (the former employing discrete-element arithmetic and the latter employing epoch arithmetic, with the results diverging at UTC offset transitions), even though dateTime.plus({days: 1}) is fundamentally the same as dateTime.plus({seconds: 86400})?

@gibson042 gibson042 added behavior Relating to behavior defined in the proposal question labels Sep 12, 2019
@pipobscure
Copy link
Collaborator Author

Etc/UTC isn’t actually real. UTC alone is canonical.

I’m thinking:

UTC serialises with Z
Iana serialises with +00:00[Region/Place]
Offset serialises as just +00:00

as for plus/minus Absolute adds time as relative to the absolute timeline; so that adding P86499S over a DST change would NOT be the same as adding P1D which is exactly the point. It is also why we wanted a combined type to begin with.

@icambron
Copy link

This is Luxon's model and it works quite well. The main downsides seem to be:

  • if you don't care how the time is expressed in some zone, you have all this extra stuff hanging around, cramping your mental model
  • something somewhere needs to decide what zone each instance is in, even if no one cares

Neither of these are very compelling disadvantages, IMO, and just amount to some extra properties you might not need.

And in particular, I think it would be super weird if plus({days: 1}) were available in Instant but just added 24 hours because it doesn't know how else to do it. The thing about days is that in IRL they're just flat-out not all the same length, and it will just cause confusion and pain to pretend that they are. It's just an operation that requires knowing the zone.

@gibson042
Copy link
Collaborator

You've got it backwards, UTC is an alias for the canonical Etc/UTC.

And I'm not comparing plus between Absolute and Instant (which is subsumed anyway), I'm comparing between Absolute and DateTime. The latter supports element-based arithmetic at all granularities, but the former is using epoch arithmetic for at least hours and smaller units... but what will it do with bigger ones? Rejecting is weird because it's supposedly a zone-enhanced DateTime, but mixing is maybe even weirder. Likewise for difference as well. I guess this problem was going to come up for ZonedDateTime too, though. Straddling the fence isn't really possible past a certain point.

@pipobscure
Copy link
Collaborator Author

pipobscure commented Sep 13, 2019

I got you right, but explained badly:

The process in Absolute for plus is to add the time bits directly as nanosecondsEpoch, since that’s a stable relationship; then add the date bits to the resultant datetime and calculate the nanosecondEpoch.

This way adding time bits respects DST changes while DateTime does not.

https://github.com/std-proposal/temporal/blob/absolute/lib/absolute.mjs#L113

@pipobscure
Copy link
Collaborator Author

Conclusions:

I've spent a lot of time talking to people about this and getting back to some of the people that suggested this. It turns out this isn't really a good idea and makes things a lot more confusing.

Also what was suggested in #139 isn't this either, on the contrary, this makes things worse.

So while the experiment / thought process was fully worth it in terms of exploring, this is NOT the way we should take things.

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

3 participants