From adbbab17c7384183ab7506fcee26eb6be7c5ff9f Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Tue, 16 Jun 2020 13:57:33 -0700 Subject: [PATCH] ios deps: Use PushNotificationIOS from react-native-community, not RN core. In a recent commit, we asked RN's PushNotificationIOS to take responsibilities that were previously taken by wix/react-native-notifications, and removed the Wix library. Now, we account for the fact that PushNotificationsIOS from RN is deprecated [1] and asks us to use @react-native-community/push-notification-ios instead. So, do. These were my steps: 1. Use the setup instructions for `PushNotificationIOS` [1] from RN v0.62 to tear it down. 2. Follow the setup instructions for @react-native-community/push-notification-ios at the latest, v1.5.0 [2]. The native code closely matches what was there before, which makes sense. There are a few additions, and notes on old iOS APIs. We no longer support iOS 10, so we leave out some methods that target iOS <=10. 3. Follow the simple "Migrating..." instructions [3] that say you just have to change the imports; no call site changes are necessary. 4. Change a few comments that refer to details of the directory structure or implementation of the library. 5. Test thoroughly, as in the previous commit, and observe the same results. [1]: https://reactnative.dev/docs/0.62/pushnotificationios [2]: https://github.com/react-native-community/push-notification-ios/blob/v1.5.0/README.md [3]: https://github.com/react-native-community/push-notification-ios/blob/v1.5.0/README.md#migrating-from-the-core-react-native-module Fixes: #4115 --- ios/Podfile | 4 -- ios/Podfile.lock | 26 ++++-------- ios/ZulipMobile-Bridging-Header.h | 1 - ios/ZulipMobile/AppDelegate.m | 19 +++++---- jest.config.js | 1 + jest/jestSetup.js | 12 ++++++ package.json | 1 + src/notification/index.js | 68 ++++++++++++++----------------- yarn.lock | 7 ++++ 9 files changed, 70 insertions(+), 69 deletions(-) diff --git a/ios/Podfile b/ios/Podfile index 1b6c2ba827f..e090ecc5983 100644 --- a/ios/Podfile +++ b/ios/Podfile @@ -10,10 +10,6 @@ target 'ZulipMobile' do # Pods from React Native use_react_native! - # This is deprecated upstream; removing it is #4115. See - # https://reactnative.dev/docs/pushnotificationios. - pod 'React-RCTPushNotification', :path => '../node_modules/react-native/Libraries/PushNotificationIOS' - # "Autolinking": automatically link pods that depend on React # Native, unless omitted in our own `react-native.config.js`. use_native_modules! diff --git a/ios/Podfile.lock b/ios/Podfile.lock index 50d167bc4a0..d18c0797480 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -192,14 +192,6 @@ PODS: - React-jsi (= 0.63.4) - React-jsiexecutor (= 0.63.4) - Yoga - - React-Core/RCTPushNotificationHeaders (0.63.4): - - Folly (= 2020.01.13.00) - - glog - - React-Core/Default - - React-cxxreact (= 0.63.4) - - React-jsi (= 0.63.4) - - React-jsiexecutor (= 0.63.4) - - Yoga - React-Core/RCTSettingsHeaders (0.63.4): - Folly (= 2020.01.13.00) - glog @@ -319,12 +311,6 @@ PODS: - React-Core/RCTNetworkHeaders (= 0.63.4) - React-jsi (= 0.63.4) - ReactCommon/turbomodule/core (= 0.63.4) - - React-RCTPushNotification (0.63.4): - - FBReactNativeSpec (= 0.63.4) - - RCTTypeSafety (= 0.63.4) - - React-Core/RCTPushNotificationHeaders (= 0.63.4) - - React-jsi (= 0.63.4) - - ReactCommon/turbomodule/core (= 0.63.4) - React-RCTSettings (0.63.4): - FBReactNativeSpec (= 0.63.4) - Folly (= 2020.01.13.00) @@ -354,6 +340,8 @@ PODS: - React - RNCMaskedView (0.1.10): - React + - RNCPushNotificationIOS (1.8.0): + - React-Core - RNDeviceInfo (6.0.2): - React - RNGestureHandler (1.8.0): @@ -454,7 +442,6 @@ DEPENDENCIES: - React-RCTImage (from `../node_modules/react-native/Libraries/Image`) - React-RCTLinking (from `../node_modules/react-native/Libraries/LinkingIOS`) - React-RCTNetwork (from `../node_modules/react-native/Libraries/Network`) - - React-RCTPushNotification (from `../node_modules/react-native/Libraries/PushNotificationIOS`) - React-RCTSettings (from `../node_modules/react-native/Libraries/Settings`) - React-RCTText (from `../node_modules/react-native/Libraries/Text`) - React-RCTVibration (from `../node_modules/react-native/Libraries/Vibration`) @@ -462,6 +449,7 @@ DEPENDENCIES: - rn-fetch-blob (from `../node_modules/rn-fetch-blob`) - "RNCAsyncStorage (from `../node_modules/@react-native-community/async-storage`)" - "RNCMaskedView (from `../node_modules/@react-native-community/masked-view`)" + - "RNCPushNotificationIOS (from `../node_modules/@react-native-community/push-notification-ios`)" - RNDeviceInfo (from `../node_modules/react-native-device-info`) - RNGestureHandler (from `../node_modules/react-native-gesture-handler`) - RNReanimated (from `../node_modules/react-native-reanimated`) @@ -575,8 +563,6 @@ EXTERNAL SOURCES: :path: "../node_modules/react-native/Libraries/LinkingIOS" React-RCTNetwork: :path: "../node_modules/react-native/Libraries/Network" - React-RCTPushNotification: - :path: "../node_modules/react-native/Libraries/PushNotificationIOS" React-RCTSettings: :path: "../node_modules/react-native/Libraries/Settings" React-RCTText: @@ -591,6 +577,8 @@ EXTERNAL SOURCES: :path: "../node_modules/@react-native-community/async-storage" RNCMaskedView: :path: "../node_modules/@react-native-community/masked-view" + RNCPushNotificationIOS: + :path: "../node_modules/@react-native-community/push-notification-ios" RNDeviceInfo: :path: "../node_modules/react-native-device-info" RNGestureHandler: @@ -681,7 +669,6 @@ SPEC CHECKSUMS: React-RCTImage: c1b1f2d3f43a4a528c8946d6092384b5c880d2f0 React-RCTLinking: 35ae4ab9dc0410d1fcbdce4d7623194a27214fb2 React-RCTNetwork: 29ec2696f8d8cfff7331fac83d3e893c95ef43ae - React-RCTPushNotification: 068f2d369fcf63b6f37766a1b821b008fa86363f React-RCTSettings: 60f0691bba2074ef394f95d4c2265ec284e0a46a React-RCTText: 5c51df3f08cb9dedc6e790161195d12bac06101c React-RCTVibration: ae4f914cfe8de7d4de95ae1ea6cc8f6315d73d9d @@ -689,6 +676,7 @@ SPEC CHECKSUMS: rn-fetch-blob: f525a73a78df9ed5d35e67ea65e79d53c15255bc RNCAsyncStorage: 3c304d1adfaea02ec732ac218801cb13897aa8c0 RNCMaskedView: 5a8ec07677aa885546a0d98da336457e2bea557f + RNCPushNotificationIOS: 61a7c72bd1ebad3568025957d001e0f0e7b32191 RNDeviceInfo: bdd61e8b070d13a1dd9d022091981075ed4cde16 RNGestureHandler: 7a5833d0f788dbd107fbb913e09aa0c1ff333c39 RNReanimated: 89f5e0a04d1dd52fbf27e7e7030d8f80a646a3fc @@ -713,6 +701,6 @@ SPEC CHECKSUMS: Yoga: 4bd86afe9883422a7c4028c00e34790f560923d6 YogaKit: f782866e155069a2cca2517aafea43200b01fd5a -PODFILE CHECKSUM: 1f0c1f7ee60f9a3456214fc0b3986dc387904891 +PODFILE CHECKSUM: 0c736f27fde9a12cc155e76c2ff6b83a2c3bf41a COCOAPODS: 1.10.1 diff --git a/ios/ZulipMobile-Bridging-Header.h b/ios/ZulipMobile-Bridging-Header.h index 85b5abcfac0..1864f48a30b 100644 --- a/ios/ZulipMobile-Bridging-Header.h +++ b/ios/ZulipMobile-Bridging-Header.h @@ -6,4 +6,3 @@ #import #import #import -#import diff --git a/ios/ZulipMobile/AppDelegate.m b/ios/ZulipMobile/AppDelegate.m index 1bc3c136679..ec00f9cc5e4 100644 --- a/ios/ZulipMobile/AppDelegate.m +++ b/ios/ZulipMobile/AppDelegate.m @@ -7,7 +7,7 @@ #import #import #import -#import +#import #import #import #import @@ -99,31 +99,34 @@ - (BOOL)application:(UIApplication *)application continueUserActivity:(NSUserAct // Required to register for notifications - (void)application:(UIApplication *)application didRegisterUserNotificationSettings:(UIUserNotificationSettings *)notificationSettings { - [RCTPushNotificationManager didRegisterUserNotificationSettings:notificationSettings]; + [RNCPushNotificationIOS didRegisterUserNotificationSettings:notificationSettings]; } // Required for the register event. - (void)application:(UIApplication *)application didRegisterForRemoteNotificationsWithDeviceToken:(NSData *)deviceToken { - [RCTPushNotificationManager didRegisterForRemoteNotificationsWithDeviceToken:deviceToken]; + [RNCPushNotificationIOS didRegisterForRemoteNotificationsWithDeviceToken:deviceToken]; } // Required for the registrationError event. - (void)application:(UIApplication *)application didFailToRegisterForRemoteNotificationsWithError:(NSError *)error { - [RCTPushNotificationManager didFailToRegisterForRemoteNotificationsWithError:error]; + [RNCPushNotificationIOS didFailToRegisterForRemoteNotificationsWithError:error]; } // Required for the notification event. You must call the completion handler after handling the remote notification. - (void)application:(UIApplication *)application didReceiveRemoteNotification:(NSDictionary *)userInfo fetchCompletionHandler:(void (^)(UIBackgroundFetchResult))completionHandler { - [RCTPushNotificationManager didReceiveRemoteNotification:userInfo fetchCompletionHandler:completionHandler]; + [RNCPushNotificationIOS didReceiveRemoteNotification:userInfo fetchCompletionHandler:completionHandler]; } -// Required for the localNotification event. -- (void)application:(UIApplication *)application didReceiveLocalNotification:(UILocalNotification *)notification +// Required for localNotification event +- (void)userNotificationCenter:(UNUserNotificationCenter *)center +didReceiveNotificationResponse:(UNNotificationResponse *)response + withCompletionHandler:(void (^)(void))completionHandler { - [RCTPushNotificationManager didReceiveLocalNotification:notification]; + [RNCPushNotificationIOS didReceiveNotificationResponse:response]; + completionHandler(); } // Called when a notification is delivered to a foreground app. diff --git a/jest.config.js b/jest.config.js index b8834e9e9ed..e0885cbb26a 100644 --- a/jest.config.js +++ b/jest.config.js @@ -9,6 +9,7 @@ const transformModulesWhitelist = [ // @rnc/async-storage itself is precompiled, but its mock-helper is not '@react-native-community/async-storage', '@react-native-community/cameraroll', + '@react-native-community/push-notification-ios', '@expo/react-native-action-sheet', 'react-navigation', '@sentry/react-native', diff --git a/jest/jestSetup.js b/jest/jestSetup.js index d4d3beb0217..93dd67df788 100644 --- a/jest/jestSetup.js +++ b/jest/jestSetup.js @@ -82,6 +82,18 @@ jest.mock('react-native-reanimated', () => { jest.mock('@react-native-community/async-storage', () => mockAsyncStorage); +// Without this, we get lots of these errors on importing the module: +// `Invariant Violation: Native module cannot be null.` +jest.mock('@react-native-community/push-notification-ios', () => ({ + presentLocalNotification: jest.fn(), + scheduleLocalNotification: jest.fn(), + cancelAllLocalNotifications: jest.fn(), + removeAllDeliveredNotifications: jest.fn(), + getDeliveredNotifications: jest.fn(), + removeDeliveredNotifications: jest.fn(), + // etc. (incomplete) +})); + jest.mock('react-native-sound', () => () => ({ play: jest.fn(), })); diff --git a/package.json b/package.json index 96f61ebdc33..621a90b6de2 100644 --- a/package.json +++ b/package.json @@ -41,6 +41,7 @@ "@react-navigation/material-top-tabs": "^5.2.19", "@react-navigation/native": "^5.7.6", "@react-navigation/stack": "^5.9.3", + "@react-native-community/push-notification-ios": "^1.5.0", "@sentry/react-native": "^2.2.1", "@unimodules/core": "~5.3.0", "@zulip/shared": "^0.0.5", diff --git a/src/notification/index.js b/src/notification/index.js index e50d8ad5447..6ba8bd6ef44 100644 --- a/src/notification/index.js +++ b/src/notification/index.js @@ -1,6 +1,7 @@ /* @flow strict-local */ -import { DeviceEventEmitter, NativeModules, Platform, PushNotificationIOS } from 'react-native'; -import type { PushNotificationEventName } from 'react-native/Libraries/PushNotificationIOS/PushNotificationIOS'; +import { DeviceEventEmitter, NativeModules, Platform } from 'react-native'; +import PushNotificationIOS from '@react-native-community/push-notification-ios'; +import type { PushNotificationEventName } from '@react-native-community/push-notification-ios'; import type { Notification } from './types'; import type { Auth, Dispatch, Identity, Narrow, UserId, UserOrBot } from '../types'; @@ -123,8 +124,8 @@ const getInitialNotification = async (): Promise => { } // This is actually typed as ?Object (and so effectively `any`); but if - // present, it must be a JSONable dictionary. (See PushNotificationIOS.js and - // RCTPushNotificationManager.m in Libraries/PushNotificationIOS.) + // present, it must be a JSONable dictionary. (See js/index.js and + // ios/RNCPushNotificationIOS.m in @rnc/push-notification-ios.) const data: ?JSONableDict = notification.getData(); if (!data) { return null; @@ -178,7 +179,7 @@ export class NotificationListener { // 'register' -> 'remoteNotificationsRegistered' // 'registrationError' -> 'remoteNotificationRegistrationError' PushNotificationIOS.addEventListener(name, handler); - this.unsubs.push(() => PushNotificationIOS.removeEventListener(name, handler)); + this.unsubs.push(() => PushNotificationIOS.removeEventListener(name)); } /** Private. */ @@ -288,53 +289,46 @@ export class NotificationListener { export const getNotificationToken = () => { if (Platform.OS === 'ios') { // This leads to a call in RNCPushNotificationIOS to this, to - // maybe prompt the user to grant permissions: - // https://developer.apple.com/documentation/uikit/uiapplication/1622932-registerusernotificationsettings?language=objc - // (deprecated after iOS 10, yikes!). + // maybe prompt the user to grant authorization, with options for + // what things to authorize (badge, sound, alert, etc.): + // https://developer.apple.com/documentation/usernotifications/unusernotificationcenter/1649527-requestauthorizationwithoptions // - // (Then, in case we're interested, the library ensures that the - // Promise we get will resolve with the user's notification - // settings for the app: whether they've enabled alerts, the badge - // count, and sound.) + // If authorization is granted, the library calls this, to have the + // application register for remote notifications: + // https://developer.apple.com/documentation/appkit/nsapplication/2967172-registerforremotenotifications?language=occ // - // The above-mentioned `registerUserNotificationSettings` function - // reports the permissions-request result to the app; - // `AppDelegate`'s `didRegisterUserNotificationSettings` method - // gets called (it's also deprecated): - // https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1623022-application?language=objc - // Following the library's setup instructions, we've asked that - // method to hand control back to the library. + // (Then, in case we're interested, the library calls + // https://developer.apple.com/documentation/usernotifications/unusernotificationcenter/1649524-getnotificationsettingswithcompl + // and sets the eventual result to be the resolution of the + // Promise we get, so we can know if the user has enabled + // alerts, the badge count, and sound.) // - // If authorization is granted, the library calls this (not - // deprecated), to have the application register for remote - // notifications: - // https://developer.apple.com/documentation/appkit/nsapplication/2967172-registerforremotenotifications?language=occ - // That function ends up sending the app a device token; the app - // receives it in our own code: `AppDelegate`'s + // The above-mentioned `registerForRemoteNotifications` function + // ends up sending the app a device token; the app receives it in + // our own code: `AppDelegate`'s // `didRegisterForRemoteNotificationsWithDeviceToken` method: - // https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1622958-application?language=objc - // (not deprecated). Following the library's setup instructions, - // we've asked that method to hand control back to the library. + // https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1622958-application?language=objc. + // Following the library's setup instructions, we've asked that + // method to hand control back to the library. // // It looks like the library then creates a notification, with the // magic-string name "RemoteNotificationsRegistered", using - // https://developer.apple.com/documentation/foundation/nsnotificationcenter/1415812-postnotificationname?language=objc - // (not deprecated). + // https://developer.apple.com/documentation/foundation/nsnotificationcenter/1415812-postnotificationname?language=objc. // It listens for this notification with // https://developer.apple.com/documentation/foundation/nsnotificationcenter/1415360-addobserver - // (not deprecated) and, upon receipt, sends a React Native event - // to the JavaScript part of the library. We can listen to this - // event, with `PushNotificationIOS.addEventListener`, under the - // alias 'register'. (We can also listen for registration failure - // under the alias 'registrationError'.) + // and, upon receipt, sends a React Native event to the JavaScript + // part of the library. We can listen to this event, with + // `PushNotificationIOS.addEventListener`, under the alias + // 'register'. (We can also listen for registration failure under + // the alias 'registrationError'.) // // In short, this kicks off a sequence: - // permissions / register settings -> + // authorization, with options -> // register for remote notifications -> // send event we already have a global listener for // // This satisfies the stern warnings in Apple's docs (at the above - // links) to request permissions before registering with the push + // links) to request authorization before registering with the push // notification service. PushNotificationIOS.requestPermissions(); } else { diff --git a/yarn.lock b/yarn.lock index e9312adaada..dce53338f78 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2250,6 +2250,13 @@ resolved "https://registry.yarnpkg.com/@react-native-community/netinfo/-/netinfo-5.9.5.tgz#3bad0d855d2e813be085ec305139d4175c512ccc" integrity sha512-PbSsRmhRwYIMdeVJTf9gJtvW0TVq/hmgz1xyjsrTIsQ7QS7wbMEiv1Eb/M/y6AEEsdUped5Axm5xykq9TGISHg== +"@react-native-community/push-notification-ios@^1.5.0": + version "1.8.0" + resolved "https://registry.yarnpkg.com/@react-native-community/push-notification-ios/-/push-notification-ios-1.8.0.tgz#7d43cb37b9e644ff3069aafa91befe71688a2dc4" + integrity sha512-vxvkeampafjmtDoQBN8Q4azP21l6cMX93+OQZemyIWZmG++OjCVDQVitobf/kWLm5zyGwdylejbpMGo75qo7rA== + dependencies: + invariant "^2.2.4" + "@react-navigation/bottom-tabs@^5.10.6": version "5.11.2" resolved "https://registry.yarnpkg.com/@react-navigation/bottom-tabs/-/bottom-tabs-5.11.2.tgz#5b541612fcecdea2a5024a4028da35e4a727bde6"