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

moment.tz(<moment instance with tz>, <same tz>) returns wrong result #135

Closed
zgmnkv opened this issue Oct 3, 2014 · 9 comments
Closed
Labels

Comments

@zgmnkv
Copy link

zgmnkv commented Oct 3, 2014

http://jsfiddle.net/bmh34p36/1/

@CarmonColvin
Copy link

I am experiencing a similar issue that may be related to this one.
The setup is different and only affects certain dates in the distant past or distant future.

http://jsfiddle.net/3nfw07wu/6/

@kashifshamaz21
Copy link
Contributor

@zgmnkv @CarmonColvin The issue here is with the moment constructor itself, not an issue with moment-timezone.
The moment constructor using an array input seems to be having a bug, as shown below:

moment([2014, 9, 1, 0, 0, 0, 0]).format(); //gives "2014-10-01T00:00:00+05:30"
moment.utc([2014, 9, 1, 0, 0, 0, 0]).format(); //gives "2014-10-01T00:00:00+00:00"

Can one of you raise this issue on https://github.com/moment/moment, and close this one as thats the place this needs to be fixed in.

@zgmnkv
Copy link
Author

zgmnkv commented Dec 19, 2014

@kashifshamaz21 why do you think it's a bug with moment constructor? For me it seems fine.

@kashifshamaz21
Copy link
Contributor

@zgmnkv Well, as i mentioned in my earlier comment, creating a moment for 2014-09-01 00:00 using the array constructor is giving incorrect results. It returns 2014-10-01 instead of 2014-09-01
For checking this out quickly, navigate to http://momentjs.com/ , open browser console, copy paste following api calls and check the result:

moment.utc([2014, 09, 01, 0, 0, 0, 0]).format();  // doesn't work. Gives incorrect result
moment.utc("2014-09-01 00:00").format();         // works.

@zgmnkv
Copy link
Author

zgmnkv commented Dec 19, 2014

@kashifshamaz21 In js months start from 0

@kashifshamaz21
Copy link
Contributor

@zgmnkv uh, my bad. Sorry for all the confusion, didn't read thru the array constructor note which mentions about the zero-indexing. Okay, digging more into this.

@kashifshamaz21
Copy link
Contributor

@timrwood Any idea on what could be going wrong here? Looks like something to do with cloning of moment.
When we pass a moment instance to moment.tz(momentInstance, zone), it clones the moment instance, right? could it be that, while cloning, we don't keep track of this instance's timezone and hence this 1 hour gap here?

@timrwood
Copy link
Member

I think the issue is here.

We need to add another case to make sure things that moments that are passed to moment.tz are not treated as an array.

@timrwood timrwood added the bug label Dec 19, 2014
timrwood added a commit that referenced this issue Dec 19, 2014
@timrwood
Copy link
Member

#169 should solve this issue.

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

No branches or pull requests

4 participants