Fix timezone issues when using rrule

[50an6xy06r6n]

An alternative implementation of #231. All tests seem to pass, including the ones added in #231.

Implementation is simpler, so hopefully this should be easier to review.

[Apollon77]

should such an exception not better crash the tests?

[50an6xy06r6n]

Uh this is some weird issue with vows.js where it doesn't properly catch thrown exceptions and won't tell you what the issue actually is (you get a bunch of "callback not fired" messages instead). I found this snippet online to try to fix it but it doesn't work, and I apparently forgot to take it back out. Here's what it outputs when there's a parsing error in the test:

$ npm run test

> [email protected] test
> xo && vows test/test.js && vows test/test-async.js && printf "\n"

···········································································································✗✗·

✗ Errored » callback not fired 
      in with test22.ics (testing dtstart of rrule with timezones) first event 
      in node-ical 
      in test/test.js✗ Errored » callback not fired 
      in with test22.ics (testing dtstart of rrule with timezones) second event 
      in node-ical 
      in test/test.js[
  '✗ \x1B[31m\x1B[1mErrored\x1B[22m\x1B[39m » \x1B[1m108\x1B[22m honored ∙ \x1B[1m3\x1B[22m errored ∙ \x1B[1m4\x1B[22m dropped\n'
]

On that note, vows seems hopelessly outdated. It still uses util.print, which was EOLed in Node 12.0.0. I had to make some changes in vow itself to get any useful output on errors.

[50an6xy06r6n]

@Apollon77 any chance you could take another look?

[Apollon77]

I honestly did not really looked into the change itself, but a test it´s there which is great ... And I mainly stumbled over the process.on(...) ... sooo LGTM from my side - but as said without looking too deep into the change itself and potential implications

[jens-maus]

@50an6xy06r6n Thanks for this PR. Please note, however, the failing CI checks for the windows builds. It seems some tests actually fail on the windows build of node-ical. So please correct.

[jens-maus]

@50an6xy06r6n In addition, please comment how much different this PR is compared to #231

[50an6xy06r6n]

@jens-maus Ah I think the Windows CI failures are an issue with the new tests, and have to do with this:

Moment Timezone found Etc/Unknown from the Intl api, but did not have that data loaded.

Pretty sure updating the moment-timezone import to moment-timezone-with-data will fix this, but I'll try it out. Hopefully the CI will re-run, since last time it was blocked on approval.

I think the biggest difference between this and #231 is that this is a more targeted change for the RRULE issue. The other PR I think also changes some other, unrelated behavior. I think our solutions to the RRULE issue are fundamentally the same, though we use different mechanisms for getting the local time. I copied the tests from that PR, so the behavior seems largely the same (minus the additional parsing behavior for "(GMT +XX:X0)" time zones)

The other difference is that I'm updating this PR, and that one hasn't been touched in almost 2 years 😛

[jens-maus]

@jens-maus Ah I think the Windows CI failures are an issue with the new tests, and have to do with this:

Moment Timezone found Etc/Unknown from the Intl api, but did not have that data loaded.

Pretty sure updating the moment-timezone import to moment-timezone-with-data will fix this, but I'll try it out. Hopefully the CI will re-run, since last time it was blocked on approval.

No problem. I can then start the CI check if it is still rejected for you. So please continue to fix this PR in getting the windows CI build running. However, please make sure that moment-timezone-with-data is really required since it is substantially larger than the normal moment-timezone and would substantially increase the overall size of code required when using node-ical.

[50an6xy06r6n]

@jens-maus ready for the CI checks to be re-run

[jens-maus]

@jens-maus ready for the CI checks to be re-run

Thanks for your info. However, the unit tests still fail on windows. See:

    with test23.ics (testing dtstart of rrule with timezones) recurring yearly first event (14 july) 
      ✗ dt start well set 
        » An unexpected error was caught: RangeError: Invalid time zone specified: Etc/Unknown 

[jens-maus]

@50an6xy06r6n it seems rrule.between is somewhat broken on windows. Similar things are reported for rrule.after, see jkbrzt/rrule#608. Thus changed the test cases to somewhat skip on windows until rrule is fixed.

[50an6xy06r6n]

Oh nice, thanks for investigating and fixing it!