Proposal-temporal: Restrict .plus to Civil objects

[pipobscure]

Instant & ZonedInstant have no sensible way to do the arithmetic that I can see.

Instant itself has no way to to add days, since there is no timezone. Assume an Instant that represents a timezone that has different dates depending on the timezone. Then imagine adding enough days to jump a month boundary. The result would be different, depending on which timezone is assumed. So this would be ambiguous.

ZonedInstant is also problematic. Imagine adding enough days to cross a DST boundary. What would the time then be. Presume that we actually store the IANA timezone then the assumption would be that the time would shift according to DST. However if we only have the offset, then there is no way to determine if there was a DST shift or not. Again this would be ambiguous.

So I suggest we drop the plus() method from instants and leave them only in Civil* objects. This way the calculations would be.

instant.withZone(<zone>).toCivilDateTime().plus({}).withZone(<zone>) there is a way to do this explicitly and avoid these ambiguities.

[pipobscure]

@maggiepint @RedSquirrelious for discussion

[ljharb]

Can you provide a concrete example where it would be ambiguous?

[pipobscure]

Assume the instant 2020-02-29T16:00:00.000000000Z

In Britain the calculation instant.plus({ month: 1 }) would result in 2020-03-29T16:00:00.000000000Z
In Sydney that is already 2020-03-01T02:00:00.000000000 so the calculation there would have to yield 2020-04-01T02:00:00.000000000 which in turn is actually several days ahead if expressed as an Instant. So it's ambiguous.

[ljharb]

Is it only ambiguous if there’s a time associated - in other words, with a type that’s just a y-m-d, it would be fine?

[pipobscure]

Excatly and both Instant & ZonedInstant have a time associated with them.

This is not a problem for CivilDateTime since that is always local

[ljharb]

would it not make sense to take the y/m/d parts, do the civil math, and then append the time? Any necessary conversion would be done upon adding a timezone

[maggiepint]

My intent had always been what Jordan said - do Y/M/D as civil math, and then do time, always applying units in descending order. This is fairly un-ambiguous. Instant can work in UTC days, and ZonedInstant can work in the appropriate zone.

[pipobscure]

The intent is fine, but since Instant & ZonedInstant don’t really have a y/m/d part it’s not intuitive. Plus the spec also speaks of adding smaller units in increasing size.

To make things clearer I’d think we should make the conversion to CivilDateTime explicit. That way we don’t have the confusion.

In addition for Instant doing the calculation in UTC would be bad, since it’s explicitly Zone-Free. UTC itself is a Zone though, so that’s not really valid either.

On 20 Jul 2018, at 17:44, Maggie Pint [e-mail] wrote:

My intent had always been what Jordan said - do Y/M/D as civil math, and then do time, always applying units in descending order. This is fairly un-ambiguous. Instant can work in UTC days, and ZonedInstant can work in the appropriate zone.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[domenic]

It seems like adding the following should be unambiguous:

• x milliseconds (+ x)
• x seconds (+ 1000 * x)
• x hours (+ 60 * 1000 * x)
• x days (+ 24 * 60 * 1000 * x)

After that is when things get ambiguous, and disallowing them would make sense to me.

[pipobscure]

Yes Domenic, but then why not just go:

instant.toCivilDateTime(zone).plus({}}.toInstant() which is effectively what would happen anyhow.

On 20 Jul 2018, at 19:55, Domenic Denicola [e-mail] wrote:

It seems like adding the following should be unambiguous:

x milliseconds (+ x)
x seconds (+ 1000 * x)
x hours (+ 60 * 1000 * x)
x days (+ 24 * 60 * 1000 * x)
After that is when things get ambiguous, and disallowing them would make sense to me.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub, or mute the thread.

[pipobscure]

See also #58

[maggiepint]

I'm thinking about this one more, and it seems like when you try to actually write out the code it makes sense. @pipobscure to take the timestamp you have and put it IN the relevant time zone, I have to convert to a ZonedInstant anyways:

Temporal.Instant.fromString('2020-02-29T16:00:00.000000000Z').withZone('Australia/Sydney')

At this point, wouldn't adding 1 month to get 2020-03-01T02:00:00.000000000 be exactly expected behavior?

[pipobscure]

For ZonedInstant that's correct. It just doesn't make sense for Instant though. And for ZonedInstant it's really just a shortcut zonedinstant.toCivilDateTime().plus({}).withZone(zonedinstant.zone) rather than zonedinstant.plus({}). Is that enough to warrant having plus() to ZonedInstant especially since the ZonedInstant has no inherent concept of year/month/day/hour/minute/second/nanosecond

[rxaviers]

79 also seems another good reason favoring this change (i.e., restrict .plus to Civil objects)

[pipobscure]

I was wrong.

---

We definitely need plus for ZonedInstant, because otherwise it will be impossible to correctly do DST transitions.

That however leads to the fact that ZonedInstant has no concept of year/month/day/hours/minutes/...

Which leads to: we need a ZonedDateTime instead of a ZonedInstant
Which leads to: #84

@maggiepint @ljharb @mj1856 thoughts/opinions/solutions ?

let xmpl = new CivilDateTime(2018, 3, 25, 1, 30);
xmpl = xmpl.plus({ hours: 1 });
xmpl.toString(); // 2018-03-25T02:30:00.000000000

vs.

let xmpl = new ZonedDateTime('Europe/Vienna', 2018, 3, 25, 1, 30);
xmpl.plus({ hours: 1 });
xmpl.toString(); // 2018-03-25T03:30:00.000000000+01:00[Europe/Vienna]

This together with:

let xmpl = new CivilDate(2018, 2, 28, 20);
xmpl = xmpl.plus({ month: 1 });
xmpl.toString(); // 2018-03-28T20:00:00.000000000

vs.

let instant = Instant.fromMilliseconds(Date.UTC(2018, 2, 28, 20, 0, 0, 0, 0));
let xmpl1 = ZonedDateTime.fromInstant('Europe/London', instant).plus({ months: 1 });
let xmpl2 = ZonedDateTime.fromInstant('Australia/Sydney', instant).plus({ months: 1 });
xmpl1.toString(); // 2018-03-28T20:00:00.000000000+01:00[Europe/London]
xmpl2.toString(); // 2018-04-01T07:00:00.000000000+11:00[Austalia/Sydney]
xmpl2.instant.milliseconds - xmpl1.instant.milliseconds; // 345600000 (4 days)

let's me conclude that either solution is actually wrong. We need something that allows both; and does so without prejudicing different calendars for the future.

The easiest way I can see that happening is creating a TimeZone object as well as a ZonedCivilDateTime.

The ZonedCivilDateTime would be of the Civil family of objects implementing a specific calendar/time system. It could therefore also do arithmetic.

The TimeZone object can be used in conjunction with Instant to create a ZonedCivilDateTime.
It can also be used in conjunction with CivilDateTime to create a ZonedCivilDateTime object (which in turn can be turned into an Instant object).

Doing that would also eliminate the need for ZonedInstant entirely.

[mj1856]

I think in your example you meant Date.Utc(2018,1, 28 ...) (february?) We should make an Instant.fromUtc(year, month day, hour, minute, second, fraction) that uses one-based months to avoid this problem.

I think your argument is a good one for switching to ZonedDateTime. I don't think we should call it ZonedCivilDateTime though. It's still mapable to an instant, so it belongs in that family.

Either way, it's absolutely essential that we allow plus operations that cross DST boundaries. That's one of the main advantages to this API. We need to correctly handle ambiguous/invalid results with sensible and overridable defaults, as discussed in other threads.

I think having a TimeZone object or not is a separate discussion. We can pass time zone as a string until we decide otherwise.

[ljharb]

I very much prefer not passing around strings whenever possible, ie, i prefer to avoid stringly-typed APIs. I'd like to include a TimeZone object in this proposal irrespective of the other decisions discussed here.

[pipobscure]

I have implemented the Polyfill for this here https://github.com/pipobscure/tc39-proposal-temporal/tree/ZonedCivilDateTime (though I haven't run/tested it yet. This is just an experiement in what this would mean.)

Do have a look. Seeing the actual API might bring the discussion into more practical territory.

The relevant new files are:

• TimeZone
• ZonedCivilDateTime

[maggiepint]

Leaving the time zone object out, I want to call out the reasoning behind the choice of ZonedInstant as a name. The purpose of having this Instant naming convention was to clearly separate types that have a relationship with the global timeline from types that do not have that relationship. The point of Civil was to explicitly illustrate that objects did NOT have a relationship to the global time line. This is a good mental model for teaching, and I have no desire to break it.

Under that logic, I don't think ZonedCivilDateTime makes much sense, as it breaks all paradigms. I'm willing to entertain ZonedDateTime, but I also see no reason why ZonedInstant can't simply contain calendar computations.

[pipobscure]

Totally get you. I’m coming at it from the other end.

To me anything that has a concept of year/month/day does not belong to Instant.

To me those are part of the Calendar implementation. So ZonedCivilDateTime is the thing that ties that calendar implementation to the real world.

Which is why I was fine with ZonedInstant sine it had no notion of year/month/day/hour/minute/...

Which might go a way toward explaining the idea behind the ISO/Gregorian name. To me that’s the implementation of a calendar/clock system that then needs to be tied to the timeline.

How about we perform this debate on stage to the bemusement of all 😉

On 27 Sep 2018, at 17:38, Maggie Pint [e-mail] wrote:

Leaving the time zone object out, I want to call out the reasoning behind the choice of ZonedInstant as a name. The purpose of having this Instant naming convention was to clearly separate types that have a relationship with the global timeline from types that do not have that relationship. The point of Civil was to explicitly illustrate that objects did NOT have a relationship to the global time line. This is a good mental model for teaching, and I have no desire to break it.

Under that logic, I don't think ZonedCivilDateTime makes much sense, as it breaks all paradigms. I'm willing to entertain ZonedDateTime, but I also see no reason why ZonedInstant can't simply contain calendar computations.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[maggiepint]

Does it help to call it ZonedDateTime? That at least keeps Civil out - so I'm reasonably okay with heading that direction.

[pipobscure]

I’m not 100% that this is the hill I want to die on, but in my mind it groups it too much with the generic object, rather than the calendar specific ones.

Let’s let it rest though and give it a bit more thought. This is very connected to the naming of “Civil” objects as such. And you already know my preference for calling them by their calendar/clock type.

On 27 Sep 2018, at 19:08, Maggie Pint [e-mail] wrote:

Does it help to call it ZonedDateTime? That at least keeps Civil out - so I'm reasonably okay with heading that direction.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

[pipobscure]

Ok. In #97 this has become ZonedDateTime as per this discussion. In terms of mental model:

Instant - a fixed point on the absolute timeline with no concept of dates/times
Civil* - a way to specify date/time/datetime in local/non-absolute time
ZonedDateTime - the child of the unnatural union of absolute and non-absolute time that glues the local CivilDateTime to the absolute time represented by Instant with the help of a timezone.

Which in turn means that Civil* objects and ZonedDateTime have a .plus() method, while Instant does not. Due to the fact that not all times are legal ZonedDateTime values (DST-gap) ZonedDateTime does not have a .with() method while Civil* objects do.

[pipobscure]

Can we close this?

[gibson042]

ZonedDateTime still needs plus, but that belongs in its own issue.