Remove DateTime arithmetic?

[ptomato]

While discussing #307 in the Feb. 27 meeting it was remarked that any arithmetic results you get with DateTime are potentially wrong due to DST changes, so perhaps it's an antipattern to do DateTime arithmetic.

Here are the use cases that we think should be supported, which don't require DateTime.difference:

• If you need an exact, unambiguous difference in days/hours/minutes/seconds, then use inTimeZone() and take Absolute.difference().
• If you need a difference in years/months/days, and don't care about the hours/minutes/seconds which may be incorrect, then use getDate() and take Date.difference().

Additionally, does it make sense to keep DateTime.plus and DateTime.minus without DateTime.difference? Their results may also be incorrect if adding/subtracting across a DST boundary.

[gibson042]

I consider plus, minus, and difference to be part of a composite bundling of functionality, and am in favor of removing all three from Temporal.DateTime unless someone can present a compelling case that overcomes the above DST issues.

[ptomato]

Meeting, Mar. 5: A valid use case for DateTime arithmetic is when you have a meeting at a particular DateTime (e.g. March 9, 2020, 3 PM) and you need to postpone it to the same time a week later, regardless of whether there is a DST change in between: meetingDateTime.plus({ days: 7 }).

We agree it's possible to misuse this but it's not necessarily an anti-pattern. We'll close this issue, and @sffc took an action to open a new issue to make sure to explain carefully in the reference docs when to use DateTime arithmetic and when to use Absolute arithmetic.

[justingrant]

App developer here. I'm building a calendar app so know these problems painfully well. Here's a bunch more use-cases:

• I want to populate a recurring monthly event in my calendar for the next 24 months, so I need to build an array of each of those 24 dates.
• I want to schedule a tweet to go out in 2 days. (At same local time, even if DST starts/ends before then).
• (e-commerce use-case) this discount code expires at midnight in 3 days, relative to the store's time zone.

I could keep going but you get the idea-- there's lots of reasons why you want to add local days, weeks, months, years to datetime values while trying to keep local time constant. The default rule of thumb that I've used is this:

• Adding hours (and smaller) units is always UTC by default, because "one hour" is always the same in every timezone and every point in time.
• Adding days (and larger) units is always local by default because "one day/week/month" is usually relative to a fixed local time. There are unusual cases where UTC is desired, but that's opt-in behavior.

Also (I'm getting on my knees and begging here) PLEASE don't remove local DateTime arithmetic from the Temporal proposal. I've used date math in every big project I've worked on in the last 10 years. And I've learned painfully that date math is really hard to get right, esp. around DST corner cases. Just this week I fixed two different DST-related date math bugs in date-fns which is a relatively mature library. Having corner cases like these solved "officially" in one place would, IMHO, be a big win for the ecosystem.

Also, I really, really like the API y'all are designing. I can't wait to be able to use it in production!

[kaizhu256]

Having corner cases like these solved "officially" in one place would, IMHO, be a big win for the ecosystem.

what if the "official" dst calculation in temporals is inconsistent with dst calculations in mysql/sqlserver/postgres/etc backend? most ppl would typically defer to databases as source-of-truth for dst-corrections, rather than javascript.

the only way to create temporally-correct programs in javascript (and by correct i mean having results consistent with backend-databases) is to restrict datetime arithmetic to utc in javascript.

[ryzokuken]

Thanks @justingrant for your two cents, it's always great to hear use-cases from app developers. Please check out our cookbook at https://tc39.es/proposal-temporal/docs/cookbook.html which has a bunch of example that might interest you. We'd also love to hear back from you if you have any ideas for the cookbook or any general suggestions 😇

[justingrant]

what if the "official" dst calculation in temporals is inconsistent with dst calculations in mysql/sqlserver/postgres/etc backend? most ppl would typically defer to databases as source-of-truth for dst-corrections, rather than javascript.

@kaizhu256, this is not a big concern for me. For a few reasons:

• Sometimes you just need to do some date math without persisting that result on the server.
• DB calls are expensive (in milliseconds, bandwidth, and hosting costs) so avoiding them, even if it makes some things harder, is often the right tradeoff.
• Inconsistencies between parts of a tech stack is a common problem that's not unique to time/date math. Here's a few more: floating point calculations, integer sizes and limits, random number generation, encryption, regex processing, null/empty-string handling, unicode, locale-aware sorting/collating, UUID generation, anything related to language processing, and so on. IMHO the right solution to these inconsistencies isn't to use overly-minimalistic platforms that lack features that some other platform might handle differently. Rather, it's simply to make an explicit choice of which part of the stack should "own" a particular type of calculation or operation, and to be consistent in that choice throughout the app. (Or to design the app so that inconsistency won't break the app.)
• Some back-ends (databases like MongoDB, not also obviously Node) are JS-centric. If you're running an all-JS stack, then you'll still need a way to do date math!
• Many companies want to reduce the surface area of their DB dependency to make it easier to migrate to a different storage provider in the future. For example, I'm working on a project that started on MongoDB but is likely to move to DynamoDB or Postgres. By keeping as much logic in my Node back-end, it will make that migration easier in the future.
• Test automation support for JS tends to be much better than for SQL, which is yet another reason to avoid putting logic in your database if it can be avoided.

[justingrant]

We'd also love to hear back from you if you have any ideas for the cookbook or any general suggestions 😇

@ryzokuken - Thanks! I looked through the docs and filed a few issues and suggestions. Great work on this, BTW.

[justingrant]

Whoa. When I first commented above, I didn't realize that DateTime has no timezone and therefore no DST awareness in DateTime arithmetic. Now I understand this, so I want to revise my feedback above.

Without DST awareness, offering DateTime arithmetic is dangerous because most of the time it will work fine and sometimes it will unexpectedly fail to match expected results for most use-cases. Worse, the closest corresponding APIs in legacy Date, moment, and date-fns are all DST-aware so this behavior will be unexpected for many developers.

The underlying problem is that there are two subtly different operations implied with date math. "Add one hour" could mean "advance the wall clock by one hour" (ignore DST) or "one hour later in the real world" (account for DST). In my experience (~10 years of time-series-data reporting apps + 2 years building a calendaring app) the "once hour later in the real world" is used at least 10x (maybe 100x?) more than the "move the clock later by one hour" case. IMHO it's bad if the default API path leads devs to the least common use-case. Especially if discovering that it's the wrong API is only obvious on two days per year.

My first-choice recommendation would be to add timezone awareness to DateTime which would enable its arithmetic behavior to match all other major JS libraries' behavior and would solve tricky cases like subtracting one hour from the end of DST which should yield the same clock time but a different underlying instant.

My second choice would be to add a required TimeZone parameter to all DateTime arithmetic methods. Devs who want ignore-DST behavior could pass in the UTC timezone. But this would need to be a required parameter. If choosing to ignore DST isn't opt-in, it'll be a bug factory for inexperienced app devs. True, this would make the "ignore DST" API harder to use, but honestly that's a relatively unusual case anyways so adding more hassle seems OK. Also, #568 would make this option a little easier.

My third choice would be to move add/subtract/difference APIs from DateTime to TimeZone. This wouldn't be as discoverable, but at least it'd provide a one-line-of-code way to do DST-aware math for devs who could find it.

My fourth choice would be to remove add/subtract/difference APIs from DateTime, and instead document how to build these APIs on top of TimeZone and Absolute classes. I think this would be a considerable loss for the usefulness of Temporal because hand-rolling date math is hard!

The only option that I think would be a disastrous outcome would be the current state because many devs would probably make the same mistake I did: assume that DST is handled by DateTime when it's not.

Sorry again for not having this context when I originally commented above... it's taken me a day to wrap my head around the proposal.

[sffc]

This is good feedback. Wall clock time is unintuitive to many (most) unversed programmers to whom I've described it. Much (most) of the time, programmers want an Absolute. However, there are some legitimate use cases for wall clock DateTime, and having a first-class type makes those use cases significantly more ergonomic than requiring that you carry a [date, time] pair, for example.

I'll reopen the issue to discuss at the next Temporal Champions call.

FYI:

My fourth choice would be to remove add/subtract/difference APIs from DateTime, and instead document how to build these APIs on top of TimeZone and Absolute classes. I think this would be a considerable loss for the usefulness of Temporal because hand-rolling date math is hard!

It's not that hard to do.

const tz = Temporal.TimeZone.from("America/Chicago");
const newDateTime = oldDateTime.toAbsolute(tz)
    .plus(Temporal.Duration.from({ days: 2 }))
    .toDateTime(tz);

[ptomato]

The relevant part of our previous discussion is at https://github.com/tc39/proposal-temporal/blob/main/meetings/agenda-minutes-2020-03-05.md

[sffc]

The current position of the Temporal Champions Group is this one:

PDL: I see it as our job beyond stage 3 to promote this mental model so that users understand it. Whatever we do, it will be misused simply because dates and times are complicated.

[justingrant]

Great discussion. A few notes:

@sffc It's not that hard to do.

const tz = Temporal.TimeZone.from("America/Chicago");
const newDateTime = oldDateTime.toAbsolute(tz)
.plus(Temporal.Duration.from({ days: 2 }))
.toDateTime(tz);

It's not necessarily hard in lines of code... it's conceptually hard. And therefore easy to introduce bugs. For example, your code above works great for "n hours later" use-cases, like "meet me at the bar in 2 hours, not 1 or 3 if tonight is a DST transition".

But "n days later" use-cases almost always require keeping local time constant across DST transitions, e.g. "Alexa please reschedule my Friday 6:00PM dinner reservation one week later". Your sample fails for those use-cases.

For this reason, the default add/subtract behavior of moment and date-fns acts like Absolute.plus for hours (and smaller) units but acts like DateTime.plus for days (and larger) units. IMHO this is the right default behavior because it works for the vast majority of real-world use-cases. In the minority case of wanting Absolute behavior for days, there's an easy workaround: force a UTC timezone or use{hours: n*24}.

[justingrant]

Last night I realized that DateTime.compare() is also vulnerable to the same problem because under the covers the comparison is really a subtraction. Here's the case (from #407 (comment)) that's problematic:

...the hour before and after DST ends. For times in this weird period, it's possible for the Absolute difference to be positive while the DateTime difference can be negative, or vice versa.

For all hours of the year except one weird two-hour period, Absolute.compare() behaves identically to DateTime.compare() for the same real-world instants. Without a carefully constructed unit test or a user's bug report, mistakenly using DateTime.compare will be a maddeningly difficult bug for developers to find in their code.

So I'd suggest adding compare() to the list of APIs that should be considered "arithmetic" for the purpose of this issue.

[justingrant]

FWIW, the same DST-unsafe problems are triggered by the DateTime constructor and DateTime.with() if balancing is done between hours (or smaller) units and days (and larger) units, because a "clock-time day" could be 23, 25, or even 24.5 or 23.75 hours long.

[gibson042]

Thanks @justingrant, I very much appreciate your feedback and especially your use cases. Could I push you a little bit further and ask for some example scenarios in which you anticipate time-zone-ignorant DateTime arithmetic (including plus/minus and difference/compare, if you can) to cause problems? I'm unfortunately quite steeped in the Temporal data model and this point, which I fear may hinder my ability to anticipate mistakes by those newer to it.

[sffc]

I think much of the problem here is that the difference between absolute time and clock time is unknown or unintuitive to many programmers. See #569 for other potential solutions.

[justingrant]

@gibson042 - Happy to help. I listed three examples below, with code to illustrate the problem. Caveat: I didn't actually run the code so there may be errors!

The general problem that unifies all three examples is that DST-safe code usually needs to perform Time arithmetic in Absolute terms (e.g. "meet me at the bar in 2 hours, not 1 or 3 hours if DST happens tonight") but Date arithmetic using DateTime/Wall-Clock semantics, e.g. "Alexa, reschedule my Friday night dinner reservation for one week later" where local time stays constant as days/weeks/etc are changed.

This expectation difference between time math vs. date math is intuitive once it's explained, but it's unknown to most developers. Therefore, having it implemented in a library is important to avoid bugs.

For Temporal, this could mean that DateTime (or some class with DateTime's API e.g. Temporal.BikeShed) would modify its add/compare/etc to manipulate times (not dates!) by converting to UTC (which implies a TZ parameter or storing TZ on the object), doing the math, and then converting results back to local time. In other libraries like moment, different behavior is possible (e.g. do date math in UTC), but it's opt-in by using a UTC timezone.

Example 1: Date Arithmetic Across Time Zones
My calendar just showed me a reminder that at this (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)
  // date math in local timezone in case DST starts/ends tonight 
  const departureLocal = localNow.plus({days:1})
  const departureInstant = departureLocal.inTimeZone(departureTz)
  // 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()
}

// Lazy/novice developers can write the code below that works
// almost all the time. But if there's a nearby DST transition, the
// function could return a time that's wrong by 1 or even 2 hours.
function getArrivalTimeFromReminder(reminderInstant, departureTz, arrivalTz) {
  const localNow = reminderInstant.inTimeZone(departureTz)
  const arrivalLocal = localNow.plus({days:1, hours:8}).inTimeZone(arrivalTz)
  return arrivalLocal.getTime()
}
// API changes that would make the code above DST-safe:
// 1. `Absolute.inTimeZone` must return an object that knows its timezone
// 2. By default, `add` should act like DateTime.add for the Date portion, 
//    but act like Absolute.add for the Time portion.

Example 2: Finding Events from 0:00-12:00 today
Fetch this morning's events (meaning local midnight to local noon) from a back-end API that has start (inclusive) and end (exclusive) UTC timestamp parameters.

// correct code
function callApi () {
  const localTz = Temporal.now.timeZone()
  const midnight = Temporal.now.dateTime().withTime({hour: 0})
  const noon = today.plus({hours: 12})
  return myApi(midnight.inTimeZone(tz), noon.inTimeZone(tz))
}

// will return an extra hour or missing hour of events if DST starts/ends today
function callApi () {
  const localTz = Temporal.now.timeZone()
  const midnight = Temporal.now.DateTime.withTime({hour: 0}).inTimeZone(localTz)
  const noon = today.plus({days: 1})  // bug!  Need to use DateTime.add
  return myApi(midnight, noon)
}

// API changes that would make the code above DST-safe:
// 1. `Absolute.inTimeZone` returns an object that knows its timezone
// 2. By default, `add` should act like DateTime.add for the Date portion, 
//    but act like Absolute.add for the Time portion.

Example 3: Sorting Today's Events
Given an un-ordered array of today's events (local today), return an array of {time, id} records sorted from earliest to latest where "time" is local clock time.

// Correct code: sort timestamps before converting to local time
const localTz = Temporal.now.timeZone()
const sortedEvents = events.sort((r1, r2) => r1.timestamp.compare(r2.timestamp))
const sortedRecords = sortedEvents.map(e => ({
  time: e.timestamp.inTimeZone(localTz).getTime(), 
  id: e.id
}))

// This code may sort out-of-order during the hour before & after DST ends
// where later-timestamped events may have earlier clock times.
const localTz = Temporal.now.timeZone()
const records = events.map(e => ({
  time: e.timestamp.inTimeZone(localTz).getTime(), 
  id: e.id})
}))
const sortedRecords = records.sort((r1, r2) => r1.time.compare(r2.time))

[pipobscure]

To me this is such a hard disagree. DateTime arithmetic is a key functionality that removing it obviates the proposal.

[littledan]

The idea from here is to ship the polyfill with Temporal.DateTime arithmetic included, see how it's received, and consider revisiting before Stage 3.

[ptomato]

I think this can be closed, since we are addressing the DST-safety problems in another way by adding a zoned type.