Emit VTIMEZONE components for referenced TZIDs - #249
Merged
Conversation
alies-dev
force-pushed
the
feat/ics-vtimezone
branch
3 times, most recently
from
August 20, 2026 16:11
dfd2126 to
550ef58
Compare
An event with distinct start and end timezones is written with a TZID parameter on each endpoint, but the file carried no definition of those zones. RFC 5545 section 3.6.5 requires one VTIMEZONE per unique TZID. Clients that resolve bare IANA identifiers imported the event anyway, while older Outlook desktop versions read the endpoints as floating local times and shifted the event. The observances are derived from DateTimeZone::getTransitions() over a window reaching a year either side of the event, so a zone that changes its clocks contributes both STANDARD and DAYLIGHT while a fixed offset zone contributes STANDARD alone. The window comes from the event's own dates rather than the current date, keeping the output deterministic.
alies-dev
force-pushed
the
feat/ics-vtimezone
branch
from
August 20, 2026 16:20
550ef58 to
c277518
Compare
This was referenced Aug 20, 2026
pull Bot
pushed a commit
to KornaPhp/php-calendar-links
that referenced
this pull request
Aug 20, 2026
Every timezone defect closed by spatie#234, spatie#235, spatie#238 and spatie#239 had the same shape: a category of DateTimeZone name nobody had thought of. Each was found by hand, one at a time, and each needed its own targeted fix and its own targeted test. Sweep every name DateTimeZone::listIdentifiers(ALL_WITH_BC) returns, plus the offset and abbreviation spellings only DateTimeZone accepts, through three event shapes and all five generators, asserting the five invariants from spatie#252: 1. A TZID parameter carries the whole zone name, so a name holding a colon shows up as the truncation a client would read (RFC 5545 3.1). 2. Every VTIMEZONE states at least one observance (RFC 5545 3.6.5). 3. Every endpoint written with a TZID resolves, against the observances in the file, to the offset PHP reports for that zone at that instant. 4. A ctz, stz or etz is one of the link's own zones, the link reports resolvable timezones, and the name needs no URL encoding (RFC 3986). 5. No generator raises a PHP warning, notice or deprecation. Nothing is measured against a hardcoded transition date. The tzdb ships with PHP and moves, and the CI matrix spans three PHP versions carrying different releases of it, so every expectation is asked of DateTimeZone at runtime. A wider spread of tzdata widens the search. Invariants 2 and 5 also sweep the referencedTimezones() extension point. A zone with no transition table is never resolvable, so it reaches no TZID through an endpoint, and a subclass putting one in a component is the only path that gets there at all. That is the path the empty VTIMEZONE and its warning came down. Verified by mutation. Restoring the pre-spatie#249 observance handling trips invariant 3 for 194 zones, Casablanca included; dropping the getTransitions() === false guard trips 2 and 5; widening the resolvable set trips 1 and 4. The full sweep adds ~120ms. The targeted regression tests stay as they are, since they document why each individual rule exists.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context and Purposes
Since #233 an event whose endpoints sit in two different timezones is written with a
TZIDparameter on each endpoint:The file carried no definition of those zones. RFC 5545 §3.6.5 makes that a MUST: "An individual VTIMEZONE calendar component MUST be specified for each unique TZID parameter value specified in the iCalendar object."
Google Calendar and Apple Calendar happen to resolve bare IANA identifiers and import the event correctly, so the omission is invisible there. Older Outlook desktop versions (2007 through 2013) do not: they either refuse the import or read the values as floating local times, which shifts the event by the whole offset. Strict validators reject the file outright.
This emits the missing components. Every zone named by a
TZIDnow gets aVTIMEZONEat the VCALENDAR level, after the calendar properties and beforeBEGIN:VEVENT:How the observances are derived
The offsets come from PHP's own zone database, through
DateTimeZone::getTransitions(), rather than a rule table that would need maintaining every time a country changes its mind about summer time.STANDARDand aDAYLIGHTobservance while staying small. It is derived from the event's own dates and never from the current date, so the output is stable and snapshot tests do not drift.RRULEapplies from itsDTSTARTonwards until the next one begins (§3.6.5), so that pair describes exactly the period the event falls in.DTSTARTinside an observance is a local time read against the offset being left behind, per §3.8.2.4, so it is computed as the transition instant plusTZOFFSETFROM.Asia/Kolkata,UTC) has no daylight saving to describe and gets aSTANDARDobservance alone, withTZOFFSETFROMequal toTZOFFSETTO.TZIDvalue, not on the properties that reference it, so a zone named twice is defined once.Scope
Only the path that actually writes
TZIDparameters is affected, which isLink::hasDistinctTimezones(). An event whose ends share a zone, and an all-day event, are still written as before with plain UTC instants and noVTIMEZONE. The other generators are untouched.Note for whoever merges this alongside #234
A
VTIMEZONEis only meaningful when a property in the file references it, so the components and theTZIDparameters have to be gated on the same condition. #234 narrows the condition for writingTZID, because an offset style zone name such as+02:00cannot appear in a param-value (no unquoted colon) and no service resolves it, so those events fall back to plain UTC endpoints. If the two gates drift apart, such an event gets components defining zones the file never names.To keep that merge to a single edit, the condition here is hoisted into
Ics::shouldDefineTimezones()rather than left inline. Once both land, that one method needs the same narrowing #234 applies to theTZIDcondition, and nothing else in this change moves. The docblock on the method says so, and a test pins the behaviour by closing the gate from a subclass and asserting no component survives.referencedTimezones(),generateTimezoneComponents()andgenerateTimezoneComponent()are protected, matching theadditionalCalendarProperties()andadditionalEventProperties()seams already inIcs, so a subclass can add a zone or replace the whole component.Fixes #235