Open idkq opened 3 years ago
The reason for this limitation is that Dart's date/time handling is very limited, and DateTime
s can only be in either the local time zone or in UTC. When working with DateTime
s in the local time zone, calculations can easily give unexpected results, like the length of a day not being 24 h if summer time changes to winter time on that day or vice versa. Those problems don't occur when working with UTC-based DateTime
s, hence I chose those to do all calculations. Note that this package doesn't care about UTC's offset being zero, but only about calculations being correct without caring about peculiarities of individual time zones.
If you're working with DateTime
s in the local time zone, I recommend creating a UTC-based DateTime
based on the same year/month/day/hour/etc. to work with this package (and not using localDateTime.toUtc()
, which preserves the same point in time but can change the values of those fields, making calculations incorrect):
final localDateTime = DateTime.now();
final dateTimeForRrule = DateTime.utc(
localDateTime.year,
localDateTime.month,
localDateTime.day,
localDateTime.hour,
localDateTime.minute,
localDateTime.second,
localDateTime.millisecond,
localDateTime.microsecond,
);
Thanks - that is what I ended up doing, and it worked fine for now. I have to fake UTC and reverse as well, because the result is not true UTC. My concern was maintenance since, like you said, a developer could try to convert utc/local with .toUtc()
or .toLocal()
and it wouldn't work.
DateTime _fakeUTC(DateTime nonUtc) {
return DateTime.utc(nonUtc.year, nonUtc.month, nonUtc.day, nonUtc.hour,
nonUtc.minute, nonUtc.second, nonUtc.millisecond, nonUtc.microsecond);
}
DateTime _reverseFakeUTC(DateTime fakeUtc) {
return DateTime(
fakeUtc.year,
fakeUtc.month,
fakeUtc.day,
fakeUtc.hour,
fakeUtc.minute,
fakeUtc.second,
fakeUtc.millisecond,
fakeUtc.microsecond);
}
The problem of using UTC is that when retrieving the instances, any timezone specific conditions is lost. If you retrieve an instance post a change in daylight savings, it would not be reflected accordingly.
The current recipe (i.e. copyWith(isUtc: true)
and then back again) does seem to work (although due to the complexity of timezones I have the feeling it could fail in some obscure scenario), but it seems to me that it's a cumbersome workaround rather than a solution, and morover as you both have mentioned one could easily fall in the trap of using .toUtc()
and .toLocal()
(which I've done).
This package is great, and I think this is the most important missing feature, handling local DateTimes directly. There are ways to handle the intricacies of local timezones, such as flutter's DateUtils.addDaysToDate
which adds calendar days instead of 24h. The main thing to keep in mind when doing this is that calendar "units" (year, month, week, day) do not correspond to exact duration units: adding day is not the same as adding 24h, adding a week is not the same as adding 7 * 24h. As such they should be handled separately from proper duration units such as hour
, minute
, etc. (This is why Java has separate Period
and Duration
classes, for example.). I could look into it if you want.
If you think that it's not worth the trouble, I would at least include the conversion to and from UTC in the package itself, so that users don't have to worry about which is the correct way of converting their local datetimes to a fake UTC datetime and back.
In any case, thanks for this package!
I thought about this problem some more and want to offer a different solution: A separate class called, e.g., LocalDateTime
or PlainDateTime
. This class makes it clear that the stored date and time are independent of any timezone and, hence, timezone details like the change between summer and winter time are not factored into the recurrence calculation.
That class would be inspired by the no longer maintained time_machine package (a port of .NET's Noda Time) and JavaScript's Temporal proposal and its API could look like the following:
@immutable
class LocalDateTime implements Comparable<LocalDateTime> {
/// Creates an instance based on [dateTime]'s year, day, hour, etc.
///
/// This ignores `dateTime.millisecondsSinceEpoch` and `dateTime.isUtc`.
LocalDateTime.fromDateTime(DateTime dateTime);
static LocalDateTime get now => LocalDateTime.fromDateTime(DateTime.now());
// This class will store the year, month, day, hour, minute, second, and
// millisecond using helper classes like `LocalDate` and `LocalTime`.
/// Returns a new [DateTime] with the same year, day, hour, etc. and `isUtc == true`.
DateTime get dateTimeInUtc;
/// Returns a new [DateTime] with the same year, day, hour, etc. and `isUtc == false`.
DateTime get dateTimeInLocalZone;
}
These classes would also have more utilities like LocalDateTime.now
, arithmetic operations, as well as parsing and stringification for subsets of ISO 8601 / RFC 3339. Calculations use DateTime
internally. They will only support the Gregorian calendar, and related classes like ZonedDateTime
and probably localization will be out of scope since I don't have the time to implement or maintain them.
Since these classes are useful outside recurrence rules as well (e.g., in my Flutter package timetable, I could also make good use of LocalDate
and enums for the weekday and month), they would live in a separate package.
What do you think about this proposal?
I thought about this problem some more and want to offer a different solution: A separate class called, e.g.,
LocalDateTime
orPlainDateTime
. This class makes it clear that the stored date and time are independent of any timezone and, hence, timezone details like the change between summer and winter time are not factored into the recurrence calculation.
I think that could be a good long-term solution. I would call it something different than LocalDateTime
, because to me that sounds like a datetime in the local timezone, but instead it's a datetime without any timezone. PlainDateTime
, or Python's distinction between "aware" and "naive" date/time objects, sounds better. I personally like how Python's datetime
deals with all of this: separate date
, time
and datetime
classes, the naive/aware distinction, etc. The only thing I don't like about it is that naive/aware objects are implemented by the same classes. Your solution (like time_machine
) has all of these features and it does separate aware/naive classes.
That class would be inspired by the no longer maintained time_machine package (a port of .NET's Noda Time) and JavaScript's Temporal proposal and its API could look like the following:
I didn't know about the time_machine
package. Now that you mention it, it seems to have everything we need. Is it no longer being maintained? I haven't found any warnings in the README or package description page. It sounds like a pain having to re-implement it from scratch. If it's indeed no longer being mainained or doesn't satisfy some of your requirements, perhaps you could make a fork?
@plammens Valid point, I think I'll call these classes PlainFoo
then. I'll have a more detailed look at Python's version of this topic (and also look at Kotlin/Java), but also prefer separate classes for date/time with or without time zone information (“aware”). (I'll probably also add an Instant
class that's basically DateTime.utc
.)
Actually, both rrule and timetable once used time_machine, but, due to bugs and lacking maintenance of that package, users requested a change away from time_machine: https://github.com/JonasWanke/timetable/issues/33, https://github.com/JonasWanke/rrule/issues/16, and https://github.com/JonasWanke/rrule/issues/9. Both packages now use Dart's native DateTime
and have asserts that the objects are valid. In particular, time_machine uses assets for time zone information but Dart doesn't offer a way to ship assets with code (unlike Flutter, where that's pretty easy). Hence, these asset folders also have to be present at the correct place depending on how the code is distributed (see https://github.com/Dana-Ferguson/time_machine/issues/25, https://github.com/Dana-Ferguson/time_machine/issues/53, and also https://github.com/Dana-Ferguson/time_machine/issues/55). That's also why I do have a fork of time_machine: https://github.com/JonasWanke/time_machine. But time_machine contains a lot of code for timezone handling using data from the Time Zone Database or parsing/formatting dates/times with custom formats which are not relevant here and more effort to maintain.
I'm slightly confused as to why "all DateTimes must be in UTC".
Requirement: I want a recurrence every business day of the week (MON-FRI) at 23:59:59 in local time . Scenario: Let's say user is on UTC-4 and wants a recurrence every day of the week MON-FRI at 23:59:59 in local time. If we convert FRI 23:59:59 to UTC, we now have SAT 03:59:59 which breaks my requirement of every business day of the week. So in the end, when retrieving the instances, I would only have a total of 4 instances since the 5th instance (Friday) is actually on Saturday_UTC, but Friday_Local.
I guess is this a new requirement? To support local recurrence, or is there any other way to deal with that? I am not sure if anyone would care about days of the week in UTC time, since business days/weekends are always relevant in local time.
Obviously I could fake my local time to be UTC, but that would be wrong by definition, and it would only be to retrieve instances.