Using built-in Date in local time zone as a backend of timezone-js is a fundamental mistake

[haozhun]

Suppose my system timezone is Pacific Time. See this example:

> new Date(2012,2,11,1,59,0);
Sun Mar 11 2012 01:59:00 GMT-0800 (PST)
> new Date(2012,2,11,2,0,0);
Sun Mar 11 2012 01:00:00 GMT-0800 (PST)
> new Date(2012,2,11,3,0,0);
Sun Mar 11 2012 03:00:00 GMT-0700 (PDT)

When using the local timezone, there exists date that simply doesn't exist. Therefore, you shouldn't use the built-in Date in the local timezone as the internal representation of the timezoneJS.Date. However, you are doing this all over your code.

A solution to this issue is to use the UTC time of the built-in Date to represent the timezoneJS.Date, which is guaranteed to work, as long as we don't take Gregorian cut-off time into consideration.

I appreciate your work. This indeed is something useful if correct. And it's great that you're willing to help the community and make it available to everyone. However, This issue and issue #36, are both critical enough that warrants immediate fix. Given issue #36 has been outstanding for quite some time, you probably won't have time to fix both bugs any time soon. Currently, when searching for time zone conversion in js on Google, several top results are directing users to try your code here. Therefore, I believe you should put a noticeable warning on this project's main readme to advise any serious user that wants accurate time conversion to avoid using this code and find alternatives.

[longlho]

The UTC time itself doesn't carry any timezone information, plus all timezone rules are based on local time in that specific area so using just the UTC time itself will not be sufficient. This library isn't aim to solve cross-system timezone issues as even Chrome & Firefox themselves handle time inconsistently (Chrome doesn't show daylight saving abbreviation correctly sometimes).

Due to daylight saving, there're indeed date that doesn't exist, which is fine. I don't see a problem here.

The library does store both UTC time & Date object as its core model. Storing 1 but not the other would be very inefficient due to overhead in conversion. Operations such as setDay, setHour... would take a long time when doing in bulk, and given the library is not meant to be async, this is just purely slow.

The license of this library already talks about this library working as is w/ no warranty whatsoever, so posting such notes would be redundant imo.

[haozhun]

The problem is, date like this is perfectly valid

new TimezoneJS.Date(2012,2,11,2,0,0, "Asia/Shanghai");

But this code won't work right in your implementation if the local timezone is America/Los_Angeles

[longlho]

can you be more specific?

[haozhun]

In your implementation, you will instantiate

new Date(2012,2,11,2,0,0)

when some one writes

new TimezoneJS.Date(2012,2,11,2,0,0, "Asia/Shanghai")

This time exists as Shanghai doesn't observe DST and all time is valid. But if the local time zone is "America/Los_Angeles", the first Date is not valid, and will become another time, new Date(2012,2,11,1,0,0) in this case. Therefore, it will yield

new TimezoneJS.Date(2012,2,11,1,0,0, "Asia/Shanghai")

[haozhun]

I don't mean you shouldn't store 2 representations (I don't like it. But I don't fully understand the code and performance is a valid point. Also, it's out of this topic.)

Here, 3 timezones are involved. For clarity,

• target time zone, for example, Shanghai in case of new TimezoneJS.Date(2012,2,11,2,0,0, "Asia/Shanghai")
• UTC
• local time zone, for example, Los Angeles in case the computer configures so system-wide

In your implementation, you have two representation, target and UTC store internally. But that target time is in fact stored in a Date, which is in local time zone (despite you use it as if it's in the target timezone).

I'm not suggesting that you should remove the target representation. Instead, I'm suggesting that you should let the UTC time of the Date object representing the target time match the time in the target timezone. Currently, you are letting the local time of the Date object match the time in target timezone. As local time is not continuous in some regions, your current implementation can lead to the problem I described.

I hope I've made myself clear.

[longlho]

So for each constructor there's a different way of constructing core object. In this case, the UTC time cannot be extracted since we don't know what timezone "Asia/Shanghai" is yet. (new Date(...).getUTCTime() would be incorrect)

Therefore, using the Date object as the proxy and apply set* is the only way to store year, month, day and so on (other than a primitive data structures like list or object, but using these will render set* method crippled).

This is indeed prone to errors and should probably be handled separately but this will require a decent level of effort. You can check out the branches to see if anyone has worked on this. I'll try to fix this if time allows.

[haozhun]

I don't quite get what you were trying to say in your first paragraph. Let's take 2010-1-2 03:04:05 Shanghai, and LA local timezone for example.

You don't know what timezone Shanghai is in, but it's fine. Currently, you'll build 2010-1-2 03:04:05 LA. Why not build 2010-1-2 11:04:05 LA? You can achieve this with new Date(Date.UTC(...)) and replace some getXxx with getUTCxxx. I believe this change won't take any information away (after all, we are just changing from a random city which happened to be LA to UTC).

However, I do agree that this require a decent level of effort because you'll need to change quite some code to adapt with the change of this internal representation. The replacement mentioned above isn't easy. I put 'fundamental' in the title because I understand that to fix it, quite some code will need change.

[NodeGuy]

Not sure I completely follow this conversation but I am confused about how to use the library for perhaps the reason mentioned in this issue.

Following the README, I get this:

var dt = new timezoneJS.Date('10/31/2008', 'America/New_York');
dt.toISOString();
"Fri, 31 Oct 2008 02:00:00 GMT"

But given that New York is UTC−05:00, I would have expected to get this:

"Fri, 31 Oct 2008 05:00:00 GMT"

What's going on?

[vogievetsky]

Sorry to jump on this thread a little late.

I am in need of a timezone library for JS and this one looks great at a glance. I really like the idea of having it have the same API as the native Date but with timezone support. It is a very elegant idea.

@haozhun you have identified an interesting and importnat problem. Are you aware of any other timezone libraries for JS that work of the Olsen data? If so would you mind linking them here? It would be curious to see their implementation.

There are several problems in the code (already pointed out in the current open issues). I have forked the library so I can fix some of the issues that are blocking me. I will be making some Pull Requests shortly to hopefully have there fixes in the main code as well.

[vogievetsky]

@BallBearing I believe you are correct. It is sort of a gotcha in the current code. The first argument in your call ('10/31/2008') is interpreted in your locale and not in the timezone you specified, which is counter intuitive.

Side Note: Does anyone know how to set your system locale for the purposes of a unit test running in node.js? I know you can always literally tell your OS that you are in, say, America/Los_Angeles but is there any way to do it through a command line argument? I have not looked into it but if anyone knows please let me know.

Cheers

[moll]

@vogievetsky Try setting either the TZ env variable or look into using libfaketime.

[najamelan]

This is quite frustrating... The readme seems to imply that you can work with dates independant of the local browsers timezone. After writing a bunch of code relying on this lib I figure out that it's just fundamentally broken. I think you should be clear about this in the README. The below snippet easily shows the problem, and honestly I'm wondering what this library can do that native Date can't if this snippet doesn't work:

<!doctype html>
<html lang="en">
<head>
    <meta charset="UTF-8">
    <title>Test TimezoneJS</title>

    <script type="text/javascript" src="timezone-js/src/date.js" /></script>
    <script type="text/javascript" src="jquery-2.0.3.js" /></script>

    <script type="text/javascript">

        timezoneJS.timezone.zoneFileBasePath = "tz/"
        timezoneJS.timezone.defaultZoneFile  = "europe"
        timezoneJS.timezone.init()

        var dt = new timezoneJS.Date( "1970-01-01T00:00", "Europe/London" )

        console.log( dt.getTime() )

        // TimezoneJS commit bb34845 and v0.4.6 and v0.4.5

        // outputs 0        when my system timezone is set to Europe/London
        // outputs -3600000 when my system timezone is set to Europe/Paris

    </script>

</head>
<body>

</body>
</html>

[mde]

Yes, having timezones independent of the browser's local timezones is precisely the point of this library, and big companies like Apple and Merrill Lynch, and small companies like Uber have used it happily for just that, so the likelihood that it's "fundamentally broken" is pretty small. What is more likely is that your don't understand well how it works, or doesn't support some part of the Date API that you expect. (@longlho, is this the thing with the string-arg to the constructor?) Have you actually looked at the tests? They provide a good example of the API, and show what is expected to work (and indeed, does work assuming tests are passing).

[longlho]

It is the thing with the string arg to the constructor. Unless we don't use Date to parse that and implement our own RFC-compliant parser and deal w/ various Date string formats I can't see a way around it. If we take out the support for it it's not fully compatible w/ the Date object itself. Hmm?

[mde]

Is the problem here more the heuristic of knowing it's a datetime string instead of the timezone string param? Or is it a problem with parsing the datetime string? Even different JS engines don't parse those strings in a consistent way. For example, V8 parses the string 'Monday, Sept. 9, 2013' just fine, but in SpiderMonkey it's an invalid date.

Looks like a single string arg is indeed part of the spec: http://www.ecma-international.org/ecma-262/5.1/#sec-15.9.3.2, so handling that case would be great if we can.

[longlho]

We have a regex that detects whether it's a timezone string param so the former is not an issue. The issue here is that since different JS engines parse datetime string differently, do we wanna maintain that inconsistency as well? How strict do we want to deal w/ datetime string parsing? Relying on browser date parsing would maintain the expectation when it comes to browser-specific behavior.

We can definitely write our own parser it shouldn't be too hard. Just lmk what your thoughts are.

[mde]

I think we just defer to the JS engine's Date.parse. Easy to implement, and it's 100% compatible with that engine's native Date. Should be trivial to add that, right?

[longlho]

That's what we're doing already but Date.parse parses the time string in its local timezone, which causes all sorts of problem since we need the time string to lookup timezone data, but the time string was meant to be in that timezone already. It becomes sort of chicken & egg afterwards, or maybe I'm not thinking clearly enuf.

Right now as @BallBearing and @vogievetsky mentioned, the constructor with time string argument usage is sorta counter-intuitive, specifically for the reason above.

[mde]

This is only a problem for datetime strings that include timezone info, correct?

[longlho]

Correct

[mde]

Then I don't think it's unreasonable to say we don't support that, since our way of setting a timezone is via that second param.I'm not sure what you'd do in that case, but you could either detect and strip that timezone info, or just let the Date.parse do its thing with it. Either of those would be defensible. We would just need to include some obvious documentation to state that passing the timezone info as part of the parseable datetime string is not supported.

[najamelan]

Thanks for reacting to this.

This is only a problem for datetime strings that include timezone info, correct?

Did you have a look at the snippet I posted? The string does not contain timezone information.

If the problem is really as simple as you describe, then the fix would be as well I think:

1. use Date.parse
2. determine the browsers offset (this can help you with DST)
3. use the offset to get UTC time
4. from UTC time adapt to the timezone asked in the second parameter of the constructor

I have not made a thorough study of how timezone-js is implemented, but I have the impression that the problem rather lays with not correctly getting DST information from the olson files (step 4), and that being rather hard (albeit not impossible) if you don't parse the string yourself, since olson doesn't define the DST switches as ms since UTC epoch, but as local dates.

For me the whole point of having this functionality is because this conversion to UTC ms is the hard part for a user too. Say someone wants to input historical events (say a description of events in WWII), than all the resources that user will have will say something like on 1944 may 7th at 6 am the troops landed on the beaches of Normandy, and you don't want to tell them they have to input it as UTC. Its just not an option for an end user.

note that walltime also only provides conversion in one direction (actually I have not found anything that does the other way around).

[longlho]

There're definitely 2 issues here, and my bad for getting confused with @mde discussion. As you said Olson does not have tz rules in epoch ms so any input coming in that requires parsing, we use Date.parse instead. That includes Date in string format and epoch ms.

• For Date in String format we can certainly write a simple parser as long as the input is reasonably formatted. However, if the input includes tz in itself, it's indeed very hard to correctly determine the correct timezone.
• For Date in epoch ms format, daylight saving becomes screwy as you said.

For the case you mention, timezonejs.Date(1944, 4, 7, 6, <tz>) should work.

[najamelan]

For the case you mention, timezonejs.Date(1944, 4, 7, 6, <tz>) should work.

Ergh, no it doesn't. Changing the sample that I posted above with this line gives exactly the same results (depending on the system timezone settings): var dt = new timezoneJS.Date( 1970, 0, 1, 0, 0, "Europe/London" )

[jboolean]

In agreement with najamelan. I just spent a long while replacing a lot of built-in Date usage in a system with timezoneJS, which looked amazing. I just realized that it doesn't really fix the main symptom here, which is that totally valid dates in the specified timezone cannot be created. This is very frustrating.

[slominskir]

Wow.... I guess the fix is to use moment.js instead

[jboolean]

My solution was to write my own timezone conversion utility. Not easy, but a lot smaller than momentjs and tailored to my purposes. Unfortunately not open source.