sproutsocial / walltime-js

A JavaScript library for easily translating a UTC time to a "Wall Time" for a particular time zone.
MIT License
121 stars 12 forks source link

Bug: TimeZoneTime.setHours does not update timezone offset for DST #20

Open jacobg opened 11 years ago

jacobg commented 11 years ago

Example:

dt = WallTime.UTCToWallTime(new Date('2013-03-10T01:00:00-0500'), 'America/New_York'); // before DST
console.log(dt.getTimezoneOffset()); // correctly prints 300
dt.setHours(3); // set to 3:00 AM, i.e., after DST
console.log(dt.getTimezoneOffset()); // incorrectly prints 300, but it should be 240
jgable commented 11 years ago

That's a tough one, The only real way to update the @offset and @save are to run the whole WallTime.UTCToWallTime process again, but the TimeZoneTime has no concept of your current region and I'd prefer that it didn't need to.

Let me think about it for a bit.

jacobg commented 11 years ago

What's TimeZoneTime.zone for, if not for region?

jgable commented 11 years ago

I did find that you can get the zone name from the @zone.name value. I think I can work something together for this later tonight.

jacobg commented 11 years ago

Thanks!

jacobg commented 11 years ago

FYI, looks like setDate also has this issue, as well as the other setter methods.

dt = WallTime.UTCToWallTime(new Date('2013-03-15T00:00:00-0500'), 'America/New_York'); // after DST
console.log(dt.getTimezoneOffset()); // correctly prints 240
dt.setDate(1); // set to March 1, i.e., before DST
console.log(dt.getTimezoneOffset()); // incorrectly prints 240, but it should be 300
jgable commented 11 years ago

Yes, they all use the same underlying _updateUTC. This is a hard problem that I'm thinking of not solving.

There was a reason I originally did not implement the setX functionality, it requires the timezonetime to have knowledge of time zones and I'm not sure it should. The goal here isn't necessarily to mimic everything a native date object does; because native dates are broken in regards to timezones.

I'm going to think about it more, but in the mean time you'll have to work around it by using the helper functions that I previously posted; specifically the localDatePlus method.

jacobg commented 11 years ago

This is your puppy so you decide. But I think it is useful to emulate the native Date object with timezone-awareness, and TimeZoneTime does have reference to its timezone. There are other libraries that operate on Javascript objects, so it would be great to just be able to just pass the TimeZoneTime object as if it were a Date object into these libraries.

jgable commented 11 years ago

It has a reference to one zone, not the entire set. If you set a date which falls under another zone line, it would not be possible to accurately get the offset and save without passing down the whole set. Which kind of ties the timezonetime to only being used with a whole zone set.

The goal here is not to be the jQuery of date objects, it's to be the one you use when you want to launch a monkey into space via JavaScript at exactly 12:31:07 PM America/Chicago time.

I'm not ruling it out, I'm just gonna marinate on it for a little longer. Maybe we'll need to have another layer of abstraction above the exposed api. Or maybe we'll just pass down the whole zone set.

jgable commented 11 years ago

I'm leaving this open as a notice to other people, but the official stance on this right now is that for version 1.0 we'll have another WallTimeDate object that will wrap the TimeZoneTime and perform the set functions.

jacobg commented 11 years ago

Thanks so much Jacob!