From 5bda5b52fe0f13ef63f9cac5cc78d5f0d2c3417b 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 | 3 -- ios/Podfile.lock | 25 ++++-------- 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(+), 67 deletions(-) diff --git a/ios/Podfile b/ios/Podfile index 0ae06ab11a5..a419a3999d8 100644 --- a/ios/Podfile +++ b/ios/Podfile @@ -74,9 +74,6 @@ target 'ZulipMobile' do pod 'React-jsi', :path => '../node_modules/react-native/ReactCommon/jsi' pod 'React-jsiexecutor', :path => '../node_modules/react-native/ReactCommon/jsiexecutor' pod 'React-jsinspector', :path => '../node_modules/react-native/ReactCommon/jsinspector' - # This is deprecated upstream; removing it is #4115. See - # https://reactnative.dev/docs/pushnotificationios. - pod 'React-RCTPushNotification', :path => '../node_modules/react-native/Libraries/PushNotificationIOS' pod 'ReactCommon/callinvoker', :path => "../node_modules/react-native/ReactCommon" pod 'ReactCommon/turbomodule/core', :path => "../node_modules/react-native/ReactCommon" pod 'Yoga', :path => '../node_modules/react-native/ReactCommon/yoga', :modular_headers => true diff --git a/ios/Podfile.lock b/ios/Podfile.lock index a62434d6eac..a527969533f 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -193,14 +193,6 @@ PODS: - React-jsi (= 0.62.2) - React-jsiexecutor (= 0.62.2) - Yoga - - React-Core/RCTPushNotificationHeaders (0.62.2): - - Folly (= 2018.10.22.00) - - glog - - React-Core/Default - - React-cxxreact (= 0.62.2) - - React-jsi (= 0.62.2) - - React-jsiexecutor (= 0.62.2) - - Yoga - React-Core/RCTSettingsHeaders (0.62.2): - Folly (= 2018.10.22.00) - glog @@ -316,11 +308,6 @@ PODS: - RCTTypeSafety (= 0.62.2) - React-Core/RCTNetworkHeaders (= 0.62.2) - ReactCommon/turbomodule/core (= 0.62.2) - - React-RCTPushNotification (0.62.2): - - FBReactNativeSpec (= 0.62.2) - - RCTTypeSafety (= 0.62.2) - - React-Core/RCTPushNotificationHeaders (= 0.62.2) - - ReactCommon/turbomodule/core (= 0.62.2) - React-RCTSettings (0.62.2): - FBReactNativeSpec (= 0.62.2) - Folly (= 2018.10.22.00) @@ -353,6 +340,8 @@ PODS: - React - RNCMaskedView (0.1.10): - React + - RNCPushNotificationIOS (1.7.3): + - React-Core - RNDeviceInfo (6.0.2): - React - RNGestureHandler (1.8.0): @@ -453,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 +450,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 +564,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 +578,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 +670,6 @@ SPEC CHECKSUMS: React-RCTImage: e70be9b9c74fe4e42d0005f42cace7981c994ac3 React-RCTLinking: c1b9739a88d56ecbec23b7f63650e44672ab2ad2 React-RCTNetwork: 73138b6f45e5a2768ad93f3d57873c2a18d14b44 - React-RCTPushNotification: d544b6b5bf8def493b87da896a1cc0f4b61291c7 React-RCTSettings: 6e3738a87e21b39a8cb08d627e68c44acf1e325a React-RCTText: fae545b10cfdb3d247c36c56f61a94cfd6dba41d React-RCTVibration: 4356114dbcba4ce66991096e51a66e61eda51256 @@ -689,6 +677,7 @@ SPEC CHECKSUMS: rn-fetch-blob: f525a73a78df9ed5d35e67ea65e79d53c15255bc RNCAsyncStorage: 3c304d1adfaea02ec732ac218801cb13897aa8c0 RNCMaskedView: 5a8ec07677aa885546a0d98da336457e2bea557f + RNCPushNotificationIOS: edeeea93b7210d2f918fc0e46e1a2a189b5fdacc RNDeviceInfo: bdd61e8b070d13a1dd9d022091981075ed4cde16 RNGestureHandler: 7a5833d0f788dbd107fbb913e09aa0c1ff333c39 RNReanimated: 89f5e0a04d1dd52fbf27e7e7030d8f80a646a3fc @@ -713,6 +702,6 @@ SPEC CHECKSUMS: Yoga: 3ebccbdd559724312790e7742142d062476b698e YogaKit: f782866e155069a2cca2517aafea43200b01fd5a -PODFILE CHECKSUM: 225a865018bba07be314e8afa4e91319ab40adac +PODFILE CHECKSUM: bb21f5f2cffae3c57e18eab3696b63eff8e52585 COCOAPODS: 1.9.3 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 a351f537e26..f8ab87ee125 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 a8267491857..71f133c986e 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 3bf71e5dd0f..4d50ad011dc 100644 --- a/jest/jestSetup.js +++ b/jest/jestSetup.js @@ -67,6 +67,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 4648af6907b..7082b147c16 100644 --- a/package.json +++ b/package.json @@ -36,6 +36,7 @@ "@react-native-community/cameraroll": "^1.7.2", "@react-native-community/masked-view": "^0.1.10", "@react-native-community/netinfo": "^5.9.5", + "@react-native-community/push-notification-ios": "^1.5.0", "@sentry/react-native": "^1.0.9", "@unimodules/core": "~5.3.0", "@zulip/shared": "^0.0.2", diff --git a/src/notification/index.js b/src/notification/index.js index f9b2e468aee..05b2231d09d 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, User } from '../types'; @@ -122,8 +123,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; @@ -177,7 +178,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. */ @@ -272,53 +273,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 377392866c9..c346e2e87a1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2200,6 +2200,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.7.3" + resolved "https://registry.yarnpkg.com/@react-native-community/push-notification-ios/-/push-notification-ios-1.7.3.tgz#7b41add329996df59382820d8a729fe609c2fb75" + integrity sha512-SLGQMxSB4WTvATjCXELxansnseLcmqJ6jIC8U4AyjxL30k3m1YmbrO4wGMw/ZF/VSC1toK+a39aD+ozDjon3mw== + dependencies: + invariant "^2.2.4" + "@react-navigation/core@^3.7.6": version "3.7.6" resolved "https://registry.yarnpkg.com/@react-navigation/core/-/core-3.7.6.tgz#e0244fcdc22937825b252197f70308bbe5709c58"