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

Daylight savings time not handled properly in America/Santigo. #1121

Open
1 task done
andrewlmurray opened this issue Sep 12, 2023 · 5 comments
Open
1 task done

Daylight savings time not handled properly in America/Santigo. #1121

andrewlmurray opened this issue Sep 12, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@andrewlmurray
Copy link

Bug Description

Dates set to midnight of the day of a DST transition in Chile get moved to the prior day at 11 vs the intended day at 1am.

This is inconsistent with the behavior of other runtimes - there may be further similar issues with other timezones.

  • I have run gradle clean and confirmed this bug does not occur with JSC

Hermes version: current
React Native version (if any): 0.71.8
OS version (if any): os x, ios, android
Platform (most likely one of arm64-v8a, armeabi-v7a, x86, x86_64): arm64-v8a

Steps To Reproduce

  1. Set your machine timezone to America/Santiago
  2. Go to the hermes playground
  3. Paste in this code
const first = new Date(2023,8,1);
const second = new Date(2023,8,2);
const third = new Date(2023,8,3);
const fourth = new Date(2023,8,4);

print(first);
print(second);
print(third);
print(fourth);

This will output:

Fri Sep 01 2023 00:00:00 GMT-0400
Sat Sep 02 2023 00:00:00 GMT-0400
Sat Sep 02 2023 23:00:00 GMT-0400
Mon Sep 04 2023 00:00:00 GMT-0300

Expected behaviour

On v8 / jsc you see the following:

Fri Sep 01 2023 00:00:00 GMT-0400 (Chile Standard Time)
Sat Sep 02 2023 00:00:00 GMT-0400 (Chile Standard Time)
Sun Sep 03 2023 01:00:00 GMT-0300 (Chile Summer Time)
Mon Sep 04 2023 00:00:00 GMT-0300 (Chile Summer Time)

As is this causes the popular date-fns library to spin into an infinite loop in some cases.
See this function for an example: https://github.com/date-fns/date-fns/blob/main/src/eachDayOfInterval/index.ts

If you're in America/Santiago the following will loop forever:

const days = eachDayOfInterval({
  start: parseISO("2023-09-01"),
  end: parseISO("2023-09-04")
});
@tmikov
Copy link
Contributor

tmikov commented Sep 12, 2023

Thank you for reporting this! We are looking into it.

@dannysu
Copy link
Contributor

dannysu commented Sep 20, 2023

I investigated the issue a bit. Here are my findings.

The bug with time correctness stems from Hermes' implementation of LocalTZA and DaylightSavingTA in lib/VM/JSLib/DateUtil.cpp. These functions are implemented to match ES5.1 spec, and it's the spec itself that was written poorly with regards to dates. They don't correctly take into account of the time to be converted. For example, LocalTZA does not take in any parameters.

The newer ECMAScript standard such as ES13 correctly updated the procedures dealing with timezone conversion. The new spec combines LocalTZA and DaylightSavingTA into a single function still named LocalTZA. The function now accepts two arguments. One for the timestamp to be converted, and one for whether that timestamp is UTC.

The question becomes how to update the implementation in Hermes to the newer spec. There are two conversions to deal with: UTC to localtime, and localtime to UTC.

UTC to localtime is unambiguous. We can use the standard library std::localtime function to do that conversion. Currently the code in DateUtil.cpp always uses the current timestamp when calling std::localtime, so it's just a matter of passing the timestamp from the argument instead.

However, the tricky one is the localtime to UTC conversion. The standard library std::mktime function can be used to do that conversion for years after 1900. But there are some issues with using std::mktime. ES13 specifies the following:

When tlocal represents local time repeating multiple times at a negative time zone transition (e.g. when the daylight saving time ends or the time zone offset is decreased due to a time zone rule change) or skipped local time at a positive time zone transitions (e.g. when the daylight saving time starts or the time zone offset is increased due to a time zone rule change), tlocal must be interpreted using the time zone offset before the transition.

For Santiago in 2023, there is a rollback of 1 hour on April 2 midnight, and there is a move forward of 1 hour on September 3 midnight. The example given in this issue is for Sep 3, but there is the same issue to deal with for Apr 2. For example, 11:30 PM on Apr 1 repeats multiple times. First time in UTC-03, then after Apr 2 midnight hits, 11:30 PM on Apr 1 will happen again but this time in UTC-04. The spec says for the conversion, the offset should be treated as if UTC-03 is the offset.

std::mktime gives no control of what the behavior should be. I found that it does not give the result we want in the rollback case some of the time. I tested Santiago and Berlin. Neither give the offset prior to the DST transition. Though weirdly, it seems to give the desired answer for New York.

Based on this finding, it seems to me that in order to implement the timezone conversion semantic according to ECMAScript spec, it might be necessary to use the IANA timezone data. Standard library functions don't seem to be sufficient.

@andrewlmurray
Copy link
Author

Hello all. Thanks for all the hard work you're putting into Hermes. I understand this issue has spidering consequences / is complex to fix - just curious if there are any updates on it?

@antoniopro7
Copy link

antoniopro7 commented Aug 8, 2024

Hello everyone, anyone has found any possible solution or workaround? I'm facing the same issue

@JosManMx
Copy link

JosManMx commented Aug 28, 2024

Is there a solution for this issue? as a temporary measure, we are considering disabling Hermes until the problem is resolved. We are unsure if versions of Hermes later than RN 0.71.x fix this issue.

another related issues:
wix/react-native-calendars#2321
wix/react-native-calendars#2505

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants