NativeScript / ios

NativeScript for iOS and visionOS using V8
https://docs.nativescript.org/guide/ios-marshalling
131 stars 33 forks source link

Timezone changes are not correctly applied to NS runtime #216

Closed cjohn001 closed 5 months ago

cjohn001 commented 1 year ago

Issue Description

When changing the timezone settings on iOS the locale seems to be changed, but the time is not set to the correct time for that timezone.

Here is an example on IOS from Date object:

Starting in London timezone: time: Mon Jun 05 2023 15:50:00 GMT+0100 (BST)

switching timezone to CEST (GMT+0200): time: Mon Jun 05 2023 15:50:15 GMT+0100 (CEST) --> you can see, time zone was switched but time is not updated to reflect time in timezone (should be +0200)

After restarting application the timezone is set up correctly. time: Mon Jun 05 2023 16:53:10 GMT+0200 (CEST)

Please note, I had an initial issue in the NS core repo

https://github.com/NativeScript/NativeScript/issues/10300

However, seems like the correct place is here. When setting:

 "android": {
        "handleTimeZoneChanges": true
  } 

like recommended here:

https://github.com/NativeScript/NativeScript/issues/10300#issuecomment-1584389309

things are working correctly on Android.

I tried also setting this flag for"ios", however, this had no effect.

Reproduction

console.log( new Date());

switch timezone via device settings

Workaround

The only workaround I have found so far is forcing the app to close. On restart the timezone settings or ok.

Environment

OS: macOS 13.4
CPU: (10) arm64 Apple M1 Pro
Shell: /bin/zsh
node: 18.12.1
npm: 8.19.2
nativescript: 8.5.3

# android
java: 11.0.18
ndk: Not Found
apis: Not Found
build_tools: Not Found
system_images: Not Found

# ios
xcode: 14.3.1/14E300c
cocoapods: 1.12.0
python: 3.11.2
python3: 3.11.2
ruby: 2.7.7
platforms: 
  - DriverKit 22.4
  - iOS 16.4
  - macOS 13.3
  - tvOS 16.4
  - watchOS 9.4

Dependencies

"dependencies": {
  "@angular/animations": "16.0.3",
  "@angular/common": "16.0.3",
  "@angular/compiler": "16.0.3",
  "@angular/core": "16.0.3",
  "@angular/forms": "16.0.3",
  "@angular/platform-browser": "16.0.3",
  "@angular/platform-browser-dynamic": "16.0.3",
  "@angular/router": "16.0.3",
  "@apollo/client": "3.7.14",
  "@mnd/external-web-view": "file:../mnd-plugins/dist/packages/external-web-view/mnd-external-web-view-1.0.0.tgz",
  "@nativescript/angular": "16.0.0",
  "@nativescript/core": "8.5.3",
  "@nativescript/iqkeyboardmanager": "2.1.1",
  "@nativescript/localize": "5.1.0",
  "@nativescript/mlkit-barcode-scanning": "2.0.0",
  "@nativescript/mlkit-core": "2.0.0",
  "@nativescript/secure-storage": "3.0.0",
  "@nativescript/theme": "3.0.2",
  "@nativescript/ui-charts": "0.2.4",
  "apollo-angular": "5.0.0",
  "apollo3-cache-persist": "0.14.1",
  "d3-ease": "3.0.1",
  "graphql": "16.6.0",
  "graphql-tag": "2.12.6",
  "intl": "1.2.5",
  "moment": "2.29.4",
  "nativescript-health-data": "file:../mnd-custom-plugins/nativescript-health-data/publish/package/nativescript-health-data-2.0.0.tgz",
  "nativescript-oauth2-ext": "file:../mnd-custom-plugins/nativescript-oauth2-ext/publish/package/nativescript-oauth2-ext-3.0.1.tgz",
  "nativescript-sqlite": "2.8.6",
  "nativescript-ui-calendar": "15.2.3",
  "nativescript-ui-gauge": "15.2.3",
  "rxjs": "7.8.1",
  "uuid": "9.0.0",
  "zone.js": "0.13.0"
},
"devDependencies": {
  "@angular-devkit/build-angular": "16.0.3",
  "@angular/compiler-cli": "16.0.3",
  "@graphql-codegen/cli": "4.0.0",
  "@graphql-codegen/fragment-matcher": "5.0.0",
  "@graphql-codegen/introspection": "4.0.0",
  "@graphql-codegen/typescript": "4.0.0",
  "@graphql-codegen/typescript-apollo-angular": "3.5.6",
  "@graphql-codegen/typescript-operations": "4.0.0",
  "@nativescript/android": "8.5.0",
  "@nativescript/ios": "8.5.2",
  "@nativescript/types": "8.5.0",
  "@nativescript/webpack": "5.0.14",
  "@ngtools/webpack": "16.0.3",
  "@types/d3-ease": "3.0.0",
  "@types/intl": "1.2.0",
  "@types/node": "20.2.5",
  "@types/uuid": "9.0.1",
  "keycloak-request-token": "0.1.0",
  "rimraf": "5.0.1",
  "sass": "1.62.1",
  "ts-node": "10.9.1",
  "typescript": "4.9.5"
}
CatchABus commented 1 year ago

A note regarding this.

In android runtime, we have this feature configurable using handleTimeZoneChanges as mentioned above. See: https://github.com/NativeScript/android/pull/1033

In iOS runtime, we'll need a listener that calls V8's Date::DateTimeConfigurationChangeNotification to reset timezone.

cjohn001 commented 1 year ago

@CatchABus , thanks for the directions. The Android solution (handleTimeZoneChanges) I have already found. Do you know of an example app where I could see how this needs to be done on IOS? Would also be great if the info could be added to the NS documentation. Likely having timezone changes activitated by default would be quite helpful as well. I assume people expect that those are applied to the runtime till they realise it is not the case. Maybe a good change for future NS versions.

edusperoni commented 1 year ago

I created a draft that exposes the V8 function to the user, so you can manually reset the timezone (by attaching a notification observer and calling that function).

It's still a draft because I believe we can implement handleTimeZoneChanges in the runtime itself which would handle threading a bit better (to avoid unnecessary main thread locks when updating worker times)

cjohn001 commented 1 year ago

@edusperoni : thanks for the directions. I assume you mean I need to call it from an application event

Application.on(Application.resumeEvent, (args: ApplicationEventData) => {
   if(timezoneChanged){
      DefineDateTimeConfigurationChangeNotificationMethod()
   }
}

What would I have to provide for the parameters? v8::Isolate* isolate, v8::Local globalTemplate

Thanks for the directions. I am also wondering, why the timezone changes are not set automatically in the runtime. Should this not be the default behaviour? Was wondering about handleTimeZoneChanges on android side as well. I mean the ios and android runtimes also change the timezone automatically. Hence, as app developer I would expect the same behaviour with NS.

I think there should also be a refresh of all opened ui pages. I have seen on android, that I manually have to reload all data on all opened pages in order for the timezone change to take effect in the ui

edusperoni commented 1 year ago

@cjohn001 tbh I didn't even know that flag existed on Android 😅. The PR I did exposes the function on iOS so the user can register the notification listener and call that in userland. We're still debating if this change is better handled as in core (core will register the notification and call it) or in the runtime as Android does it.

Tbh I expect the TimeZone to be accurate even without any flags.

cjohn001 commented 1 year ago

@edusperoni : I am absolutely with you, handleTimeZoneChanges on android should be true by default and it would be great if the user would not have to handle things manually with a notification listener on ios as well. I will than wait for the result of your discussions :)

CatchABus commented 1 year ago

@cjohn001 tbh I didn't even know that flag existed on Android 😅. The PR I did exposes the function on iOS so the user can register the notification listener and call that in userland. We're still debating if this change is better handled as in core (core will register the notification and call it) or in the runtime as Android does it.

Tbh I expect the TimeZone to be accurate even without any flags.

@edusperoni I digged like a lot to find a 2018 PR implementing this and I was shocked about its existence as well :D I think your idea is much better than handleTimeZoneChanges configuration, and we could let core contain timezone change listeners for both platforms.

@cjohn001 If we call that runtime method for both platforms, android will have to use a BroadcastReceiver to track change by checking for Intent.ACTION_TIMEZONE_CHANGED See: https://github.com/NativeScript/android/blob/main/test-app/app/src/main/java/com/tns/RuntimeHelper.java#L258

Regarding iOS, I think an NSSystemTimeZoneDidChangeNotification observer would do to call the runtime method. Check this post: https://stackoverflow.com/a/27779954

edusperoni commented 1 year ago

@CatchABus my only issue is that it needs to be called per isolate, and the callbacks are called in the main thread. This means that it'll try to enter the worker isolates from the main thread which could lead to locking the main thread if the worker is doing heavy work. If done at the runtime level, we can dispatch a message to the worker's runloop.

If done at core level, I'd most likely create a native helper that would wrap the callback to replicate the same behavior.