jens-maus / node-ical

NodeJS class for parsing iCalendar/ICS files
Apache License 2.0
124 stars 52 forks source link

Keep string properties of VCALENDAR #141

Open Aggtaa opened 3 years ago

Aggtaa commented 3 years ago

Many different ICAL generators, namely MS Exchange, Google Calendars, Apple Calendar, add string properties to VCALENDAR that should be kept during parsing.

E.g. X-WR-CALNAME is an extremely important string property, as well as X-WR-RELCALID, X-WR-CALDESC, X-PRIMARY-CALENDAR and actually all others.

However, current node-ical contains code that intentionally removes all string properties from VCALENDAR component. See here: https://github.com/jens-maus/node-ical/blob/4195ab2fd36cda2329769c40e638d9cd84edbd93/ical.js#L354

What's the point of that removal?

jens-maus commented 3 years ago

This seems to be a quite old change before I took over the project, see here:

https://github.com/jens-maus/node-ical/commit/4b4a4bbdb27386eccf1c2dabe7884a868e3f886f

However, i dunno what the purpose of this change was. Perhaps its time to ditch it if there is a good reason and example ics where we can test it on.

Aggtaa commented 3 years ago

Sure. Here you go:

BEGIN:VCALENDAR
PRODID:-//Google Inc//Google Calendar 70.9054//EN
VERSION:2.0
CALSCALE:GREGORIAN
METHOD:PUBLISH
X-WR-CALNAME:node-ical test
X-WR-TIMEZONE:Europe/Moscow
X-WR-CALDESC:A simple calendar to test node-ical parser
END:VCALENDAR

The simplest calendar that can be. No events at all. Created with Google Calendar web interface.

Available from this url also: https://calendar.google.com/calendar/ical/3bafqi1jabdlhhmbvsr7gf39dg%40group.calendar.google.com/public/basic.ics

Can be tweaked for tests if needed.

Aggtaa commented 3 years ago

What's interesting is that radicale generates iCal with these properties instead:

X-WR-CALDESC;VALUE=TEXT:test description
X-WR-CALNAME;VALUE=TEXT:test 2

that result in object properties that are not being deleted.

        "WR-CALDESC": {
                "params": {
                        "VALUE": "TEXT"
                },
                "val": "test description"
        },
        "WR-CALNAME": {
                "params": {
                        "VALUE": "TEXT"
                },
                "val": "test 2"
        },

They are pain to access as export type CalendarResponse = Record<string, CalendarComponent>; and those two are obviously of not CalendarComponent type, but thats another topic, I guess.

twalling commented 2 years ago

We've commented out that delete step in a fork because we needed the string properties. In our case we simply wanted the title of the calendar which is defined as:

X-WR-CALNAME:Holidays in United States

see: https://en.wikipedia.org/wiki/ICalendar

I can confirm that google's ICS output outputs these extensions like so:

X-WR-CALNAME:Holidays in United States
X-WR-TIMEZONE:UTC
X-WR-CALDESC:Holidays and Observances in United States
twalling commented 2 years ago

@jens-maus looks like that change was made to keep pace with the original project (non-node) version of this library.

https://github.com/peterbraden/ical.js/pull/38

I wish there was a discussion of it in either project. It's hard to see what the reasoning was and there are useful properties one misses out on because of it.

Maybe I could submit a PR to make it optional behavior?