Skip to content

WIP: Cookbook example showing how to handle icalendar time zones #2894

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

ptomato
Copy link
Collaborator

@ptomato ptomato commented Jun 11, 2024

This is an example of how a ZonedDateTime object with a custom time zone could work, if TimeZone objects are removed. (#2853)

This shows integration with ical.js. It's a ZonedDateTime-wrapper object that internally maintains a real Temporal.ZonedDateTime object, but it takes an ICAL.Timezone instead of a Temporal.TimeZone. If the ICAL.Timezone object has the ID of an IANA time zone, we delegate everything to the real Temporal.ZonedDateTime and use the host's time zone database. If the ID is not an IANA ID, we fall back to the ICAL.Timezone's VTIMEZONE data.

Draft; still needs test data with which to make sure everything works, and a cookbook page.

Copy link
Collaborator

@justingrant justingrant left a comment

Choose a reason for hiding this comment

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

Thanks for doing this, seems like a reasonable starting point.

}
// this.#impl with a non-IANA time zone uses UTC internally, so we can just
// calculate the plain date-time in UTC and add the UTC offset.
return this.#impl.toPlainDateTime().add({ nanoseconds: this.offsetNanoseconds });
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this break around DST transitions that PDT is unaware of? That may be OK for a cookbook sample, but it may deserve a comment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't think it'll break. this.#impl.toPlainDateTime() will be unaware of any DST transitions at all, but adding this.offsetNanoseconds will correctly account for DST transitions in the VTIMEZONE.

@justingrant justingrant added documentation Additions to documentation no-spec-text PR can be ignored by implementors labels Oct 9, 2024
@ptomato
Copy link
Collaborator Author

ptomato commented Jul 10, 2025

@justingrant If iCalendar is removing the custom time zone stuff from their specification after all, should we delete this example? Replace it with something else? Combining it with #1370 might be the best option, in that case.

@justingrant
Copy link
Collaborator

@justingrant If iCalendar is removing the custom time zone stuff from their specification after all, should we delete this example?

It's JSCalendar not iCalendar that's removing support for custom time zones, so that you can provide a time zone name only. In iCalendar AFAIK all time zones are custom-ish; you must supply time zone rules, not just a name.

Replace it with something else? Combining it with #1370 might be the best option, in that case.

Maybe have one sample that does both? AFAIK the rule semantics are similar even if the syntax is different.

@ptomato ptomato force-pushed the cookbook-custom-timezone branch from f47e152 to 99ced20 Compare July 19, 2025 00:55
Copy link

codecov bot commented Jul 19, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.85%. Comparing base (2a9fec8) to head (99ced20).
Report is 11 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2894   +/-   ##
=======================================
  Coverage   96.85%   96.85%           
=======================================
  Files          21       21           
  Lines        9983     9983           
  Branches     1829     1829           
=======================================
  Hits         9669     9669           
  Misses        268      268           
  Partials       46       46           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Additions to documentation no-spec-text PR can be ignored by implementors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants