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

Explicit time zones break utcOffset(..., true) #187

Open
petebrowne opened this issue Mar 9, 2015 · 16 comments
Open

Explicit time zones break utcOffset(..., true) #187

petebrowne opened this issue Mar 9, 2015 · 16 comments

Comments

@petebrowne
Copy link

I'm trying to use the utcOffset with the keepLocalTime parameter as true. It looks like if you use an explicit time zone when creating the object, it sets the the internal property _isUTC to true, which breaks the utcOffset.

var m1 = moment('2014-11-21T10:30:00-06:00');
moment(m1).utcOffset('UTC', true).format(); // 2014-11-21T10:30:00+00:00
m1._isUTC; // false

var m2 = moment.tz('2014-11-21T10:30:00', 'America/Chicago');
moment(m2).utcOffset('UTC', true).format(); // 2014-11-21T16:30:00+00:00
m2._isUTC; // true ??
@petebrowne petebrowne changed the title Explicit time zones break utcOffset('...', true) Explicit time zones break utcOffset Mar 9, 2015
@mattjohnsonpint
Copy link
Contributor

The isUtc() function still returns the correct result. The general guidance is to not use the underscore-prefixed properties externally, as many of the moment functions combine multiple properties to achieve the correct result.

Also, by wrapping the moment in another moment, you're cloning it, so the second line of each block isn't manipulating anything other than the moment used by the format call.

That said, I actually don't get the same results as you showed. In the second example, I get "2014-11-21T04:30:00-06:00" - which kept the original offset and did shift the time. So I think there's still a bug here.

@timrwood any ideas?

@petebrowne
Copy link
Author

Yeah I get 2014-11-21T04:30:00-06:00 now, very odd...could DST changes affect this as well?

Just to clarify: I'm not using _isUTC, I just noticed the flag was different while debugging

@timrwood
Copy link
Member

The value passed to utcOffset should be a number or offset string, not a timezone name. utcOffset(0, true) or utcOffset('+00:00', true).

@petebrowne
Copy link
Author

I still get inconsistent results with 0 instead of 'UTC'

var m1 = moment('2014-11-21T10:30:00-06:00');
m1.utcOffset(0, true).format();
// "2014-11-21T10:30:00+00:00"

var m2 = moment.tz('2014-11-21T10:30:00', 'America/Chicago');
m2.utcOffset(0, true).format();
// "2014-11-21T04:30:00-06:00"

@timrwood
Copy link
Member

Ok, I think I see what is going on here.

While utcOffset is being called, it calls moment.updateOffset to keep the offset in sync over dst changes. Because the moment is still in America/Chicago it's updating its offset back to Chicago time.

What you should do is remove the timezone from the moment and then set the offset. Right now there is no official way to remove a timezone, but you could do m2._z = null; m2.utcOffset(0, true).format();.

It is kinda weird to parse into a timezone, then switch to another offset while keeping the same time. Why not just do moment.utc('2014-11-21T10:30:00') instead?

@petebrowne
Copy link
Author

I'm receiving a datetime from an API in UTC and converting it to a user's timezone for display. That seems like a pretty valid use-case?

@timrwood
Copy link
Member

If you want to display a utc datetime in local time you can do this.

moment.utc('2014-11-21T10:30:00').local().format() // 2014-11-21T04:30:00-06:00

If you want to display a utc datetime in a specific timezone you can do this.

moment.utc('2014-11-21T10:30:00').tz('America/Los_Angeles').format() // 2014-11-21T02:30:00-08:00

Maybe I'm missing the use case, but do those help?

@petebrowne
Copy link
Author

The problem I have is that I can't guarantee the original timestamp is UTC, which is why I'm initializing with moment.tz instead of moment.utc

@ashleydw
Copy link

I think I have the same problem here. I am creating a moment object from a utc timestamp, and setting the timezone of the object. However, when returning utcOffset() the offset is always my local offset, not the parsed timezone.

// 1441955500 = 2015-09-11T07:11:40
// Melbourne is +10 hrs
var mo = moment.utc(1441955500, 'X');
mo.tz('Australia/Melbourne');
console.log(mo.format(), mo.format('Z'), mo.utcOffset());

Gives: 2015-09-11T17:11:40+10:00 +10:00 600

So the timezone is set correctly, but utcOffset() returns 600 - my local offset.

@mattjohnsonpint
Copy link
Contributor

@ashleydw - utcOffset() returns a number in terms of minutes East of UTC. 60 minutes in an hour, so 600 / 60 = 10 hours East of UTC, which we show in string format as +10:00.

@ashleydw
Copy link

sorry my fault! I was reading it incorrectly

@wyefei
Copy link

wyefei commented Aug 18, 2017

moment('2017-08-18T20:30:00-04:00').utcOffset() returns -420. Not sure whether this is a bug or I am misunderstanding it. Shouldn't it return -240 instead?

@mattjohnsonpint
Copy link
Contributor

@wyefei - in the example you gave, the offset is your local offset, not the one passed in. You'd have to use moment.parseZone(...) to retain the offset passed in.

@mattjohnsonpint
Copy link
Contributor

To be clear, the bug being tracked in this issue is related to calling utcOffset with the true second parameter (to keep the local time) as shown here, should clear any time zone set on the moment object, and it is not. If you're not passing true, or your not using moment-timezone, this is not the bug for you. 😄

This should be relatively easy to fix, so marking up for grabs.

@mattjohnsonpint mattjohnsonpint changed the title Explicit time zones break utcOffset Explicit time zones break utcOffset(..., true) Sep 8, 2017
@jonseymour
Copy link

For reference, here is a similar example, although with the false parameter.

var moment = require('moment-timezone');
m0 = moment.tz("2019-01-08 11:55", "Australia/Sydney");
m1 = m0.clone().add(6, 'months')
m2 = m0.clone().utcOffset(m1.utcOffset(), false)

m3 = moment("2019-01-08 11:55:00+11:00")
m4 = m3.clone().add(6, 'months')
m5 = m3.clone().utcOffset(m4.utcOffset(), false)

m6 = m2.clone().utc().utcOffset(m1.utcOffset(), false)

vars = [ m0, m1, m2, m3, m4, m5, m6 ]
for (var i in vars) {
	m = vars[i]
	console.log(i, ":", m.toString(), m.utcOffset(), m.isDST(), m.unix())
}
if (m5.toString() != m2.toString()) {
	console.log("assertion failed: display string (2) is showing wrong offset compared to (5)")
}

which results in this output:

0 : Tue Jan 08 2019 11:55:00 GMT+1100 660 true 1546908900
1 : Mon Jul 08 2019 11:55:00 GMT+1000 600 false 1562550900
2 : Tue Jan 08 2019 11:55:00 GMT+1100 600 false 1546908900
3 : Tue Jan 08 2019 11:55:00 GMT+1100 660 true 1546908900
4 : Mon Jul 08 2019 11:55:00 GMT+1000 600 false 1562550900
5 : Tue Jan 08 2019 10:55:00 GMT+1000 600 false 1546908900
6 : Tue Jan 08 2019 10:55:00 GMT+1000 600 false 1546908900

assertion failed: display string (2) is showing wrong offset compared to (5)

The expectation is that the output object 2 and object 5 would be the same. The workaround appears to be converting the input moment object to the UTC timezone (to remove the association with a DST timezone) and then adjust the utcOffset (see m6 above)

@mattshin64
Copy link

Is this still a bug, if not would love to work on it.

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

No branches or pull requests

7 participants