moment-timezone icon indicating copy to clipboard operation
moment-timezone copied to clipboard

utcOffset returns wrong value for some timezones

Open khataev opened this issue 2 years ago • 3 comments

Moment-timezone version which you use:

Version: 0.5.33

Note: many issues are resolved if you just upgrade to the latest version

Issue description:

For some timezones results that give utcOffset and moment() are different, ie for Europe/Belgrade. Currently it is GMT+2:

`Now: ${currentMoment.format()}`
// Now: 2022-06-09T12:12:08+03:00 

moment.tz.zone("Europe/Belgrade").utcOffset(moment().unix())
// -60 - wrong, 1hr

moment.tz("Europe/Belgrade").format()
// 2022-06-09T11:07:03+02:00 - correct

Sandbox

khataev avatar Jun 10 '22 20:06 khataev

I think this comes down to a misunderstanding of what moment().unix() returns. As described in the documentation:

This value is floored to the nearest second, and does not include a milliseconds component.

tz.zone().utcOffset() is expecting a value in milliseconds, but you're providing one in seconds.

ms = moment().valueOf();  // 1654910732690
s = moment().unix();      // 1654910732

moment.tz(ms, 'Europe/Belgrade').format();  // "2022-06-11T03:25:32+02:00"
moment.tz(s, 'Europe/Belgrade').format();   // "1970-01-20T04:41:50+01:00" 

Your example code is asking for the offset of a date in 1970, which is why you're getting -60.


As a side note, please provide a more descriptive issue title. Creating a issue just titled "Wrong" is not helpful for anyone.

gilmoreorless avatar Jun 11 '22 01:06 gilmoreorless

@gilmoreorless Thank you for the answer, got it! I also can give a feedback - that misunderstanding partially comes from the documentation as well:

zone.utcOffset(timestamp); // 480

it is not clear what is timestamp, unless you read about the difference just right there where you point to. Maybe it would be helpful to add a hint, that timestamp here is with milliseconds (or result given by valueOf).

Anyway, thank you!

khataev avatar Jun 11 '22 09:06 khataev

Maybe it would be helpful to add a hint, that timestamp here is with milliseconds (or result given by valueOf).

Yep, that's a good point. It's not 100% clear what the input format should be.

gilmoreorless avatar Jun 11 '22 12:06 gilmoreorless

@khataev it is now clarified in the documentation: https://github.com/moment/momentjs.com/commit/d0dc7f13dfbb08de1774ab01fdb4590293f558e4

Just a few notes:

  • in general all APIs work with millisecond-based timestamps (because this is the default in JavaScript Date)
  • in the examples in the documentation the timestamps are in ms (because they're too long)

.unix() is just a special function to use when you need a second based ts, for a particular reason, so it's not as ambiguous as you say, if you've written any js/moment code.

@gilmoreorless thanks for the help!

ichernev avatar Aug 25 '22 12:08 ichernev