feat: implement DST-aware timezone conversion and handling#24436
feat: implement DST-aware timezone conversion and handling#24436tusharchawre wants to merge 3 commits intocalcom:mainfrom
Conversation
|
@tusharchawre is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
|
github-actions
bot
added
booking-page
WalkthroughThis PR centralizes DST-aware timezone handling: it adds a new module packages/lib/timezone-conversion.ts with functions to convert UTC times to timezones, format times with DST awareness, detect DST transition days, and get timezone offsets. It updates packages/lib/dayjs/index.ts to use formatTimeWithDST, revises local-midnight alignment in packages/lib/date-ranges.ts to compute and apply offset differences robustly across DST, and adjusts slot time conversion and imports in packages/features/bookings/components/AvailableTimes.tsx. No existing exported signatures were removed; the new module exposes several utility functions. Possibly related PRs
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dosubot
bot
added
bookings
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
packages/lib/slots.ts (1)
134-139: Consider using the new DST-aware utility function.The comment states "Use DST-aware conversion for accurate timezone handling," but the code continues to use the standard
slotStartTime.tz(timeZone)approach. For consistency with the DST-aware improvements introduced in this PR, consider usingconvertUTCToTimezonefrom the newpackages/lib/timezone-conversion.tsmodule.Apply this diff to use the new DST-aware utility:
+import { convertUTCToTimezone } from "@calcom/lib/timezone-conversion"; + // Convert to target timezone BEFORE checking if rounding is needed // This ensures we check minute alignment in the local timezone, not UTC // This prevents issues with half-hour offset timezones like Asia/Kolkata (GMT+5:30) // Use DST-aware conversion for accurate timezone handling -slotStartTime = slotStartTime.tz(timeZone); +slotStartTime = convertUTCToTimezone(slotStartTime, timeZone);packages/features/bookings/components/AvailableTimes.tsx (1)
118-120: Consider using the new DST-aware utility function.The DST-aware conversion approach using
dayjs.utc(slot.time).tz(timezone)is correct. However, for consistency with the new DST-aware utilities introduced in this PR, consider usingconvertUTCToTimezonefrompackages/lib/timezone-conversion.ts, which encapsulates this logic with additional validation and fallback handling.Apply this diff:
+import { convertUTCToTimezone } from "@calcom/lib/timezone-conversion"; + - // Use DST-aware timezone conversion for accurate display during DST transitions - const slotTimeUTC = dayjs.utc(slot.time); - const computedDateWithUsersTimezone = slotTimeUTC.tz(timezone); + // Use DST-aware timezone conversion for accurate display during DST transitions + const computedDateWithUsersTimezone = convertUTCToTimezone(slot.time, timezone);packages/lib/timezone-conversion.ts (2)
34-59: Reconsider the validation logic necessity.The validation logic at lines 48-56 checks if the converted offset matches either standard or DST offset, but:
- Day.js's
.tz()method already handles DST transitions correctly when provided with IANA timezone identifiers- The validation only logs a warning without correcting the issue
- This may produce false positives in edge cases (e.g., during the actual transition hour)
- It adds computational overhead by creating additional dayjs instances for January and July
Consider simplifying to trust Day.js's DST handling:
- // For DST-observing timezones, we need to be more careful const convertedTime = utcMoment.tz(targetTimezone); - - // Check if this date falls on a DST transition day - const year = convertedTime.year(); - - // Get the DST difference for this timezone - const dstDifference = getDSTDifference(targetTimezone); - - if (dstDifference !== 0) { - // This timezone observes DST, so we need to ensure proper handling - // The dayjs library should handle most cases correctly, but we add validation - - // Verify that the conversion is reasonable - const expectedOffset = convertedTime.utcOffset(); - const standardOffset = dayjs.tz(`${year}-01-01T00:00:00`, targetTimezone).utcOffset(); - const dstOffset = dayjs.tz(`${year}-07-01T00:00:00`, targetTimezone).utcOffset(); - - // If the current offset doesn't match either standard or DST offset, - // there might be an issue with the conversion - if (expectedOffset !== standardOffset && expectedOffset !== dstOffset) { - console.warn(`Unexpected timezone offset for ${targetTimezone} on ${convertedTime.format()}`); - } - } - return convertedTime;If you want to keep validation for debugging purposes, consider moving it behind a feature flag or environment variable.
115-118: Consider if this utility function is necessary.The
getTimezoneOffsetfunction is a thin wrapper arounddate.tz(timezone).utcOffset(). While centralizing this logic allows for future enhancements, consider if the added indirection is worth it for such a simple operation.If keeping this function, consider adding error handling for invalid timezones:
export function getTimezoneOffset(date: dayjs.Dayjs, timezone: string): number { + if (!timezone) { + return date.utcOffset(); + } + try { const dateInTimezone = date.tz(timezone); return dateInTimezone.utcOffset(); + } catch (error) { + console.warn(`Failed to get timezone offset for ${timezone}:`, error); + return date.utcOffset(); + } }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
packages/features/bookings/components/AvailableTimes.tsx(2 hunks)packages/lib/date-ranges.ts(1 hunks)packages/lib/dayjs/index.ts(2 hunks)packages/lib/slots.ts(1 hunks)packages/lib/timezone-conversion.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
**/*.ts: For Prisma queries, only select data you need; never useinclude, always useselect
Ensure thecredential.keyfield is never returned from tRPC endpoints or APIs
Files:
packages/lib/dayjs/index.tspackages/lib/date-ranges.tspackages/lib/timezone-conversion.tspackages/lib/slots.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Flag excessive Day.js use in performance-critical code; prefer native Date or Day.js
.utc()in hot paths like loops
Files:
packages/lib/dayjs/index.tspackages/lib/date-ranges.tspackages/lib/timezone-conversion.tspackages/features/bookings/components/AvailableTimes.tsxpackages/lib/slots.ts
**/*.{ts,tsx,js,jsx}
⚙️ CodeRabbit configuration file
Flag default exports and encourage named exports. Named exports provide better tree-shaking, easier refactoring, and clearer imports. Exempt main components like pages, layouts, and components that serve as the primary export of a module.
Files:
packages/lib/dayjs/index.tspackages/lib/date-ranges.tspackages/lib/timezone-conversion.tspackages/features/bookings/components/AvailableTimes.tsxpackages/lib/slots.ts
**/*.tsx
📄 CodeRabbit inference engine (.cursor/rules/review.mdc)
Always use
t()for text localization in frontend code; direct text embedding should trigger a warning
Files:
packages/features/bookings/components/AvailableTimes.tsx
🧠 Learnings (2)
📓 Common learnings
Learnt from: hariombalhara
PR: calcom/cal.com#23918
File: packages/features/schedules/lib/use-schedule/useTimesForSchedule.test.ts:16-23
Timestamp: 2025-09-23T08:00:07.619Z
Learning: In calcom/cal.com test files, particularly packages/features/schedules/lib/use-schedule/useTimesForSchedule.test.ts, the TIMEZONE_OFFSETS mapping is intentionally limited to only UTC variants and Asia/Kolkata. This is by design and should not be flagged as incomplete in future reviews (confirmed by maintainer hariombalhara).
📚 Learning: 2025-09-23T08:00:07.619Z
Learnt from: hariombalhara
PR: calcom/cal.com#23918
File: packages/features/schedules/lib/use-schedule/useTimesForSchedule.test.ts:16-23
Timestamp: 2025-09-23T08:00:07.619Z
Learning: In calcom/cal.com test files, particularly packages/features/schedules/lib/use-schedule/useTimesForSchedule.test.ts, the TIMEZONE_OFFSETS mapping is intentionally limited to only UTC variants and Asia/Kolkata. This is by design and should not be flagged as incomplete in future reviews (confirmed by maintainer hariombalhara).
Applied to files:
packages/features/bookings/components/AvailableTimes.tsx
🧬 Code graph analysis (1)
packages/lib/timezone-conversion.ts (1)
packages/lib/dayjs/index.ts (2)
timeZoneWithDST(207-211)getDSTDifference(221-225)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Install dependencies / Yarn install & cache
🔇 Additional comments (9)
packages/features/bookings/components/AvailableTimes.tsx (1)
9-9: LGTM: Import consolidation.The type import consolidation improves code organization.
packages/lib/dayjs/index.ts (2)
21-23: LGTM: Documentation improvement.The JSDoc comment clearly indicates DST-awareness, improving code documentation.
59-59: LGTM: Simplified error handling.The bare
catchclause is appropriate when the error value is not used.packages/lib/timezone-conversion.ts (5)
1-2: LGTM: Import statements.The imports are correctly structured and necessary for the DST-aware functionality.
12-27: Good error handling for invalid timezones.The function correctly handles missing or invalid timezones by falling back to UTC with appropriate warnings. The use of
timeZoneWithDSTto validate the timezone before conversion is a good defensive programming practice.
29-32: LGTM: Optimization for non-DST timezones.The early return for non-DST-observing timezones is a good performance optimization.
100-105: LGTM: Clean DST transition detection.The function correctly identifies DST transition days by comparing UTC offsets at the start and end of the day. The early return for non-DST timezones is a good optimization.
71-91: No circular dependencies detected
Confirmedtimezone-conversion.tsimports from dayjs modules anddayjs/index.tsdoes not importtimezone-conversion.packages/lib/date-ranges.ts (1)
70-76: Add unit tests for DST transition days: Cover spring-forward, fall-back, and non-standard DST offsets (30- and 45-minute shifts) to validate this alignment logic in packages/lib/date-ranges.ts (lines 70–76).
dhairyashiil
left a comment
dhairyashiil
left a comment
There was a problem hiding this comment.
Please attach before and after loom video, making it draft until then.
|
This PR has been marked as stale due to inactivity. If you're still working on it or need any help, please let us know or update the PR to keep it active. |
|
This PR has been closed due to inactivity. Please feel free to reopen it if you'd like to continue the work. |
3 participants
What does this PR do?
Fixes calcom#24350
This PR resolves a critical daylight saving time (DST) issue where booking times displayed incorrectly during DST transitions, causing ±1 hour discrepancies between the intended booking time and the displayed time.
Problem
When users book meetings around DST transition periods (e.g., Europe/Berlin end of March/October), the frontend displayed incorrect times while the backend correctly stored UTC timestamps. This led to:
Solution
Implemented comprehensive DST-aware timezone handling:
packages/lib/date-ranges.tspackages/lib/timezone-conversion.tspackages/features/bookings/components/AvailableTimes.tsxpackages/lib/dayjs/index.ts