search

react-native-push-notification / ios

React Native Push Notification API for iOS.
MIT License
741 stars 285 forks source link

v1.4.1 has an unmentioned breaking change #236

Open nbolender opened 3 years ago

nbolender commented 3 years ago

PR #175 published in v1.4.1 changed how local notification data is transformed such that it breaks local notifications if you have not updated your AppDelegate to use UNUserNotificationCenter.

Can we add some documentation to make it clear that you must change your AppDelegate? If you are still using application:didReceiveLocalNotification:, the notification will appear but the title and body will be undefined.

cristianoccazinsp commented 3 years ago

Can you clarify what has changed or needs to be changed? I'm still using application:didReceiveLocalNotification but also - (void)userNotificationCenter:(UNUserNotificationCenter *)center didReceiveNotificationResponse:(UNNotificationResponse *)response withCompletionHandler:(void (^)(void))completionHandler so I'm not sure what's wrong.

nbolender commented 3 years ago

v1.2.0 introduced support for UNUserNotificationCenter and added userNotificationCenter:didReceiveNotificationResponse:withCompletionHandler: to the readme. Once you set up UNUserNotificationCenter, I believe that application:didReceiveLocalNotification: is not called anymore.

Then, either starting at that version or sometime later, the problem in https://github.com/react-native-push-notification-ios/push-notification-ios/issues/169 was introduced, because the notification object received by application:didReceiveLocalNotification: and userNotificationCenter:didReceiveNotificationResponse:withCompletionHandler: have different schemas. This problem only occurs if you have set up UNUserNotificationCenter.

The fix in v1.4.1 corrects this ONLY for apps that have the newer UNUserNotificationCenter setup, and it actually introduces the same bug if you have not yet done that (your app is still using only application:didReceiveLocalNotification).

So the result is that the AppDelegate setup described in the current readme is required, but it was not mentioned as required at any point in the changelog.

I came across this issue as I was upgrading from v1.0.2 to v1.7.1, and luckily I noticed that local notification data was coming through as undefined.

So I propose that this change be mentioned in the changelog and/or readme, and maybe we should remove support for application:didReceiveLocalNotification: altogether, which was deprecated in iOS 10.