mde / timezone-js

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

getUTCDateProxy sometimes returns 'Invalid Date' #125

Closed tinytim84 closed 10 years ago

tinytim84 commented 10 years ago

When using pre-parsed JSON data, some timezones were returning getUTCMinutes and getTimezoneOffset as a string (e.g. "30-60").

longlho commented 10 years ago

Can u provide some code samples please?

LH

On Jan 6, 2014, at 9:02 AM, tinytim84 notifications@github.com wrote:

When using pre-parsed JSON data, some timezones were returning getUTCMinutes and getTimezoneOffset as a string (e.g. "30-60").

You can merge this Pull Request by running

git pull https://github.com/tinytim84/timezone-js master Or view, comment on, or merge it at:

https://github.com/mde/timezone-js/pull/125

Commit Summary

getUTCDateProxy sometimes returns 'Invalid Date' File Changes

M src/date.js (2) Patch Links:

https://github.com/mde/timezone-js/pull/125.patch https://github.com/mde/timezone-js/pull/125.diff — Reply to this email directly or view it on GitHub.

tinytim84 commented 10 years ago

var _tz = timezoneJS.timezone; _tz.loadingScheme = _tz.loadingSchemes.MANUAL_LOAD; _tz.loadZoneDataFromObject(tzObj);

var dateObj = new timezoneJS.Date('1/6/2014', "Etc/UTC"); dateObj.setTimezone("Africa/Algiers"); //dateObj.setTimezone("America/New_York");

console.log(dateObj);

tzObj is the JSON output for all cities which I retrieved using: https://github.com/mde/timezone-js#pre-parsed-json-data

"Africa/Algiers" will fail, but "America/New_York" will work. The only commonality I could find between timeszones that fail are that they all appeared to come from countries that don't use DST.

radhikalism commented 10 years ago

I think due to #118, the offset field is now a string when loading preparsed data, and this isn't accounted for in some code paths.

Isolated example:

> new timezoneJS.Date(2014, 1, 1, 2, 0, 0, 0, 'Europe/Amsterdam').getTimezoneOffset() // goes through getAdjustedOffset
-60
> new timezoneJS.Date(2014, 1, 1, 2, 0, 0, 0, 'Asia/Kolkata').getTimezoneOffset() // never adjusted
"-330"

JS arithmetic works on strings that are number-like, but only when the operation is unambiguous. + falls back to string concatenation, so this is unreliable.

Since the offset value is expected to be a number after the zone data has been loaded, I'd suggest tweaking line 1022:

      var off = +z[0];

Having said that, lazy-loaded data doesn't appear to read offsets as strings (they're plain old numbers). That makes it harder to write a failing unit test for this. Should it be made consistent too?

longlho commented 10 years ago

hmm maintaining consistency between reading from pre-parsed one & lazy-loaded data would be ideal but I see what you're saying. The "pre-parsing" process essentially is eagerly loading all the data then serialize out what's loaded in memory into JSON. Ideally they should be identical. Can you write a simple unit test & make some notes to demo the differences in behavior?