Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handling timezones regardless of server TZ setting. #24

Open
wants to merge 57 commits into
base: master
Choose a base branch
from
Open

Handling timezones regardless of server TZ setting. #24

wants to merge 57 commits into from

Conversation

s2young
Copy link

@s2young s2young commented Aug 7, 2013

I was running into parsing differences on different boxes and realized it was the TZ setting. The addTZ method in ical.js was only superficially setting the timezone. If you console.log the date object, the TZ will be both GMT and whatever TZ you set in addTZ. The timestamp (getTime function), which should always be UTC, was not changing.

I needed date parsing where I could take the floating time in the ICS file and tell the date object what timezone it needs to be in. The node-time module does this with its setTimezone method (passing in true in the second position).

With this change, I can parse the same ICS feed on any computer (with any TZ setting) and the date objects are identical. I hope this is helpful. Please let me know if you think I missed something.

Stuart Young added 10 commits August 7, 2013 16:31
…n the date object, because I don't think that changes the nature of the underlying date object.
…ne method in addTZ function. This uses node-time 0.9.2.
…ean on the node-time object. This makes it easier because JS dates always have a time, but in this case the time is not valuable.
@peterbraden
Copy link
Owner

The issue with this is that it only works in node, whereas I'd like to keep the library browser compatible - can you use node-time in your own code and monkey patch the library?

@s2young
Copy link
Author

s2young commented Aug 10, 2013

Understood. I didn't realize the module was intended for client-side use. But I can see problems with making use of it on the client-side. Maybe you can clear it up for me.

Just to illustrate my problem a bit further, take this example using 12 pm (noon), Aug 10, 2013:

var dNow = new Date(2013,07,10,12,00,00); 
console.log(dNow);
dNow.tz = 'America/New_York';
console.log(dNow);

The printed output (on my machine, which is on Central Daylight Time) is as follows:

Tue Aug 10 2013 12:00:00 GMT-0500 (CDT)
{ Tue, 10 Aug 2013 17:00:00 GMT tz: 'America/New_York' }

That second line is confusing, because the tz now says 'America/New_York' but the GMT time is still expressed relative to parsing timezone (CDT). I can see this being especially problematic on the client-side because I don't know of any good modules out there that do timezone conversion using the Olsen tz name as the input. So you're stuck with a date object that you cannot express properly.

How would one fix this on the client-side? I still reserve the right to be missing something completely here. I seem to have a mental block when it comes to dates and times!

Stuart Young added 29 commits August 28, 2013 13:21
…he fromString method so the array of date instances that comes back with the .all() method doesn't include the cumulative dates during a large import.
@0rvar
Copy link

0rvar commented May 14, 2020

Any news on this? This library is useless due to this bug.

EDIT: My workaround:

import * as dateFns from 'date-fns';
import tzOffset from 'tz-offset';

const NOW = new Date();

/**
 * Fixes a datetime constructed incorrectly by the ical library
 * Only rrule and exdate dates seem to be incorrectly constructed
 * @param datetime the fauly datetime
 * @param tz The ics timezone
 */
export function fixFaultyIcalTime(datetime: Date, tz: string): Date {
  const offset = dateFns.differenceInHours(tzOffset.timeAt(NOW, tz as tzOffset.Timezone), NOW) + 1;
  return dateFns.subHours(datetime, offset);
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants