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

Fix invalid timezone errors for valid timezones on Apple platforms. #1611

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

chrsmys
Copy link

@chrsmys chrsmys commented Feb 8, 2025

Summary

This addresses issue #1607 where valid timezones were raising invalid time zone errors. The root of the issue is that hermes relies on NSTimeZone.knownTimeZoneNames to determine valid time zones, but this does not provide a complete list of all time zones NSTimeZone supports. US/Eastern is an example of a timezone that is not listed in knownTimeZoneNames but will generate a valid NSTimeZone.

To address this problem this will first attempt to create an NSTimeZone before raising the range exception. If it's a valid time zone then it will get added to the validTimeZoneNames list, otherwise an exception will be raised.

Test Plan

To test this I built Hermes locally on a Mac and ran this gist to detect which timezones do & do not work. I also tested negative cases to ensure exceptions were still raised when an invalid time zone is provided. Here is list of timezones that previously did not work, but now do:

Africa/Asmera
Africa/Timbuktu
America/Argentina/ComodRivadavia
America/Atka
America/Buenos_Aires
America/Catamarca
America/Coral_Harbour
America/Cordoba
America/Ensenada
America/Fort_Wayne
America/Indianapolis
America/Jujuy
America/Knox_IN
America/Louisville
America/Mendoza
America/Porto_Acre
America/Rosario
America/Virgin
Asia/Ashkhabad
Asia/Chungking
Asia/Dacca
Asia/Istanbul
Asia/Kolkata
Asia/Macao
Asia/Saigon
Asia/Tel_Aviv
Asia/Thimbu
Asia/Ujung_Pandang
Asia/Ulan_Bator
Atlantic/Faeroe
Atlantic/Jan_Mayen
Australia/ACT
Australia/Canberra
Australia/LHI
Australia/North
Australia/NSW
Australia/Queensland
Australia/South
Australia/Tasmania
Australia/Victoria
Australia/West
Australia/Yancowinna
Brazil/Acre
Brazil/DeNoronha
Brazil/East
Brazil/West
Canada/Atlantic
Canada/Central
Canada/Eastern
Canada/Mountain
Canada/Newfoundland
Canada/Pacific
Canada/Saskatchewan
Canada/Yukon
Chile/Continental
Chile/EasterIsland
CST6CDT
Cuba
Egypt
Eire
EST5EDT
Etc/GMT
Etc/GMT+0
Etc/GMT+1
Etc/GMT+10
Etc/GMT+11
Etc/GMT+12
Etc/GMT+2
Etc/GMT+3
Etc/GMT+4
Etc/GMT+5
Etc/GMT+6
Etc/GMT+7
Etc/GMT+8
Etc/GMT+9
Etc/GMT-0
Etc/GMT-1
Etc/GMT-10
Etc/GMT-11
Etc/GMT-12
Etc/GMT-13
Etc/GMT-14
Etc/GMT-2
Etc/GMT-3
Etc/GMT-4
Etc/GMT-5
Etc/GMT-6
Etc/GMT-7
Etc/GMT-8
Etc/GMT-9
Etc/GMT0
Etc/Greenwich
Etc/UCT
Etc/Universal
Etc/UTC
Etc/Zulu
Europe/Belfast
Europe/Nicosia
Europe/Tiraspol
Factory
GB
GB-Eire
GMT+0
GMT-0
GMT0
Greenwich
Hongkong
Iceland
Iran
Israel
Jamaica
Japan
Kwajalein
Libya
MET
Mexico/BajaNorte
Mexico/BajaSur
Mexico/General
MST7MDT
Navajo
NZ
NZ-CHAT
Pacific/Samoa
Pacific/Yap
Poland
Portugal
PRC
PST8PDT
ROC
ROK
Singapore
Turkey
UCT
Universal
US/Alaska
US/Aleutian
US/Arizona
US/Central
US/East-Indiana
US/Eastern
US/Hawaii
US/Indiana-Starke
US/Michigan
US/Mountain
US/Pacific
US/Samoa
W-SU
Zulu

@facebook-github-bot facebook-github-bot added the CLA Signed Do not delete this pull request or issue due to inactivity. label Feb 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Do not delete this pull request or issue due to inactivity.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants