moment / moment-timezone

Timezone support for moment.js
momentjs.com/timezone
MIT License
3.83k stars 837 forks source link

toDate() creating incorrect Date object #309

Closed sdesmond closed 7 years ago

sdesmond commented 8 years ago

moment.tz(1493092800000, 'America/New_York');

produces a Moment of "Tue Apr 25 2017 00:00:00 GMT-0400"

but moment.tz(1493092800000, 'America/New_York').toDate()

produces a Date of "Mon Apr 24 2017 22:00:00 GMT-0600 (Mountain Daylight Time)"

I am in Mountain Time, but I would expect toDate() to construct a date object that matches the current moment, not a timezone adjusted one.

mattjohnsonpint commented 8 years ago

The Date object has no time zone abilities other than working with the local time zone. We can't do anything about that. When you use toDate, any of moment or moment-timezone's ability to "represent" other time zones is stripped away. You're left with the raw instant in time represented by the timestamp, and then the Date object converts that to local time when displaying it as a string.

You might as well have just done moment(1493092800000).toDate() or moment.utc(1493092800000).toDate(). It's all the same as just new Date(1493092800000).

To benefit from moment-timezone, you have to use moment's native functions, such as format.

sdesmond commented 8 years ago

Consider this detail from my above post. I have a date that was saved in Eastern time, but I'm not necessarily in Eastern time. I need to use the date as parsed in Eastern. I need to work with this date throughout my application, and I can as long as I keep it a moment. What do I do when I need to convert this to a Date then? I currently have this long-winded solution.

new Date(moment.year(), moment.month(), moment.date(), moment.hours(), moment.minutes(), moment.seconds(), moment.milliseconds())

What's the alternative?

mattjohnsonpint commented 8 years ago

Sorry for the delayed response...

The approach in your last comment is not good. The Date object it creates no longer reflects the correct point in time. It will be shifted by the difference between the local time zone and the one being used in moment.

You said you have a date saved in Eastern time, and that you're parsing it in Eastern. But your example doesn't show that. Your example shows a numeric timestamp, 1493092800000, which will always be interpreted as UTC. By using the tz function with America/New_York, you're converting that value from UTC to Eastern for display purposes - such as when calling .year(), .month(), .date(), .format(), etc. Internally though, it's still the same number.

Also, you haven't said why you need a Date object. In many cases, you can do without it.

You seem to think that somehow moment could give you back a Date object that is in Eastern time. It can't do that. Nobody can, because that's not how the Date object works. The Date object is simply tracking UTC, and converts it to local time for display. That's it.

sdesmond commented 8 years ago

You may be right that if I converted to moment throughout the application this wouldn't be an issue. I get that I created a moment with a timestamp that represents a certain moment in time, but if I then modify that object, I would have expected it to be truly modified. I guess I need to do some experimenting, but you're kind of making it sound like if I do moment.tz(1493092800000, 'America/New_York').hours(12).toDate() I would get the same object as if I did new Date(1493092800000) because the initial date was what the timestamp represents. That seems counter intuitive since all my moment modifications are lost (or is it just timezones that don't carry over?).

My initial reason for wanting a Date was that I'm using Angular and they have a nice filter for Date objects it wouldn't be hard to make my own moment filter though. Also, I am using Date in most places in the application, and introduced moment only recently to deal with some tricky date manipulation. I had a date that was saved to the db as something like Jan 1, 2000 00:00:0000 (eastern time) but when I tried to parse it and display in mountain time I got Dec 31, 1999 so all my dates were one day off (I needed the display date to be Jan 1).

I'll do some experimenting, but I would have thought any manipulation I did with my moment object would be carried over using .toDate(), I may just be mistaken there. Thanks for the responses.

mattjohnsonpint commented 8 years ago

Mutating the object will indeed change the timestamp. When you call hours(12), you're setting it to the 12:00 noon hour in whatever the current mode of the moment object is. Since it's currently set for the America/New_York time zone, then you're setting it to noon Eastern time. The timestamp will be adjusted to the equivalent UTC point in time, which would be 1493136000000, or 2017-04-25T16:00:00Z.

If your own computer is in the Mountain time zones, then that same timestamp would be 2017-04-25T10:00:00-06:00, so a Date object would show that as the equivalent local time.

mattjohnsonpint commented 8 years ago

If you're going to be using moment with angular, then my recommendation would be to use the angular-moment library, which provides angular filters for moment objects without converting them to Date objects.

I'm not sure how to advise you with regard to the rest, other than to say that saving to a database usually involves serializing to some string format or timestamp - both of which you can get from moment directly without converting to a Date first.

I'll also say that usually I see folks converting to Date to pass to a date-picker UI control of some time. Many of those controls will indeed give you a Date object, but they also (usually) have a different property to work with the value as a raw string, either in the provided input format, or in ISO format. Use those methods when available.

Hope that helps.

sdesmond commented 8 years ago

Alright. I had assumed that the .tz() method would mutate the object. From what you've already said, it's just for display purposes. I might argue that it should mutate the object, but I'm sure you've thought this through more than I have. I'll probably just look at converting to moments everywhere and that should solve my issues. Thanks again.

trip-somers commented 8 years ago

I think I'm having a related issue that could use some advice. I'm using moment-timezone with jQuery UI plugins datepicker and datetimepicker. I'm all squared away with getting the timestamp to save in UTC.

When I parse this to local time using the following code that should show me 12:00 PM, it actually gives me 11:00 AM. I think Daylight Savings Time is somehow the problem, since the given date is in CST while the current timezone is CDT, but it looks to me like I'm doing everything correctly.

    localTime = moment.tz("2016-03-09 18:00:00", "YYYY-MM-DD HH:mm:ss", "America/Chicago");
    $('#expires_at').datetimepicker('setDate', localTime.toDate());

I've verified that localTime.toDate() returns "12:00 -0600", so it seems like it should be working. My guess is that datetimepicker isn't able to parse the -0600 out of current timezone context which is now -0500 for America/Chicago thanks to DST. Based on what I've read above, this seems like an appropriate conclusion.

Is there a way to handle DST that I haven't figured out, or is this an issue that hasn't been conquered?

Forgive me if this is a duplicate in some way. I searched "DST" in issues and couldn't find anything that matched this.

TL;DR -- I'm getting an "11:00 ET" display instead of "12:00 CT" display when using a date from CST while browser time is CDT.

UPDATE -- For what it's worth, I wound up tackling this a different way by stripping timezone data completely from the DateTimePickers. Essentially, my app knows what timezone it's in and converts to/from UTC appropriately before and after using the DateTimePicker. Still interested in why 3/9/2016 @ 12:00-0600 was being read as 3/9/2016 @ 11:00-0500 though. Maybe a question for the DateTimePicker people...

mattjohnsonpint commented 8 years ago

Not all datepickers are created equally. You might consider one that's designed with moment in mind.

Here's a few that look promising that use moment internally. I've not tried them personally. http://eonasdan.github.io/bootstrap-datetimepicker/ http://makingsense.github.io/moment-datepicker/ http://ionden.com/a/plugins/ion.calendar/en.html

I'm sure there are others out there. If you're set on jQuery UI, see if you can get/set the date directly as a string instead of as a Date object.

Softman65 commented 6 years ago

moment.clone().toDate() works.........

McQuinTrix commented 5 years ago

Try : moment(moment().tz('America/New_York').format("DD/MM/YYYY HH:mm"), "DD/MM/YYYY HH:mm").toDate()