search

react-native-push-notification / ios

React Native Push Notification API for iOS.
MIT License
740 stars 284 forks source link

Inconsistency between docs and example regarding completionHandler() #244

Open xsamix opened 3 years ago

xsamix commented 3 years ago

Call to completionHandler() and code around are originally from here: https://github.com/react-native-push-notification-ios/push-notification-ios/pull/104/files

Here completionHandler() call is removed from README.md and example AppDelegate.m: https://github.com/react-native-push-notification-ios/push-notification-ios/pull/207/files

Here it is added back to example AppDelegate.m, but README.md is left missing: https://github.com/react-native-push-notification-ios/push-notification-ios/pull/241/files

Last pull addresses completion handling and is included in newest release, so this must be meaningful.

So my question to @Naturalclar is, which way should it be? Presumably example is correct and docs are lagging, but...

leethree commented 3 years ago

yes i've found the same issue. it's very confusing because completionHandler is supposed to be called.

apfritts commented 3 years ago

@leethree @xsamix you can't call the completionHandler() directly from the AppDelegate file because it will call before your JavaScript code executes (in my case anyway), especially if you do any asynchronous processing in your JavaScript handler. I believe this is actually a bug in the native code of this library and would love you to try out the updated version and let me know if it fixes your problem.

leethree commented 3 years ago

@apfritts i'm not familiar with iOS, but my understanding it that completionHandler should eventually be called (not necessarily in AppDelegate). is it correct?

the README doesn't give any guidance on that 🤷🏼

apfritts commented 3 years ago

@leethree by calling notification.finish() from Javascript, it should have been calling the completionHandler but wasn't. My PR fixes that so you shouldn't call completionHandler from AppDelegate.m and instead make sure your notification handler is calling the notification's finish method.

In my instance, if I call the completionHandler from the AppDelegate.m file, the actual Javascript notification code runs afterwards which means iOS may kill the app before your code runs.

leethree commented 3 years ago

@apfritts I see. It makes sense to me 👍🏼

malgorzatatanska commented 2 years ago

@apfritts is your PR merged? I use @react-native-community/push-notification-ios": "1.10.0" should your changes be available in this version?