feat: add IANA timezone support to trialPeriod config#1201
feat: add IANA timezone support to trialPeriod config#1201lenisko wants to merge 3 commits intoWatWowMap:developfrom
Conversation
When a timezone (e.g. "Europe/Warsaw") is set in trialPeriod, start/end times are interpreted as local time in that timezone. Interval advancement preserves local time across DST transitions by decomposing UTC dates into local components via Intl.DateTimeFormat and re-resolving the offset each cycle, so e.g. 17:00 Warsaw stays 17:00 Warsaw regardless of CET/CEST shifts. Timer uses chained setTimeout instead of fixed setInterval when timezone is set, so each delay adapts to the actual wall-clock gap (167h or 169h around DST boundaries for a 168h interval). Without timezone set, behavior is unchanged (server-local time, fixed ms intervals).
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: eded2386cd
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
…endency P1: Add _stopped flag to Timer so the recursive setTimeout chain does not re-arm after clear() is called mid-callback. Reset on activate() to allow re-use. P2: Replace new Date(y,m,d,h) + zonedTimeToUtc with Date.UTC + Intl.DateTimeFormat offset resolution (localToUtc). This avoids the host timezone normalising DST-gap hours before zonedTimeToUtc can read them. A second-pass offset check handles cases where the initial guess straddles a DST boundary. Removes date-fns-tz dependency from both Timer and Trial.
|
This PR cannot be merged in its current form. The main issue is cost versus value. It adds a substantial amount of custom timezone/DST scheduling logic across Timer and Trial just to support an optional trialPeriod.timezone convenience field. That is long-term maintenance burden in one of the easiest areas to get subtly wrong. More importantly, the implementation is still not robust. The recurrence logic used when fast-forwarding old trial windows is not the same as the logic used by the runtime timers, which means DST-crossing windows can drift by an hour. If we are going to take on timezone-aware scheduling, it needs to be correct and much more contained than this. At this point, I do not think the feature justifies the additional complexity. It is better to keep trial periods in server time / UTC than to merge a bespoke calendar system that we will have to maintain indefinitely. If IANA timezone support is ever required, it should come back as a smaller design with one dedicated scheduling helper and explicit DST-focused tests. |
Check again. Current version is flawed as hell due to time shifts and requires changing config twice time per year. Your (AI, not yours) costs are worth getting it working like it should. |
When a timezone (e.g. "Europe/Warsaw") is set in trialPeriod, start/end times are interpreted as local time in that timezone. Interval advancement preserves local time across DST transitions by decomposing UTC dates into local components via Intl.DateTimeFormat and re-resolving the offset each cycle, so e.g. 17:00 Warsaw stays 17:00 Warsaw regardless of CET/CEST shifts.
Timer uses chained setTimeout instead of fixed setInterval when timezone is set, so each delay adapts to the actual wall-clock gap (167h or 169h around DST boundaries for a 168h interval).
Without timezone set, behavior is unchanged (server-local time, fixed ms intervals).