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

Intl: toLocaleTimeString does not support timeZone shift #431

Closed
1 task done
dlebedynskyi opened this issue Dec 9, 2020 · 3 comments
Closed
1 task done

Intl: toLocaleTimeString does not support timeZone shift #431

dlebedynskyi opened this issue Dec 9, 2020 · 3 comments

Comments

@dlebedynskyi
Copy link

Bug Description

I believe this should be a part of #23.

tl;dr;
timeZone options does not adjust time. As result toLocaleTimeString always uses timezone of the device and does not allow time adjustment to other timezones.

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

Hermes version: 0.7.1
React Native version (if any): 0.64
Android version (if any): 10, android SDK simulator
Platform (most likely one of arm64-v8a, armeabi-v7a, x86, x86_64):x86

Steps To Reproduce

  1. create brand new RN app
  2. link 0.7.1 hermes - I've linked published from releases page
  3. add following code to demo project

code example:

const time = 1607370988273; // 4:56 PM Monday, December 7, 2020 (GMT-3), Time in São Paulo, State of São Paulo, Brazil
const d = new Date(time);
const timeZone = 'America/Sao_Paulo';

<Button title="Test" onPress={() => {
              console.info('Time in Brazil', {
                d,
                actualJS: d.toLocaleTimeString('en-us'),
                actualJSWithTimezone: d.toLocaleTimeString('en-us', { timeZone }),
                expected: '4:56 PM',
                moment: moment(time).tz(timeZone).format('HH:mm:ss'),  // always correct on latest version
              });
          }} />
// device is using at Los Angeles time zone 
const time = 1607370988273; // 4:56 PM Monday, December 7, 2020 (GMT-3), Time in São Paulo, State of São Paulo, Brazil
const d = new Date(time);
const timeZone = 'America/Sao_Paulo';

d.toLocaleTimeString('en-us') // "11:56:28 AM" - CORRECT  as LA timezone applied
d.toLocaleTimeString('en-us', { timeZone }) // "11:56:28 AM" - WRONG - should be 4:56
// device in Brazil São Paulo
d.toLocaleTimeString('en-us') // "4:56:28 PM" - CORRECT  as LA timezone applied
d.toLocaleTimeString('en-us', { timeZone }) // "4:56:28 PM" - same as it is using current device timezone

Note: JSC behaviour

JSC actually has exact same behavior as Hermes now does

// in LA
Time in Brazil {"actualJS": "11:56:28", "actualJSWithTimezone": "11:56:28", "d": 2020-12-07T19:56:28.273Z, "dayjs": "11:56:28", "dayjsInLocaltimezone": "11:56:28", "expected": "4:56 PM", "moment": "16:56:28"}
 BUNDLE  ./index.js

// in Sao_Paulo
 LOG  Running "RN064" with {"rootTag":1}
Time in Brazil {"actualJS": "16:56:28", "actualJSWithTimezone": "16:56:28", "d": 2020-12-07T19:56:28.273Z, "dayjs": "16:56:28", "dayjsInLocaltimezone": "16:56:28", "expected": "4:56 PM", "moment": "16:56:28"}

The Expected Behavior

Expected - d.toLocaleTimeString('en-us', { timeZone }) will adjust timezone to desired and will allow to override device timezone. Bahaviour is expected to match V8 (Chrome) or Firefox.

@dulinriley
Copy link
Contributor

Hi @dlebedynskyi, thanks for reporting this. Hermes currently doesn't have full Intl support, and one of the things we don't support is Date.prototype.toLocaleTimeString. We'll format the date, but we ignore all of the arguments (both 'en-us' and { timeZone }).

We're actively working on an Intl implementation in #23 (as you mention), and once we release support for that, this should work. We don't currently have a target release where Intl is included, but it is in active development right now.

So I'm going to close this issue for now, and we can re-open if it's still occurring once the Intl implementation is released.

@dulinriley
Copy link
Contributor

As for JSC, I believe RN + JSC releases a version of JSC with Intl included, which should fix this problem on JSC.
There's some detail about how to use this in the JSC Android build scripts repo:
https://github.com/react-native-community/jsc-android-buildscripts#international-variant

@dlebedynskyi
Copy link
Author

dlebedynskyi commented Dec 9, 2020

We're actively working on an Intl implementation in #23 (as you mention), and once we release support for that, this should work. We don't currently have a target release where Intl is included, but it is in active development right now.

So I'm going to close this issue for now, and we can re-open if it's still occurring once the Intl implementation is released.

@dulinriley
Fair point. Would be great if above support is implemented, was not sure about " ignore all of the arguments". Hopefully we can get it in soon.

Update: JSC with Intl still has issue with Brazil DSL.

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

No branches or pull requests

2 participants