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

Interaction of Duration vs DST (or other TZ offset changes) #559

Closed
justingrant opened this issue May 13, 2020 · 14 comments
Closed

Interaction of Duration vs DST (or other TZ offset changes) #559

justingrant opened this issue May 13, 2020 · 14 comments

Comments

@justingrant
Copy link
Collaborator

I'm reviewing the latest proposal and couldn't find discussion of how Duration is affected by DST, if at all. Is there docs or GH issue discussion on this? The particular line in the docs that concerned me was this one:

Balancing between the different units of durations excludes years and months, because years and months can have different lengths.

This is also a problem for days, which can have 23, 24, or 25 hours... or in theory even 23.75 or 24.5 in oddball cases like when countries change their timezone offsets, per Wikipedia:

Newfoundland, India, Iran, Afghanistan, Myanmar, Sri Lanka, the Marquesas, as well as parts of Australia use half-hour deviations from standard time, and some nations, such as Nepal, and some provinces, such as the Chatham Islands of New Zealand, use quarter-hour deviations.

It's a valid decision for Duration to ignore DST and to treat all "duration days" as exactly 24 hours, but I think this decision should be much more explicit in the docs, e.g. DST notes in every method's docs for the Duration API.

The plus and minus methods of Duration seem particularly vulnerable to this misunderstanding, because P24H may not yield equivalent results (e.g. when adding or subtracting from a DateTime) as P1D in a DST timezone, and it's easy to introduce this bug by using these methods.

@ryzokuken
Copy link
Member

@justingrant while Durations treat all days as 24 hour long while balancing, all DateTime arithmetic accurately reflects DST transitions. Do you propose that we stop balancing 24 hours into days as well? That sounds like a decent idea.

@justingrant
Copy link
Collaborator Author

@ryzokuken Honestly I'm not familiar enough with balancing use-cases to have an opinion. I know that the classic JS Date.setXXX methods do balancing, but (other than X [unit] ago formatting) I've only used those balancing features as a workaround for lack of proper date plus/minus/diff in today's JS.

@ryzokuken
Copy link
Member

@justingrant keep in mind that you don't have to balance either. date.plus({ days: 900 }) is perfectly valid. But when balancing, we balance 24 hours into 1 day. I was wondering if it's not correct enough in your opinion though...

@ryzokuken
Copy link
Member

(of course you could balance and then use something like Intl.DurationFormat to format into an appropriate string)

@ptomato
Copy link
Collaborator

ptomato commented May 13, 2020

Whether to balance 24 hours ↔ 1 day is something we've gone back and forth on a few times, so I think that's open to discussion if need be.

@justingrant
Copy link
Collaborator Author

While researching #558, I learned how Java handles the problem of DST and durations.

Java has a time-only duration type Duration and a date-only duration type Period. This is a smart way to evade DST issues by forcing the developer to choose if they want "calendar date" durations or "absolute time" durations. This kind of explicitness reminded me of the spirit of Temporal's API.

@ryzokuken ryzokuken added this to the Stage 3 milestone May 20, 2020
@ryzokuken ryzokuken removed this from the Stage 3 milestone May 20, 2020
@justingrant
Copy link
Collaborator Author

@ryzokuken - following up on your question from a few weeks ago:

while Durations treat all days as 24 hour long while balancing, all DateTime arithmetic accurately reflects DST transitions. Do you propose that we stop balancing 24 hours into days as well? That sounds like a decent idea.

I agree. Now that I understand the Temporal API better, my answer is "yes, we should stop balancing 24 hours into days" from Absolute, because days can be different lengths because of DST. Specifically, these cases should throw:

  • Absolute.prototype.plus or Absolute.prototype.minus with days or larger
  • Absolute.prototype.difference with largestUnit of days or larger

It's an interesting question whether DateTime.prototype.difference with largestUnit 'hours' or smaller should also be verboten. My instinct is "no" because because clock hours actually exist on clocks while "absolute days" are inherently ambiguous because they don't always correspond to real-world calendar days.

BTW, I have more research and suggestions to share about how Duration can be made safer for DST-- will share later this week.

@justingrant
Copy link
Collaborator Author

While researching #558, I learned more about how iCalendar handles durations and DST:

iCalendar durations are standardized in RFC 5545. AFAIK, Durations in this standard are designed for two use cases: to define the length of meetings in local clock time AND to model the reminder time delta before calendar events.

These use cases require deterministic arithmetic behavior, because it'd be bad for one calendar app to show a meeting length or reminder time differently than other apps. Therefore the spec defines a specific order of operations (add or subtract larger units before smaller ones) and specific DST behavior.

The DST behavior is interesting. According to this doc, RFC 5545 has a concept of "nominal durations" (in local clock time, like Temporal.DateTime) and "exact durations" (a real-world stopwatch, like Absolute). Duration persistence matches ISO 8601 durations, with three differences:

  • months and years are not allowed (for the same reason we don't balance Duration months/years)
  • negative durations (e.g. calendar reminders before an event) are persisted with a minus sign prefix
  • RFC 5545 considers time units to be "exact time" but considers date units to be "nominal dates". So P1DT12H apparently means "one calendar day and 12 real-world hours", while PT36H means "36 real-world hours". Here's the relevant line in the spec:

The format can represent nominal durations (weeks and days) and accurate durations (hours, minutes, and seconds).

BTW, this "duration times are absolute, duration dates are not" convention matches how moment.js has been handling date-time math since 2011.

Temporal use-cases are broader than iCalendar's, so this "absolute time/calendar date" convention may not cover all Temporal use cases. But it's also a common use case, so as we investigate improvements in the Duration API, we should make sure that any improvements work for this case too.

@sffc
Copy link
Collaborator

sffc commented Jun 4, 2020

When dealing with Absolutes, 1 day == 24 hours, which is consistent with the expectations shared by the champions. When dealing with Dates and DateTimes, 1 day == 1 calendar day. A Duration is merely a bag of options that is interpreted in the context of the concrete Temporal type you give it to.

@justingrant
Copy link
Collaborator Author

A Duration is merely a bag of options that is interpreted in the context of the concrete Temporal type you give it to.

Yep, that's the core problem. Unlike all other Temporal types, if you have a function that accepts a Duration parameter (or a persisted duration string, or a Duration-like bag) there's no way to know what context (Absolute, DateTime, or hybrid like RFC 5545) it's safe to use that Duration in.

This inherent ambiguity is unexpectedly different from the rest of Temporal which avoids ambiguity by using strong typing and separation of concerns to help developers avoid mistakes.

By analogy, Temporal could have offered a single date/time type instead of a DateTime vs. Absolute split. It could be up to the developer to decide what that type means depending on context. But Temporal (rightly, IMHO) chose not to do this.

Why is Duration different?

@sffc
Copy link
Collaborator

sffc commented Jun 5, 2020

Why is Duration different?

I see Duration as a helper class on the side. The absolute/clock distinction is done in the primary Temporal types.

All that being said, if we add ZonedAbsolute, then we could define Duration days the same way as RFC 5545. We just wouldn't support days in Absolute arithmetic, only ZonedAbsolute.

  1. Absolute: largest duration field = hours (not days, because you need the time zone)
  2. ZonedAbsolute: largest duration field = days (not months/years, because you need the calendar)
  3. DateTime: largest duration field = years

@justingrant
Copy link
Collaborator Author

justingrant commented Jun 9, 2020

@sffc I see Duration as a helper class on the side. The absolute/clock distinction is done in the primary Temporal types.

Yep, this is probably the core of where we're not aligned re: durations. I suspect the disconnect is related to use cases. The "three kinds" ambiguities only matter when doing math operations. Some use cases like parsing and formatting don't generally need math, so for those use cases Duration is indeed a simple helper class and its "three kinds" don't matter much because.

But for use cases that make heavy use of plus/minus/difference, Duration is more of a first-class citizen that's required for most use cases and where ambiguity can cause DST bugs. So Duration's lack of strong typing is much more challenging to deal with.

@sffc if we add ZonedAbsolute, then we could define Duration days the same way as RFC 5545.

This seems like a good option to consider as an alternative to other solutions like a kind field on Duration.

What impact would "define Duration days the same way as RFC 5545" have on DateTime's difference, plus, and minus methods? Are you thinking that they'd require a timeZone parameter so that the input or output duration would match RFC 5545? Or only require a timeZone param if largestUnit of 'days' or larger?

@justingrant
Copy link
Collaborator Author

FYI, I wrote up a bunch of options in #686 for how Temporal could address the problems discussed here in this issue related to the ambiguous meaning of the current Duration type.

It's a long writeup so I figured it made sense to move into a new issue, but also happy to merge it back into this one if that'd be better.

@justingrant
Copy link
Collaborator Author

We've moved pretty far past the discussions in this issue. In particular, LocalDateTime will land soon which is the recommended way to do DST-safe math. Developers who want to work with other kinds of durations can use Absolute or DateTime math. So closing this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants