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

Cannot create alarm with 0 trigger #519

Closed
12joan opened this issue Sep 15, 2023 · 2 comments
Closed

Cannot create alarm with 0 trigger #519

12joan opened this issue Sep 15, 2023 · 2 comments

Comments

@12joan
Copy link

12joan commented Sep 15, 2023

This line in alarm.ts makes it impossible to add an alarm with a trigger of 0 (the time of the event).

        if (!trigger) {
            this.data.trigger = null;
        }

Attempting to do so results in a runtime error: Error: No value for 'trigger' in ICalAlarm given!.

Also, I understand that this may be a large undertaking at this point, but this library would be a lot safer to use in production if runtime errors were kept to an absolute minimum. Ideally, if all arguments are typed correctly, I would expect a library like this to never produce a runtime error except in exceptional circumstances.

I noticed a few places where the types are contradictory to what ical-generator expects. For instance, type is optional on ICalAlarmData but produces a runtime error if omitted. Additionally, setting the type to 'display' is rejected by TypeScript.

const alarm: ICalAlarmData = {
  type: 'display', // Type '"display"' is not assignable to type 'ICalAlarmType | null | undefined'
  trigger: 1,
};
@sebbo2002
Copy link
Owner

Duplicate of #344 (which was closed because I have it on my list, but I reopened it just now for better visibility). I collected a ton of breaking changes which I'm working on, but due to limited time and other project what's not finished yet. I try to publish it within this year, sorry about that.

@sebbo2002
Copy link
Owner

I just pushed a preview of ical-generator v6 to develop, which should fix this issue. I would be happy if you could test the new version and give me some feedback. You can install the version with npm i ical-generator@next. All changes in this release can be found here.

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

No branches or pull requests

2 participants