invertase / notifee

⚛️ A feature rich notifications library for React Native.
https://notifee.app
Apache License 2.0
1.85k stars 223 forks source link

[iOS] Notifee somehow overwrites RNFBMessaging's userNotificationCenter:didRecieveNotificationResponse:withCompletionHandler, causing onNotificationOpenedApp() to never get called #644

Closed Avishayy closed 1 year ago

Avishayy commented 1 year ago

Preface

  1. Lately I've updated my project

    • React Native 0.67.2 -> 0.70.7
    • notifee 7.0.4 -> 7.1.0
    • react-native-firebase from 14.3.0 -> 16.4.6
    • Which now requires $RNFirebaseAsStaticFramework = true with use_frameworks! :linkage => :static
  2. Deeplinking from notifications stopped working only on iOS (Android working as expected)

    • setBackgroundMessageHandler(cb), cb is called as expected
    • onMessage(cb), cb is called as expected
    • onNotificationOpenedApp(cb), cb is NEVER called, so we never resolve the link from an opened notification
  3. I'm not familiar that much with Objective C, if I'm missing something completely obvious, please let me know.

What I did

I've started debugging the native code and found this piece of code which should

- (void)userNotificationCenter:(UNUserNotificationCenter *)center
    didReceiveNotificationResponse:(UNNotificationResponse *)response
             withCompletionHandler:(void (^)(void))completionHandler {
...
    [[RNFBRCTEventEmitter shared] sendEventWithName:@"messaging_notification_opened"
                                               body:notificationDict];
...
}

in RNFBMessaging+UNUserNotificationCenter.m

I put a breakpoint at the beginning of the function, it was never called. I searched a bit and found out that notifee also has the same function under NotifeeCoUNUserNotificationCenter.m.

I put a breakpoint there, and voilà, it stops on notifee's function. I looked a bit at the code that does the swizzling (I think?), and it looks like notifee's overwritten react-native-firebase's call. I'm still trying to understand the whole flow but I thought I'd open an issue here to see if there's quicker feedback. Here's the stack trace for this call image

For what it's worth, I uninstalled notifee and commented out its references in the code, and onNotificationOpenedApp() worked as usual.

Any help is appreciated, thank you.

Avishayy commented 1 year ago

Looks like it's somewhat intentional? https://github.com/invertase/notifee/issues/616

I'll note that I'm referring only for receiving a notification in the background, and there's no notifee.onNotificationOpenedApp() equivalent

Avishayy commented 1 year ago

I think I figured it out, the overwrite is (probably) intentional.

The confusion originated from several factors:

  1. The docs aren't clear enough on the fact rather than working alongside react-native-firebase, you overwrite its functionality
  2. The overwritten functionality does work on Android (which makes me question whether it's intentional)
  3. The overwritten functionality did work before making the upgrades 4.notifee.onBackgroundEvent() doesn't get triggered, but notifee.onForegroundEvent() does

I think the lack of documentation on this regard and the inconsistency between Android and iOS is harmful, for now I'll just change the functions I use, as the fix in my case should be trivial.

I'm leaving this open to hear about notifee's opinion.

helenaford commented 1 year ago

Thank you for your feedback! The decision to do this was needed for the most common use-case with quick actions and to simplify the event handling process. The plan is to do the same with Android.

Will close this in favour of https://github.com/invertase/notifee/issues/616. I will make this a priority to update docs.