Skip to content

Conversation

@KristjanESPERANTO
Copy link
Collaborator

@KristjanESPERANTO KristjanESPERANTO commented Sep 26, 2025

  • enhance comments for better understanding of timezone conversions and recurrence logic
  • replace arrow functions with named functions for improved readability
  • compact dtstart normalization code to reduce duplication
  • add conditional tzid reset to prevent errors on missing options
  • snap all-day recurrences to event timezone midnight using startOf("day") to fix date drift

The trigger for all this is that I am currently trying node-ical to get rid of the dependency on moment-timezone, and I wanted to better understand how we use it here.

@KristjanESPERANTO KristjanESPERANTO marked this pull request as draft September 26, 2025 19:26
…stness

- enhance comments for better understanding of timezone conversions and recurrence logic
- replace `arrow functions` with `named functions` for improved readability
- compact `dtstart` normalization code to reduce duplication
- add conditional `tzid` reset to prevent errors on missing options
- snap all-day recurrences to event timezone midnight using `startOf("day")` to fix date drift
All-day recurring events were serialized as UTC timestamps sourced from the
rrule result. Viewers west of UTC therefore saw the start date drift to the
preceding day. Parse the rrule output as a floating YYYY-MM-DD first and only
then apply viewer offsets so every client sees the same calendar date.
Clean up temporary debugging scaffolding in the process.
- map recurring event instants with moment-timezone so viewer DST offsets stay correct
- leave floating metadata available for full-day cases while returning structured payloads
- update calendar fetcher unit spec to assert against the new occurrence wrapper
Copy link
Collaborator

@sdetweil sdetweil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remember to test with modules that receive our notifications

MMM-CalendarExt3xxxx
For sure

@KristjanESPERANTO
Copy link
Collaborator Author

@sdetweil Thanks 🙂 I'll keep it in mind and test whether this would break MMM-CalendarExt3xxxx.

But I think I messed up a bit here - seems I wanted to fix things that doesn't need to be fixed 😅 I need to take deeper look at it.

@sdetweil
Copy link
Collaborator

Yeh, it’s easy to say, I wouldn’t do it like this
And then end up busting everything

@KristjanESPERANTO
Copy link
Collaborator Author

Okay. I'll drop it for now.

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.

2 participants