Fix invalid timezone errors for valid timezones on Apple platforms.#1611
Fix invalid timezone errors for valid timezones on Apple platforms.#1611chrsmys wants to merge 1 commit intofacebook:mainfrom
Conversation
facebook-github-bot
added
the
CLA Signed
|
@robhogan Do you know who might be best to take a look at this? Thanks! |
|
This PR should fix issue in our application. Please take a look soon. |
|
@tmikov sorry for the direct tag, but do you know anyone who can take a look? |
|
Will do |
chrsmys
force-pushed
the
nschris/incorrectTimezoneErrorFix
branch
from
222faef to
3ebd46d
Compare
@lavenzg Looks like it's CI is failing to find a simulator to run on. |
|
Looks like it might get fixed by this #1626 |
Yeah we are aware of that. Just ignore that for now. |
lavenzg
force-pushed
the
nschris/incorrectTimezoneErrorFix
branch
from
3ebd46d to
05e7e4a
Compare
|
@lavenzg has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
|
Would love to see this fix backported into previous react-native versions of hermes - I had great success applying this patch to the v0.76-stable branch when running on iOS 17/18. Unfortunately, when running on iOS 15/16 the app immediately crashes with a |
|
@lavenzg just wanted to bump this in case it got lost |
It has been merged. Thanks! |
…1611) (#1611) Summary: Original Author: chris@nschris.com Original Git: b8ad3c1 Original Reviewed By: avp Original Revision: D70109465 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. Pull Request resolved: #1611 Pulled By: lavenzg Reviewed By: lavenzg Differential Revision: D71332435 fbshipit-source-id: 3f8c11003d31736c28bab64a3362fc3cf5946379
|
@lavenzg could we please backport this to |
Thanks for the request. We don't backport changes to older Hermes releases as a matter of policy. Our team focuses resources on forward development rather than maintaining multiple release branches. Since Hermes is open source, you're welcome to create the backport yourself and submit a PR, if needed for your use case. Keep in mind that any backported changes would also need to be picked up by the corresponding React Native release branch to be useful. |
…acebook#1611) (facebook#1611) Summary: Original Author: chris@nschris.com Original Git: b8ad3c1 Original Reviewed By: avp Original Revision: D70109465 This addresses issue facebook#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. Pull Request resolved: facebook#1611 Pulled By: lavenzg Reviewed By: lavenzg Differential Revision: D71332435 fbshipit-source-id: 3f8c11003d31736c28bab64a3362fc3cf5946379
…acebook#1611) (facebook#1611) Summary: Original Author: chris@nschris.com Original Git: b8ad3c1 Original Reviewed By: avp Original Revision: D70109465 This addresses issue facebook#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. Pull Request resolved: facebook#1611 Pulled By: lavenzg Reviewed By: lavenzg Differential Revision: D71332435 fbshipit-source-id: 3f8c11003d31736c28bab64a3362fc3cf5946379
7 participants
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.knownTimeZoneNamesto determine valid time zones, but this does not provide a complete list of all time zones NSTimeZone supports.US/Easternis 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: