react-native-community / jsc-android-buildscripts

Script for building JavaScriptCore for Android (for React Native but not only)
BSD 2-Clause "Simplified" License
1.06k stars 97 forks source link

Android Intl doesn't work as expected #102

Open deecewan opened 5 years ago

deecewan commented 5 years ago

Issue Description

To start, I've created a small repro here: https://github.com/deecewan/TimeZoneTest

I set up a brand new project (npx react-native-cli init TimeZoneTest), and then followed the steps for React Native 0.59, and added the support for Intl. This works, because the Intl object actually exists now (yay!).

The problem is, the behaviour is not the same as on iOS when trying to format dates.

Here is the output on Android:

image

And the same code on iOS:

image

The same code on Android, with remote debug enabled:

image

(Note: My android emulator is set up to be in Australia/Brisbane. The iOS Simulator is set to my laptop's TZ, in Canada. In remote debug, it uses Chrome in my laptop's TZ).

As you can see, when formatting on the iPhone and in Chrome via Remote Debugging, the output is in the TZ of the 'device'. When rendering on Android, with Intl enabled, it renders in UTC/GMT. This seems to be inconsistent with how it should be rendering.

Version, config, any additional info

See repro reop for full details

React Native 0.59.8 jsc-android: 241213.x.x android/app/build.gradle dep: implementation "org.webkit:android-jsc-intl:r241213"

Kudo commented 5 years ago

Hey @deecewan, your repo at https://github.com/deecewan/TimeZoneTest is empty. There is no way for me to confirm the date methods you used.

However, once enabling Remote Debugging, the actually JS engine is Chrome. The Intl is supported by V8 not JSC and the timezone is your Chrome's timezone. AFAIK, the behavior you described is expected.

deecewan commented 5 years ago

hey @Kudo - sorry, forgot to push 😞

I know that when you enabled remote debugging, the engine is Chrome. The problem is that on iOS, when using JSC, it renders in the correct timezone. And, on Android, when using Chrome, it renders in the correct timezone. On Android, tho, with JSC, it always renders in UTC by default. Note that the first screenshot is rendered in UTC, and the second 2 screenshots are both in Mountain Time.

As stated, I'm using the Intl variant, so I'd expect it to behave the same way as Chrome/iOS JSC.

Kudo commented 5 years ago

Thank you for the briefly explanation. I've confirm it is an issue and root cause is that IntlDateTimeFormat use ICU's ucal_getDefaultTimeZone to query current timezone. And it is not integrate to Android persist.sys.timezone https://github.com/WebKit/webkit/blob/master/Source/JavaScriptCore/runtime/IntlDateTimeFormat.cpp#L126 Need to patch handy.

deecewan commented 5 years ago

Hmm. Interesting.

From here it looks like we could just ucal_setDefaultTimeZone, right?

Or, something like this from FreeRDP might be helpful

Kudo commented 5 years ago

Yes, I am now focusing on JSC crash issue. Will find a time to fix a problem after that.

dlebedynskyi commented 4 years ago

@Kudo any update on this issue or PR?

leethree commented 4 years ago

in my testing. default timezone in Intl is always UTC. this behaviour is different from the default Date timezone.

> time.toString()
< Tue May 12 2020 22:00:00 GMT+0800 (HKT)

> time.toLocaleString('en', { timeZoneName: 'long' })
< 5/12/2020, 2:00:00 PM GMT

In the above example, toString is in GMT+8, while toLocaleString is GMT.

great-timofey commented 4 years ago

I can confirm that this issue still exists (I'm using FormattedTime component from react-intl) and in Android version I have the same problem related with UTC zones, while iOS works correctly.

Spent a couple of hours trying to install different sets of polyfills for react-intl I've found a working solution. For someone looking for fix - instead of default installation please use yarn add jsc-android@npm:@kudo-ci/jsc-android

Many thanks to @Kudo for this. I cannot understand why his MR with fix is still in review (since September of 2019)... Dear maintainers, could you please pay more attention to this?

newyankeecodeshop commented 4 years ago

@great-timofey It seems like there are no active maintainers in this project anymore. @Kudo is there a process where you or someone else can bring someone up to speed as a maintainer for this repo? I would be interested, but I haven't done development in JavaScriptCore so I'd need lots of documentation and ramp-up time.

Kudo commented 4 years ago

Well, as https://github.com/Kudo/jsc-android-buildscripts/issues/3 mentioned, I am not able to maintain react-native-community/jsc-android-buildscripts. That's why I went for a forked version.

I guessed from the past days, RN default to use JSC and that's the reason to have maintainers strictly. If RN by default stick to Hermes, jsc-android could be a real community driven project.

I would be interested, but I haven't done development in JavaScriptCore so I'd need lots of documentation and ramp-up time.

@newyankeecodeshop To be honestly, I am not familiar JavaScriptCore internal as well. What I did is only about to build JSC Android version 😅

newyankeecodeshop commented 4 years ago

Thanks @Kudo I just assumed you are a maintainer because you are one of the largest contributors here. 🥇