change default timezone #15

Closed
fabriziofortino opened this Issue Jul 19, 2013 · 74 comments

Comments

Projects
None yet

It would be great to have a method to change the default timezone (eg: when the application starts moment.tz.setDefault('America/New_York')). Then, all the subsequent calls to moment() will automatically set the timezone with the default one.

peruzzo commented Jul 19, 2013

+1

Owner

mj1856 commented Jul 21, 2013

I like this request, because it would allow a user to follow the same thought processes as when using date_default_timezone_set in PHP, for example.

Owner

timrwood commented Jul 23, 2013

I suppose setting the zone on the moment.fn._z could work for this.

moment.tz.setDefault = function (name) {
    moment.fn._z = getZoneSet(name);
};

However, this could cause an issue. Because of the way moment.updateOffset works, there could be some weird behavior when changing the defaults.

var a = moment.utc();
moment.tz.setDefault('America/Los_Angeles');
a.format('ZZ'); // +0000
a.millisecond(0); // triggers moment.updateOffset
a.format('ZZ'); // -0700

Because the offset could change any time the moment is mutated, we need to call moment.updateOffset to make sure the offset is in sync with the moment. If a moment was created without a zone, then the zone was set without calling moment.updateOffset, the moment is in a limbo state.

+1 on being able to set a default. I'm currently implementing app-wide timezone support and it's a bit fiddly to have to get the user's timezone preference for the .tz() every time we make a moment call

vddgil commented Sep 6, 2013

+1

ste93cry commented Sep 7, 2013

+1, @timrwood any news regarding an official method in the next release to set the default timezone?

vkadam commented Sep 21, 2013

+1

+1 👍

kamote commented Oct 3, 2013

+1

+1

+1

mkoryak commented Oct 7, 2013

+2 (is this allowed? )

ismriv commented Oct 26, 2013

+1

+1

A +1 from me, too.

Owner

mj1856 commented Oct 31, 2013

Ok, ok, enough with the +1's already. 😄

I'm planning some work on moment-timezone soon enough and will look into implementing this.

+1 lol

Owner

mj1856 commented Oct 31, 2013

Somehow, I knew that was coming...

@ghost ghost assigned mj1856 Oct 31, 2013

mhayes14 commented Dec 8, 2013

+1
Just to show that the interest is still here for this. Not sure how one would go about it though - completely new to moment-timezone, about to check it out.

+1 👍

@timrwood does this mean that as long as I don't mutate a moment object, it should work fine?

Hi Matt,
I hope you will have the time for this nice little feature. Apart from setting the default timezone Moment and Moment Timzone are really great!

+1 , I need this too.

nick commented Jan 12, 2014

+1 😄

@mj1856 any news regarding this? This project seems a bit abandoned unlikely moment.js

Owner

mj1856 commented Jan 27, 2014

@ste93cry Not abandoned, no. But needs some TLC. @timrwood started both projects, but has transitioned it to the moment contributors. I volunteered to take on moment-timezone, but I've been super busy with some major work/life changes. I will get into it more soon. Promise. Thanks for being patient.

If anyone with more time wants to jump in, feel free. This is an open-source project after all!

+1

+1 beer for success!

+1

pjnovas commented Apr 4, 2014

+1

+1

Alright guys, we get it. Everybody wants this. Until one of us is willing to step up and do it, please let's stop commenting "+1." There's a subscribe button on the right if you just want notifications on this issue.

The default timezone is that of the browser. The only other one you're likely to need frequently is your server's. So add a tiny moment.mytz() function which wraps moment.tz() and adds your server's timezone as the last argument. Then call moment.mytz('1979-11-26') rather than moment.tz('1979-11-26', 'America/New_York')

moment.mytz = function(){
    arguments = [].slice.call(arguments)
    arguments.push('America/New_York')
    return moment.tz.apply(moment, arguments)
}

froodian commented Jun 6, 2014

The only other one you're likely to need frequently is your server's

This isn't necessarily true. In our business-to-business product, we have clients with multiple users around the world that all coordinate and work in a common company time zone (set via a preference), which can be distinct from both browser and server.

@froodian, why wouldn't the common company timezone also be the server timezone?

froodian commented Jun 6, 2014

These are client companies, so if the company is based in California, for instance, we would want to use PST, even though our servers are in UTC and the user's browser may be in EST (or may be in PST).

Naatan commented Jun 6, 2014

@vladtscripts that's really besides the point isn't it? The point is that there are penty of use-cases where setting the timezone (for all of moments functions) is still relevant.

For @froodian's use-case, one can set moment.myTimezone = 'America/New_York' and have moment.mytz() do arguments.push(moment.myTimezone) instead of arguments.push('America/New_York').

moment.mytz = function(){
    arguments = [].slice.call(arguments)
    arguments.push(moment.myTimezone)
    return moment.tz.apply(moment, arguments)
}
moment.myTimezone = 'America/New_York' // default, change at runtime as needed

+1

@flochtililoch flochtililoch referenced this issue in kylestetz/CLNDR Jul 21, 2014

Closed

Timezone Offset #103

@froodian's is a common use case indeed, one that we also have 👍

+100
The only missing part of the puzzle!
Otherwise moment.js is just (brilliant) library to format dates ...

+1

natiz commented Oct 12, 2014

+1

+1, is this supported?

xwartz commented Oct 22, 2014

+1

+1

Contributor

elad commented Oct 28, 2014

@timrwood @mj1856 Could you please better explain the issues with implementing this feature? I'd really like to have it.

In the code you posted:

var a = moment.utc();
moment.tz.setDefault('America/Los_Angeles');
a.format('ZZ'); // +0000
a.millisecond(0); // triggers moment.updateOffset
a.format('ZZ'); // -0700

I assume the expected behavior is that a will retain its UTC timezone because it was created before setting the default timezone. So it should either have the local timezone or, in this case since .utc() was used, UTC. I would also assume that the timezone is "sticky" and kept inside a.

Am I correct in my assumptions about expected behavior?

Now, what I don't quite understand is this:

Because the offset could change any time the moment is mutated, we need to call moment.updateOffset to make sure the offset is in sync with the moment. If a moment was created without a zone, then the zone was set without calling moment.updateOffset, the moment is in a limbo state.

Do you mean that there was no "initial" moment.tz.setDefault call and thus the actual timezone name -- as opposed to the offset -- cannot have been kept inside a? And that in turn would make moment.updateOffset use the default which is different than what's expected?

If yes, I think this is an issue that might be solved with a hard-coded 'UTC' value for the .utc() case plus some documentation on proper use of moment.tz.setDefault.

If not, could you folks please elaborate for those of us who are not deeply familiar with the moment.js code? :)

Contributor

elad commented Oct 29, 2014

Okay, I have started working on an implementation the way I understand it. For discussion purposes only...

My changes are at elad/moment and elad/moment-timezone and the test program is at elad/moment-default-timezone. You can try it out like so:

git clone https://github.com/elad/moment-default-timezone
cd moment-default-timezone
npm install
node test

Please take a look at the comments in test.js to understand what it does. Here's the output from my system:

air:moment-default-timezone elad$ node test
Local time      : 2014-10-29T17:29:54+02:00     Timezone: null
New York        : 2014-10-29T11:29:54-04:00     Timezone: America/New_York
Los Angeles     : 2014-10-29T08:29:54-07:00     Timezone: America/Los_Angeles
Local time  [*] : 2014-10-29T17:29:54+02:00     Timezone: null
New York    [*] : 2014-10-29T11:29:54-04:00     Timezone: America/New_York
Los Angeles [*] : 2014-10-29T08:29:54-07:00     Timezone: America/Los_Angeles
air:moment-default-timezone elad$ 

Basically it creates and clones a bunch of moments to test the default timezone and the stickiness on existing moment objects.

Comments welcome! :)

Owner

timrwood commented Oct 29, 2014

Ok, giant brain dump.

We have a few different things we are working with.

  • Timestamp: A number of milliseconds from the unix epoch. 1414601438469
  • Offset: The number of hours that the clock on the wall is different than Big Ben. -07:00
  • Timezone: A named group of offsets and timestamps based on the DST rules for that location. America/Los_Angeles

We store the timestamp using the built in Date object as _d, the offset using a number as _offset, and the timezone using a moment.tz.Zone object as _z.

Because the Date object does not support getHours and friends for arbitrary offsets, we work around it by manually adding the offset to the _d date object and using getUTCHours and friends.

This is why the underlying Date object sometimes appears to be wrong when debugging via the dev console.

+moment().zone('-05:00')    // 1414602020495
+moment().zone('-05:00')._d // 1414584020495

Now, when setting the timezone, we first need to find what offset to use. Because of DST, a timezone can have more than one offset.

new Date(2014, 0, 1).getTimezoneOffset() // 480
new Date(2014, 6, 1).getTimezoneOffset() // 420

This is why we need moment.updateOffset. Whenever we set any of the timestamp using .month(5) and friends, we need to check if the offset is still correct for that timezone. Changing from January 1 to July 1 would require that we update the offset from -08:00 to -07:00.

The idea for setting the default from my original comment was to set the _z timezone property on the moment prototype. However, because we need to call moment.updateOffset any time we change the timezone or timestamp, any moment that was created without a timezone would have the wrong offset.

var zone = getZone("America/Los_Angeles")
var a = moment.utc();
a._offset // 0
moment.fn._z = zone
a._offset // 0 still as we have no way to notify previously created moments of the timezone change
a._z === zone // true, as it is looked up from the Moment.prototype
moment.updateOffset(a);
a._offset // 420

I think a possible solution would be to add a moment.updateOffset call to the moment constructor. (An issue with this is the possibility of moment.updateOffset in turn creating a moment and causing infinite recursion.)

If a moment.updateOffset call is added, we can add a line here that sets the _z property to the default timezone. Something like this.

moment.updateOffset = function (mom, keepTime) {
    var offset;
    if (!mom._z && defaultZone) {
        mom._z = defaultZone;
    }
    if (mom._z) {
        ...
    }
};

This has the benefit of only affecting newly created moments, and not updating any previously created moments.

Owner

timrwood commented Oct 29, 2014

@elad, I now see that this is basically what you did in your fork, only without the _zn property. :)

Contributor

elad commented Oct 29, 2014

Hi @timrwood, thanks for looking into this issue again! :)

I tried using the _z property, but that didn't work out, maybe because I wasn't doing it properly. I made it work by keeping the zone name (or lack of it for local time) in _zn.

If I change setDefault to set moment.fn._z, and then use _z all over, the timezone doesn't stick and any changes to the default timezone affects all moments regardless of when they were created. If using _z is the correct approach, I'd appreciate some pointers on making it work and adjust the code accordingly.

Other than the issue of _z vs. _zn, is my implementation of it correct? I'm especially curious about setting config._zn. If so, I'd be happy to work this into a pull request along with necessary tests and so on.

Owner

timrwood commented Oct 29, 2014

I think you will want just these three lines in moment.js. The relevant tests should just make sure moment.updateOffset is called during moment() and moment.utc().

Then, instead of storing the zone as moment._zn, maybe create a local variable around here that moment.updateOffset can check for, and in tz.setDefault, set it to a Zone object based on the string passed in. I think that should work.

You may also want to add tests to make sure you can unset the default zone.

Contributor

elad commented Oct 29, 2014

I tried your suggestions but it loses the stickiness, so I must be missing something:

  • In moment.js I kept only the call to moment.updateOffset
  • In moment-timezone.js, I:
    • Added var defaultZone = null; above the moment.updateOffset function declaration

    • Modified moment.updateOffset to have the following lines after var offset;:

      if (!mom._z && defaultZone) {
          mom._z = defaultZone;
      }
      
    • Modified moment.tz.setDefault to look like this:

      moment.tz.setDefault = function(name) {
          defaultZone = name ? getZone(name) : null;
      }
      

The output from my test program is:

air:moment-default-timezone elad$ node test
Local time      : 2014-10-29T14:04:44-07:00     Timezone: America/Los_Angeles
New York        : 2014-10-29T17:04:44-04:00     Timezone: America/New_York
Los Angeles     : 2014-10-29T14:04:44-07:00     Timezone: America/Los_Angeles
Local time  [*] : 2014-10-29T14:04:44-07:00     Timezone: America/Los_Angeles
New York    [*] : 2014-10-29T17:04:44-04:00     Timezone: America/New_York
Los Angeles [*] : 2014-10-29T14:04:44-07:00     Timezone: America/Los_Angeles
air:moment-default-timezone elad$ 

I think this is because moments created before a timezone is set have their _z equal to null. Then in updateOffset there's no mom._z but there is a defaultZone so we use it. That's why I added the code to make _zn sticky: so that updateOffset can refer to the zone that was relevant when the moment was created rather than the zone that is relevant at the time of the update.

Owner

timrwood commented Oct 29, 2014

Ah, right. What about something like this?

if (mom._z === undefined) {
    mom._z = defaultZone;
}

That way, mom._z will only be set if it hasn't been set before. If defaultZone is null, the moment will keep a null zone and it won't update to the new default timezone when updateOffset is called later on.

Contributor

elad commented Oct 30, 2014

Yep, that works:

air:moment-default-timezone elad$ node test
Local time      : 2014-10-30T15:23:11+02:00     Timezone: null
New York        : 2014-10-30T09:23:11-04:00     Timezone: America/New_York
Los Angeles     : 2014-10-30T06:23:11-07:00     Timezone: America/Los_Angeles
Local time  [*] : 2014-10-30T15:23:11+02:00     Timezone: null
New York    [*] : 2014-10-30T09:23:11-04:00     Timezone: America/New_York
Los Angeles [*] : 2014-10-30T06:23:11-07:00     Timezone: America/Los_Angeles
air:moment-default-timezone elad$ 

I'll push the changes and look into writing tests tomorrow morning when I get to be in front of a computer again. :)

Contributor

elad commented Oct 31, 2014

@timrwood, regarding moment.utc() - a moment created in UTC mode has its ._z set to null. Is that a desired behavior or does it make sense to explicitly set it to getZone('UTC')?

Update: Added basic tests. They all pass, but please take a look to see if I forgot anything or am doing something wrong.

rinatio commented Nov 17, 2014

+1

@elad elad referenced this issue Nov 17, 2014

Merged

Default timezone #152

Contributor

kashifshamaz21 commented Dec 13, 2014

@timrwood Do we have any plans of merging this nice and highly +1'ed feature in near releases? From above comments, it seems elad's pull is working fine.

Contributor

elad commented Dec 13, 2014

It's been added to the 2.9 milestone, see moment/pull/2054.

Owner

timrwood commented Jan 13, 2015

This has been released as moment-timezone@0.3.0.

@timrwood timrwood closed this Jan 13, 2015

Fantastic, thanks!

The only downside is that for people who just want to make the default timezone UTC such as myself, we have to load in moment-timezone. I'm going to continue using moment.utc() for now, it would be nice if there was a separate option for moment to make UTC the default rather than calling moment.utc() each time. I'm not sure how many people would actually want such functionality though, plus I guess moment-timezone is small enough to pull in without worrying about it much.

Agree with @bluehayes. I just need to set default timezone, but have no other need for moment-timezone. It would be awesome to just have setDefaultTimeZone() in moment.

Contributor

elad commented Jan 14, 2015

@bluehayes, @troydemonbreun: Please open a different issue for this feature in the moment project, this one is about setting an arbitrary default time zone.

The implementation should be easy and I don't mind doing that as well if it's desired, so feel free ping me from the new issue. :)

rinatio commented Jan 16, 2015

Agree with @bluehayes and @troydemonbreun

+1

Updated solution for just in moment.js without moment-timezone:

This is what i'm doing now, right after I call moment, I call this:

var tz_offset = -7;
moment.updateOffset = function (m) {
    return m.utcOffset(tz_offset);
};

every moment created after that is in the correct timezone. Currently I'm using my server to provide tz_offset so it accounts for DST.

CAVEAT!!!!!!

As @mj1856 stated in the comment below this, depending on how the updateOffset() is used, it would only be correcting for a SINGLE timezone, and if that does not meet your needs, then keep it in mind.

Owner

mj1856 commented Oct 24, 2015

@coreyzev - be very careful. A single number can only represent the offset for a specific instant in time. You server might give you -7 or -8 depending on when DST is currently in effect, but that may or may not be the correct offset for the value you are working with in the moment object.

Example, in US Pacific Time, it's currently 2015-10-24T13:00:00-07:00. If my server sends -7 to the client, but then in the client I am working with a different date and time, such as moment('2015-11-02T13:00:00').utcOffset(-7), then I have incorrectly applied -7, as the correct offset for that date should be -8.

hi can someone help me pls?
Currently we are displaying timestamp in GMT.
Now I am trying to display the timestamp in PDT format.
tried with $('#start_time').val((moment().tz(($('#fStartDate').val()),'America/Los_Angeles').format("YYYY-MM-DDTHH:mm:ss"))+'Z');

here am getting the time and date in UTC. also tried with local time. but in both cases i couldnt get PDT time. if removed UTC in fStartDate func, then i get local time. but even used tz am not getting PDT time. Please help me what need to be done to show the time stamp in PDT with moment timezone. also tried by setting default timezone but that too doesnt work.

Owner

mj1856 commented Dec 15, 2015

@Shobana16 - I see many things wrong in that code. But commenting on a unrelated and closed issue is not the way to ask for help. Your best bet is to ask at StackOverflow and tag your question with [momentjs]. If you prefer Github, open a new issue. Thanks.

dmsouza commented May 2, 2016

+1

@mj1856 mj1856 removed the planned label May 10, 2016

@mj1856 mj1856 locked and limited conversation to collaborators May 10, 2016

Owner

mj1856 commented May 10, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.