Skip to content
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

feat: add support for timezone with seconds offset #47

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

Conversation

GianlucaWassermeyer
Copy link

@GianlucaWassermeyer GianlucaWassermeyer commented Mar 11, 2025

Why

Issue 46
I encountered an issue with the TZDate class when handling dates before the implementation of the GMT system in 1883. The TZDate class appears to only consider the hour and minute of the timezone, but not the seconds, leading to inconsistencies.
Example

Additional Information: The issue seems to stem from TZDate only considering the hour and minute of the timezone, but not the seconds. This discrepancy becomes apparent for dates before 1883, when the GMT system was implemented. The offset GMT-07:52:58 (America/Los_Angelos, GMT-08:00) causes the inconsistency.

Proposed Fix

The offsetStr in tzOffset already contains the timezone data for hours, minutes, and seconds. The tzOffset function returns the offset in minutes. To avoid breaking changes, I added the seconds offset as a decimal number.

In mini.js, we need to adjust the date to the system timezone. The native JavaScript Date object's .getTimezoneOffset method returns only the minutes and no seconds. To circumvent this, I create a new date and set the UTCSeconds to 0. With this, the seconds are the timezone offset of the system timezone. We then add this together with our tzOffset seconds to get the correct date.

If you don´t like any part of this PR, please tell me and I can try to find a different solution.

@GianlucaWassermeyer GianlucaWassermeyer marked this pull request as ready for review March 11, 2025 08:56
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.

1 participant