unicode-org / icu4x

Solving i18n for client-side and resource-constrained environments.
https://icu4x.unicode.org
Other
1.33k stars 172 forks source link

Add nextTransitions and previousTransitions functionality to ZonedDateTime #1005

Open nordzilla opened 3 years ago

nordzilla commented 3 years ago

For a ZonedDateTime, we need the ability to tell when the next and previous daylight savings time transitions are.

This interface should return an iterator over the transitions.

fn nextTransitions(_) -> impl Iterator<Item = _>;
fn previousTransitions(_) -> impl Iterator<Item = _>;

Because not all time zones participate in DST and may have no transitions. Also, a ZonedDateTime may be associated with only a GMT offset (not a named time zone), in which case it would have also have no transitions.

This depends on being able to look up the time-zone data from the IANA time-zone database.

For more context see

sffc commented 3 years ago

I don't think we need an iterator; the spec says

In both cases, startingPoint is an Instant (timestamp).

nordzilla commented 3 years ago

I don't think we need an iterator; the spec says

* 11.4.9 Temporal.TimeZone.prototype.getNextTransition ( startingPoint )

* 11.4.10 Temporal.TimeZone.prototype.getPreviousTransition ( startingPoint )

In both cases, startingPoint is an Instant (timestamp).

So my initial thought process was to do something more like what you just said, but in the Time Zones Deep Dive (Part 3) I recall we had decided that an iterator API would be nice:

Shane: The time zone format and time zone object should be able to support throwing any timestamp at it, and it is able to format it. The time zone doesn't necessarily know where it sits. Yes, it's definitely rule based. A time zone is a list of transitions. It's a list represented by generators or rule based, but it's an implementation detail.

Richard: I think the clarification is good. The observable is a sequence of transitions. Disagree that you can just cut it off. You've got a functionally different time zone if you do this approach.

Justin: I agree with Richard; my point was mainly from an end-user standpoint. The other thing is that I think an iterator-style API is sufficient for use cases; there's no need to build an API that returns a full list.

Independent of the iterator vs. single-next-item question is whether this functionality should live on the TimeZone object or the ZonedDateTime object. I'm happy to move it to the TimeZone object if there is a use case for getting next and/or previous transitions from multiple different starting dates, but I also thought it was clean to have it on the ZonedDateTime object where the starting date is implicitly the date of the DateTime itself.

So we have two questions to resolve:

1) Iterator (lazy) vs. single next transition. 2) This functionality is on TimeZone, on ZonedDateTime, or both possibly for convenience.

I quite like the idea of the iterator API. Given the lazy nature of iterators, I wouldn't expect it to be much more expensive (if at all) than returning only the next item from the start date, and people could use it to get all of them in sequence if desired.

sffc commented 3 years ago

An iterator API seems reasonable if you can make it efficient as you described.

The reason why I think this function belongs on TimeZone and not ZonedDateTime is because of the data model of the two classes. In ECMAScript Temporal, ZonedDateTime is a bag with three fields (TimeZone, Calendar, and Instant). TimeZone is the thing that contains all information relating to the rules about where transitions occur and what the offsets should be. So putting this method on ZonedDateTime would just involve delegating to TimeZone anyway. We could have it on both, but I don't see how we would have it only on ZonedDateTime and not on TimeZone.