Do not rezone dates to prevent doubly applying offset. Accommodate relevant tests.

[ScarlettZebra]

This bugfix addresses the bugs raised by the following issues, removing the additional offset applied by 'rezoning' that would see non-UTC times have their offset applied twice instead of once:

#355
#356

Since JavaScript Date objects track time via elapsed time since 00:00:00 Coordinated Universal Time (UTC), Thursday, 1 January 1970, applying an additional offset (i.e. 'rezoning') these dates should not be necessary as long as the Date was instantiated correctly.

[codecov-io]

Codecov Report

Merging #357 into master will decrease coverage by 0.15%.
The diff coverage is 100%.

@@            Coverage Diff            @@
##           master    #357      +/-   ##
=========================================
- Coverage   90.06%   89.9%   -0.16%     
=========================================
  Files          28      28              
  Lines        1962    1931      -31     
  Branches      583     581       -2     
=========================================
- Hits         1767    1736      -31     
  Misses        195     195

Impacted Files	Coverage Δ
src/datewithzone.ts	100% <ø> (ø)	⬆️
src/iterset.ts	100% <100%> (ø)	⬆️
src/iter/index.ts	98.01% <100%> (-0.1%)	⬇️
src/iter/poslist.ts	100% <0%> (ø)	⬆️
src/masks.ts	100% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e0d83f3...63b4d6d. Read the comment docs.

[davidgoli]

Hi! Thanks for contributing & investing in this library. However, I don't think this is a good change; it basically breaks timezone support, which is a feature used by some clients. Moreover, the desired behavior is already present if you simply do not provide a tzid param.

There has been some confusion around how & when to use tzid. This is partly because JS Date objects natively have poor, inconsistent, confusing, or nonexistent timezone support.

The vast majority (90%) of use cases for this library should not require tzid. tzid is only useful if you want to represent a date in one non-UTC timezone in a different non-UTC timezone. For example, if you want to represent a repeating event that happens in Tokyo, and get the resulting dates in New York time (eg 10pm in Japan is 9am in New York). If your users only ever want recurrences represented in one timezone, whether UTC or their local timezone, tzid should not be used.

Sooner than later I will have to take the time to write a more definitive article explaining all of this, since the README doesn't seem to get the message across effectively. Until then, however, this change is both unnecessary and harmful. Let me know if you have any further questions!

[ScarlettZebra]

Hi there, and thanks for the very quick and detailed feedback.

I do agree that JavaScript Dates are confusing and ambiguous when it comes to dealing with time zones, especially when the developer doesn't use Date.UTC() as mentioned in this repo's README. However, respectfully, I'm not sure that I can agree that the changes in this PR are unnecessary and harmful and in fact would argue the opposite that these changes are necessary to get time zone support working correctly again.

I have found that when using properly instantiated Dates via UTC time, non-UTC recurrences have had their offsets applied doubly when retrieving them via rrule.all(), resulting in times that are neither the local time within the selected time zone nor the UTC version of that time but instead a third, erroneously fabricated time resulting from having the offset between the local time and UTC applied to the local time once more.

I don't believe that rezoning the Dates by adding an offset is necessary since these Date objects are already set to the correct number of milliseconds past 00:00:00 Coordinated Universal Time (UTC), Thursday, 1 January 1970, and this value does not change no matter what time zone you are in.

I realize that perhaps something may be lost in translation and that we may not agree, but perhaps we can get a third opinion on this? I think we could benefit from seeing the results of others' attempts at reproducing this issue.

@jakubroztocil
@lyschoening
Thoughts?

[ScarlettZebra]

Ahh, I see my mistake. It looks like the DTSTART value should be the local time when a TZID is passed in, and my server code was not handling that. Apologies for the confusion, hope you have a great rest of your day. I will take a look at this further and if something pops up I will be sure to file an issue, but until then I'll see if I can't solve this and leave a helpful comment in the Github issues mentioned above to try to get the original poster on his way.