-
Notifications
You must be signed in to change notification settings - Fork 157
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
Consider alternatives to Temporal.DateTime (ZonedAbsolute) #569
Comments
I think education will be needed regardless of which of the 4 options chosen (the 4th being, leave the spec as it is). I don't personally find wall clock time confusing, regardless of its name, and I think it's critical that a primitive type for it exist (under whatever name). I would prefer (modulo a bikeshed on a name) option 4, then option 3, and don't consider option 1 or 2 useful or viable. |
For cleanliness sake I would change option 1 to: Add Which would also mean that generating an So yes, we can play with the idea, but I really don’t see any actual benefit. I will now admit to still not actually fully understanding generators (I get the concept, but that’s it). I think evidence of not fully understanding a complex scenario isn’t unexpected; especially from unversed software engineers. And it’s not actually anything to worry about. — As for option 2: it then leaves a huge whole that needs to be filled. Because the one thing that happens more than dealing with a date & time in a specific timezone, is dealing with date & time without a specific timezone. ` — option 3: is basically just a name bikeshed. We’ve gone around this one at least 4 times that I can actively recall, and a 5th won’t hurt any. Though I have to admit that the likelihood of anything actually beneficial to come of it seems remote at best. — Which leaves option 4. |
That all being said: if we fill the deficiencies with options 1 & 2 I’m happy to go there as well. It’s jus that these are rather big glaring holes that need to be filled first. |
For option 3, name bikeshed: It's important for the names of the types to suggest what they are intended to be used for. We should not rely only on education. Imagine an unversed programmer looking at Temporal for the first time. They want to represent a point in time, the most common use case. Given a choice between
Looking purely at the names, I have the suspicion that programmers will choose Temporal.DateTime, even if Temporal.Absolute is more generally the correct choice. (I have no data to back up this claim, other than my speculation above and observations I've made discussing with other engineers.) I would consider the following naming scheme:
Thoughts? P.S. It should be noted that in both MySQL and Ruby, a "DateTime" is actually a wall clock time (no associated time zone). However, the top-voted answer in this Stack Overflow question recommends using DATETIME over TIMESTAMP, which in most cases is just wrong (it makes your database entries sensitive to the host's time zone). |
Option 1I think the use case for Temporal.DateTime arithmetic, while less common than Temporal.Absolute arithmetic, is common enough that it merits having a first-class type in Temporal. There are not many instances in the cookbook, but there are not none either. I think it would be a source of bugs to ask users to roll their own even if it's relatively simple. The use case for DateTime arithmetic is IMO the most pressing reason to keep a Temporal.DateTime type, but there are also operations that would be significantly less ergonomic to perform on a Option 2I wouldn't be opposed to adding back Temporal.ZonedDateTime (and probably renaming it Temporal.DateTime) as it would address one of the pitfalls that I found while writing cookbook examples (#240 (comment)):
But for the same reason I mentioned in option 1 I'd want to keep Temporal.DateTime and rename it to something else. That would fill the hole that @pipobscure objected to. Option 3I think I wouldn't be opposed to Option 4Needs to be done anyway, regardless of this issue, IMO. What I think would be very effective is a flowchart for choosing which type to use, in the manner of https://stackoverflow.com/a/22671607/172999 |
I also like Timestamp over Absolute regardless :-) |
I agree with @ljharb that the "wall clock time" concept of DateTime is NOT confusing. It took me a while to understand that (unlike similarly-named concepts in other APIs) Temporal.DateTime had no linkage to time zones nor real-world instants. But once I understood that, its meaning was clear to me and I think would be clear to anyone. Overall the current design of Temporal seems like a reasonable "assembly language" of low-level primitives that can be used together to perform any date/time related use-case. I also agree with @sffc's and others' concerns about the usefulness of DateTime for many important use-cases. I also have three serious and inter-related concerns with the current design of DateTime:
Below are details about each of these concerns. Below that is a rollup of some concrete suggestions that I think might address these concerns. FWIW, @ptomato's Option #2 would work best for the use-cases I'm familiar with. DST Safety Almost all (all?) non-trivial DateTime operations are unsafe for DST, where "unsafe" means "easy for developers to write code that delivers expected results (and passes unit tests!) almost all the time, but delivers unexpected results around DST transitions". These DST-unsafe operations include:
To understand why these operations are unsafe, here's some use-cases with sample code. #407 (comment) has a few possible options to fix DST safety:
DX/Verbosity for common use-cases Once DST safety is the default, my next concern is the cognitive load and verbosity required to write DST-safe code. The biggest challenges:
Here's an illustrative use-case: My calendar just showed me a reminder. At this same (local) time tomorrow my flight will depart for London. My flight is 8 hours long. What local time will it be when I get there? // correct logic is hard
function getArrivalTimeFromReminder(reminderInstant, departureTz, arrivalTz) {
const localNow = reminderInstant.inTimeZone(departureTz)
// Do date math in local timezone in case DST starts/ends tonight
const departureLocal = localNow.plus({days:1})
const departureInstant = departureLocal.inTimeZone(departureTz)
// Do hours math in Absolute because the length of the flight is
// not affected by DST
const arrivalInstant = departureInstant.plus({hours:8})
const arrivalLocal = arrivalInstant.inTimeZone(arrivalTz)
return arrivalLocal.getTime()
}
// Here's what an easier API could look like
function getArrivalTimeFromReminder(reminderInstant, departureTz, arrivalTz) {
// `localNow` retains the timezone used when it was created
const localNow = reminderInstant.inTimeZone(departureTz)
// `days` is incremented in clock time, while `hours` is incremented in UTC
const arrivalLocal = localNow.plus({days:1, hours:8}).inTimeZone(arrivalTz)
return arrivalLocal.getTime()
} Here's the convention that makes an easier API possible:
"DateTime" name vs. developer expectations A common pattern in API design is a "main" vs. "expert-only" API:
An example of doing this badly is in the C standard library: An example of doing this right is Luckily, most Temporal types like Temporal.Date are useful both as "main" and "expert" APIs. The exception is Temporal.DateTime, which is IMHO an "expert" API because it's hard to use for real-world, DST-safe use-cases. (which might get even harder depending on what DST-safety changes are decided) If Temporal retains a zoneless date+time class, then I'd suggest a longer and less alluring name (e.g. Wishlist Here's a wishlist of suggestions to address the concerns above. Y'all understand this space more than I do, so feel free to toss ideas you think won't work.
As a newbie to this proposal and with my app developer's perspective, I'm admittedly ignorant of many of the requirements and most of the history. So I apologize in advance if I'm re-hashing settled arguments, missing important use-cases, or presenting naive suggestions. Overall I'm really impressed with the work that y'all have done! |
as product-developer, the following naming-convention seems intuitive (to me) with minimal education:
though not strictly necessary, |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
TimeZone slot in DateTimeThe question about whether to include a TimeZone slot in DateTime is related to the question of whether to include a Calendar slot. It's also worth pointing out that Temporal.Absolute does not have or need either TimeZone or Calendar. Food for thought: Why should DateTime have a Calendar slot but not a TimeZone slot? My answer to this question: The concept of a wall clock time is calendar-agnostic, but it has no useful manifestation without the existence of a calendar system. In order to get the year, month, or day, or to perform arithmetic, you need a calendar system. However, the removal of a time zone is what defines a wall clock time. Therefore, it makes sense for a wall clock type to have a calendar slot but not a time zone slot. I had originally proposed in #268 that Date and DateTime not have a Calendar slot, and that a Calendar should be provided when performing any calendar-sensitive operations. However, I was later convinced that we should indeed have a Calendar slot, in large part because of developer expectations that they should give the calendar once, and have it used for all calculations from that point forward. This is essentially the same line of reasoning as this bullet from @justingrant's post: "Having to pass the same timezone into so many subsequent APIs when I already told Temporal what timezone I wanted!" Flight to London
I think this is a beautiful example of the Temporal data model working as designed. You need to add two quantities: 1 day in wall clock time, and 8 hours in absolute time. You therefore perform one calculation with Temporal.DateTime, and the other calculation with Temporal.Absolute. The existing Temporal data model is the result of a lot of design, and I think the distinction between wall clock time and absolute time is a good one. ZonedAbsolute
The type you are proposing is essentially a ZonedAbsolute. That's the same data model as legacy JavaScript Date, the thing we know is broken and that we are trying to replace. However, you bring up a good point about a potential fatal flaw in the ZonedDateTime data model, which is the inability to distinguish between overlapping times at a DST transition. Suppose for the sake of argument that we were to adopt your data model. (For the purposes of this post, I will use the term ZonedAbsolute, but we could change the name later or replace DateTime with this new data model.) The slots for this new type would be:
We would debate whether to make arithmetic have the semantics @justingrant proposed (days and higher are wall clock time, hours are lower are absolute time), or to keep arithmetic all in wall clock time. The method to convert from Absolute to ZonedAbsolute would accept both TimeZone and Calendar. We would debate about whether those arguments are optional (default UTC/ISO) or required. ZonedAbsolute.prototype.toLocaleString would use the time zone and calendar encoded in the data model. Absolute.prototype.toLocaleString would use the environment's time zone and calendar. Other than that, the semantics of ZonedAbsolute would be indistinguishable from DateTime. How would ZonedAbsolute affect the rest of the cookbook examples? |
Thanks @sffc for the clear and even-handed summary. The app-dev use cases that I'm most familiar with would benefit a lot from something like To clarify, my concern about DateTime isn't that the wall-clock time concept is too hard for devs to learn. Rather, it's that best practices for DST safety are too hard for most devs to learn and apply. In my experience, without a higher level of abstraction like
Here's four tiny
// Get clock time from a {Temporal.Date, Temporal.Time}
// e.g. from date/time picker components
//
// Problem: novices won't know they need to validate/coerce
// the time in case it's an invalid time like 2:30AM on a day
// where DST starts at 2:00AM. BTW, I checked how MS Outlook
// handles this, and it will silently coerce invalid times
// to valid times. No user-facing error.
// naive
function getCurrentPickerTime(datePicker, timePicker) {
return DateTime.from(DatePicker.value)
.with(TimePicker.value)
}
// ZonedAbsolute (same as naive)
function getCurrentPickerTime(datePicker, timePicker) {
return DateTime.from(DatePicker.value)
.with(TimePicker.value)
}
// DST-safe (current API)
function getCurrentPickerTime(datePicker, timePicker) {
return DateTime.from(DatePicker.value)
.with(TimePicker.value)
.Temporal.Absolute.inTimeZone(Temporal.now.timeZone())
.inTimeZone(Temporal.now.timeZone())
}
// Move a deadline one day later.
//
// Problem: novices won't know they need to switch into the
// local time zone to account for possible DST
// naive
function oneDayLater(absolute) {
return absolute.plus({day: 1})
}
// ZonedAbsolute (same as naive)
function oneDayLater(zonedAbsolute) {
return zonedAbsolute.plus({day: 1})
}
// DST-safe (current API)
function oneDayLater(absolute) {
return absolute.inTimeZone(Temporal.now.timeZone())
.plus({day: 1})
.inTimeZone(Temporal.now.timeZone())
}
// Postpone a reminder by one hour.
//
// Problem: DST transitions should be ignored
// naive - "Yesterday when I was writing `oneDayLater`,
// my manager yelled at me for writing DST-unsafe code,
// so to avoid the same fate I'll just copy/paste the
// DST-safe code here and change the units."
function oneHourLater(absolute) {
return absolute.inTimeZone(Temporal.now.timeZone())
.plus({hour: 1})
.inTimeZone(Temporal.now.timeZone())
}
// ZonedAbsolute (not same as this particular
// copypasta-based naive example. but would be what
// an actual naive developer would write)
function oneHourLater(zonedAbsolute) {
return zonedAbsolute.plus({hour: 1})
}
// DST-safe (current API)
function oneHourLater(absolute) {
return absolute.plus({hour: 1})
}
// Return a sorted list of local Date & Time of flight arrivals
// in local time zone
//
// Problem: For one hour before DST ends,
// real-world sort order won't match clock-time sort.
// naive - "To avoid creating two copies of the
// array I'll get local times and then sort them."
function getSortedArrivals(arrivals) {
return arrivals.map(a => a.inTimeZone(Temporal.now.timeZone()))
.sort(Temporal.DateTime.compare)
}
// ZonedAbsolute (same as naive except class name)
function getSortedArrivals(arrivals) {
return arrivals.map(a => a.inTimeZone(Temporal.now.timeZone()))
.sort(Temporal.ZonedAbsolute.compare)
}
// DST-safe (current API)
function getSortedArrivals(arrivals) {
return [...arrivals].sort(Temporal.Absolute.compare)
.map(a => a.inTimeZone(Temporal.now.timeZone()))
} |
Here's another use case that I forgot to include above: string formatting including a time zone, like legacy // Format a date, time, timezone string
//
// ZonedAbsolute
function displayTodayNoon() {
return Temporal.now
.zonedAbsolute()
.with(new Temporal.Time(12))
.toLocaleString()
}
// Current API. Requires learning about Intl display options
// to use the default display format for a timezoned date/time.
// Also requires discovering that the correct format method
// lives on Absolute which is non-obvious because nothing else
// on that class relates to local clock time.
function displayTodayNoon() {
return Temporal.now
.dateTime()
.with(new Temporal.Time(12))
.inTimeZone(Temporal.now.timeZone())
.toLocaleString(undefined, {timeZone: Temporal.now.timeZone().name})
} |
Here's another potential model using ZonedAbsolute, which also relates to the open question of the default calendar (#292). If we add ZonedAbsolute, it can be an intermediate type between Absolute and DateTime. It would have a time zone, with the capability to change time zones, but no calendar or calendar-specific accessors like year/month/day. Arithmetic would all be performed as absolute time, not wall clock time. If you need calendar-dependent operations (accessors and wall clock arithmetic), you call Note: This model of ZonedAbsolute may make Absolute obsolete. I have more details and a nice graphic shown here: https://tc39.es/proposal-temporal/docs/calendar-draft.html#new-non-calendar-types-option-5 |
We talked about this in passing in the May 28 meeting. The initial polyfill is not going to ship with a ZonedAbsolute, but we would like to revisit it for Stage 3. @justingrant is working on a proof of concept. @gibson042 indicated he would prefer ZonedDateTime and we intend to discuss the pros and cons on this thread. |
This adapts all the relevant cookbook examples (except the meeting planner which is blocked on a bug in ZonedDateTime.toLocaleString) to use ZonedDateTime. Most of these were identified in Justin's original pull request. Co-authored-by: Justin Grant <justingrant@users.noreply.github.com> See: #569
The ZonedDateTime's time zone wins over the system's default time zone. If you explicitly give a time zone in the formatter options, and you try to format a ZonedDateTime with a time zone which is different from the explicitly given time zone, then toLocaleString() will throw an exception. formatRange() and formatRangeToParts() require ZonedDateTimes with the same time zone. See: #569
Now that ZonedDateTime.toLocaleString() works, we can fix this example. It becomes much simpler using ZonedDateTime. See: #569
This operation will be reused in ZonedDateTime.until() and since(). See: #569
The rounding in ZonedDateTime.toString() only affects time units. Unlike the rounding in ZonedDateTime.round(), it should be exact time rounding, so that the actual difference between the raw and rounded exact times is not affected by time zone offset shifts. This is the same outcome as in round() (and we add a test for both round() and toString() to prove it) but in toString() it can be implemented with fewer observable calls since we don't need to be able to round to units that are affected by DST shifts. See: #569
The rounding in ZonedDateTime.toString() only affects time units. Unlike the rounding in ZonedDateTime.round(), it should be exact time rounding, so that the actual difference between the raw and rounded exact times is not affected by time zone offset shifts. This is the same outcome as in round() (and we add a test for both round() and toString() to prove it) but in toString() it can be implemented with fewer observable calls since we don't need to be able to round to units that are affected by DST shifts. See: #569
And make a small adjustment to the polyfill to match. See: #569
The ZonedDateTime's time zone wins over the system's default time zone. If you explicitly give a time zone in the formatter options, and you try to format a ZonedDateTime with a time zone which is different from the explicitly given time zone, then toLocaleString() will throw an exception. formatRange() and formatRangeToParts() require ZonedDateTimes with the same time zone. See: #569
Now that ZonedDateTime.toLocaleString() works, we can fix this example. It becomes much simpler using ZonedDateTime. See: #569
This operation will be reused in ZonedDateTime.until() and since(). See: #569
This operation will be reused in ZonedDateTime.until() and since(). See: #569
The ZonedDateTime's time zone wins over the system's default time zone. If you explicitly give a time zone in the formatter options, and you try to format a ZonedDateTime with a time zone which is different from the explicitly given time zone, then toLocaleString() will throw an exception. formatRange() and formatRangeToParts() require ZonedDateTimes with the same time zone. See: #569
Now that ZonedDateTime.toLocaleString() works, we can fix this example. It becomes much simpler using ZonedDateTime. See: #569
And make a small adjustment to the polyfill to match. See: #569
I'm sorry to the other Temporal champions for relitigating this issue, but now that we have more evidence in the form of cookbook examples and developer feedback, I think it's at least worth revisiting the use cases behind Temporal.DateTime.
@justingrant commented in #407 (comment) that the mental model of wall clock time and wall clock arithmetic (no time zone associated with it) was unintuitive to him, a sentiment that I share and have observed when pitching Temporal to other unversed software developers.
The cookbook has some examples of Temporal.DateTime use cases. Looking through those use cases, they fall into one of a few buckets:
Here are some ways we can keep keep most of this functionality available while making it easier to do the right thing (and harder to do the wrong thing):
Temporal.Absolute.prototype.getDateAndTime(tz)
: returns a record{ date, time }
containing the wall clock date and time in a certain time zone. Also allow constructing a Temporal.Absolute from a record with those two pieces.If we do nothing, the burden is on us to run an educational campaign. The question is whether we can lean on good documentation to prevent bugs that will inevitably arise from developers misunderstanding the purpose of Temporal.DateTime.
@ptomato @gibson042 @pipobscure @justingrant @ryzokuken @maggiepint
The text was updated successfully, but these errors were encountered: