ios icon indicating copy to clipboard operation
ios copied to clipboard

Inconsistency between docs and example regarding completionHandler()

Open xsamix opened this issue 4 years ago • 6 comments

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...

xsamix avatar Dec 03 '20 10:12 xsamix

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

leethree avatar Apr 28 '21 17:04 leethree

@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.

apfritts avatar Jul 25 '21 00:07 apfritts

@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 🤷🏼

leethree avatar Jul 26 '21 16:07 leethree

@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.

apfritts avatar Jul 26 '21 16:07 apfritts

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

leethree avatar Jul 26 '21 16:07 leethree

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

malgorzatatanska avatar Aug 25 '21 07:08 malgorzatatanska