-
Notifications
You must be signed in to change notification settings - Fork 835
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
Library not handling DST switchover properly #357
Comments
We already changed it to handle ambiguous time in the way you mentioned, we just haven't updated the docs yet. See #354, moment/momentjs.com#226, and other issues linked from there. For invalid time, the most common case is indeed to pick the earliest of the two possible instants. We actually give you control over these now, with I can appreciate that if you are "in the moment" (pun intended) that it might make sense to pick the current offset. However, in the general case, it doesn't make sense to have a function that returns a different result given the same input just because it's executed at another time. It might be useful if there was a function you could implement to implement custom behavior like this. Currently though, there isn't. However, you could consider testing for ambiguity after the fact, and adjusting to the current offset when ambiguous. Here's a function that will do that. I didn't harden it for all possible moments (local, utc, etc.) but it will work for moment-timezone based moments. function adjustAmbiguousToCurrentOffset(mom) {
var currentOffset = moment.tz(mom.tz()).utcOffset();
if (mom.utcOffset() !== currentOffset && mom.clone().utcOffset(currentOffset, true).utcOffset() === o) {
mom.utcOffset(currentOffset, true);
}
}
var m = moment.tz("2016-11-06T01:00", "America/New_York"); // will be -04:00
adjustAmbiguousToCurrentOffset(m); // will use -05:00 if DST not currently in effect |
Docs updated. Thanks. |
First, thanks for making this library and supporting it! It is very much appreciated. I wanted to enter this issue because it really is preventing me from using the library. It has to do with the way the library handles ambiguity during DST switchover. Here is what the documentation says:
The problem is, rounding down gives the wrong answer, where ROUNDING UP would actually be perfect. If I enter the time "2:30:00", I want it to make the time "3:30:00" for me, since that it what the time actually has become. I don't want it to make it "1:30:00" which would cause me to lose time, and requires me to handle this case as an exception. In other words, if a person enters a time that "does not exist" it makes much more sense to convert that time to the time the user intended according to the new DST situation. I can't imagine this would be large change, but it would make this library perfect and logical in every way.
Just in case you think my "suggestion" is not valid, please consider the following: a worker goes to a time clock to punch out for the night, and he looks at his watch where it says "2:15 a.m.". If that time were entered into the clock, it would make perfect sense to change the time he entered to "3:15 a.m.", which is actually the correct interpretation of what he entered. Making the time "1:15 a.m." makes no sense, and would in fact be considered an error.
In the other case, where the times come around twice, instead of choosing the "earliest" time, the library should just accept the time, but use the offset that is current at the time of calling the function. This way, if the time is entered at the first 2:15 a.m. it would be entered using the offset at the time of entry. If the time is entered at the second 2:15 a.m. (which is really 3:15 a.m.), it would also be entered using the post switch-over offset.
Both of the behaviours I have described will give the library logical handling of these two edge-case conditions, and will avoid error conditions that have to be handled by application code.
So, thanks for giving this consideration, and thanks again for all the work done on these libraries! :-)
The text was updated successfully, but these errors were encountered: