mde / timezone-js

DEPRECATED: Timezone-enabled JavaScript Date object. Uses Olson zoneinfo files for timezone data.
824 stars 183 forks source link

Preparsed json timezone file with string offset causes incorrect calculation #152

Closed ewjoachim closed 8 years ago

ewjoachim commented 10 years ago

I'm having really strange timezone conversions that seem to come from the fact that offsets are not always converted to float when loading preparsed json files.

Consequences are that, for example, in getUTCDateProxy, I can get results like "600" + 40 = "60040" instead of 640 which makes bad results.

I've created a JSfiddle to explain the case : http://jsfiddle.net/4znkX/4/

In the file, I load the exact same data that was generated from node-preparse.js (but I load it directly to avoid cross domain / cdn loading hassle), then I create a normal Date and a localized timezoneJS.Date().

You can see that the localized date is way off. Tahiti being UTC+10, (offset 600) we can have up to about 60 000 minutes shift (40 days)

I think the cause can lie somewhere around pull request #118.

ewjoachim commented 10 years ago

I don't have a complete understanding of the consequence this may have, but it seems that changing:

var off = z[0];

here, to

var off = parseInt(z[0]);

solves the problem for me. But it could probably be better solved elsewhere so that we don't spend our time parsing values that have been parsed already.

iamnoah commented 10 years ago

+1 this is a pretty big deal, Why does node-preparse force all values to be strings when it expects numbers for the timezone offset? Really need to hear from @mde on this.

mde commented 10 years ago

I'm travelling in Japan for work right now, so my schedule is a bit overloaded, but I'll try to take a look at this sometime today.

mde commented 10 years ago

node-preparse seemed to be completely broken, tests were not working at all with fresh tz data. Could you pull the fixes from master in, and write a failing tests for this?

sdemjanenko commented 9 years ago

This was fixed in #160