mde / timezone-js

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

Lazy load happens async by default, should be sync #26

Closed davidmarble closed 12 years ago

davidmarble commented 12 years ago

There are a couple errors here. Per the readme, I used the following code on page load:

timezoneJS.timezone.zoneFileBasePath = '/tz';
timezoneJS.timezone.init();

// Then I tried to create a date with a timezone
var dt = new timezoneJS.Date();
dt.setTimezone('America/Los_Angeles'); 

This results in an error. Internally, date.js reports loadedZones['northamerica'] == true, because it sets it so before it actually loads the file in loadZoneFile:

this.loadedZones[fileName] = true;
return builtInLoadZoneFile(fileName, opts);

That's the first error. This shouldn't be set until some kind of success from the loading occurs.

Also, in getTzInfo, even if this.loadedZones[fileName] returned false, you have this code:

if (!this.loadedZones[zoneFile]) {
      // Get the file and parse it -- use synchronous XHR
      this.loadZoneFile(zoneFile, true); // <-- Don't you mean {async: false} ?
}

The second parameter in the loadZoneFile call doesn't seem to be for anything -- loadZoneFile expects an object of options. More likely you mean {async: false} ?

For now, as a workaround, the above issues can be circumvented by just forcing async to false on init.

timezoneJS.timezone.init({async: false});
longlho commented 12 years ago

loadedZones[zoneFile] was set beforehand to avoid adding a zone file. I can re-assess this. However, changing default init to sync wouldn't be ideal.

The correct method for your use case would be to to timezoneJS.timezone.init({ callback:callbackFn })

In terms of loadZoneFile(zoneFile, true) it's pretty much the same as loadZoneFile(zoneFile, 'foo') in terms that the async property is not there. THis can be tightened up.

NodeGuy commented 11 years ago

This is still a bug because the README still implies sync. I almost gave up on using the library until I found this issue.

mde commented 11 years ago

@BallBearing Please feel free to send us a pull-request with improved docs. We would love to make it easier for people to use this.