From 48e14db2355254ffc3e54b245cea1f35875ce9aa 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.60 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. 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.60/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 | 22 ++++------- ios/ZulipMobile-Bridging-Header.h | 1 - ios/ZulipMobile/AppDelegate.h | 3 +- ios/ZulipMobile/AppDelegate.m | 25 ++++++++---- jest.config.js | 1 + jest/jestSetup.js | 12 ++++++ package.json | 1 + src/notification/index.js | 66 ++++++++++++++----------------- yarn.lock | 7 ++++ 10 files changed, 77 insertions(+), 64 deletions(-) diff --git a/ios/Podfile b/ios/Podfile index 7dd80ddd425..8afc1ad7ea9 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/jscallinvoker', :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 8c692141f4c..c609d9ad194 100644 --- a/ios/Podfile.lock +++ b/ios/Podfile.lock @@ -190,14 +190,6 @@ PODS: - React-jsi (= 0.61.5) - React-jsiexecutor (= 0.61.5) - Yoga - - React-Core/RCTPushNotificationHeaders (0.61.5): - - Folly (= 2018.10.22.00) - - glog - - React-Core/Default - - React-cxxreact (= 0.61.5) - - React-jsi (= 0.61.5) - - React-jsiexecutor (= 0.61.5) - - Yoga - React-Core/RCTSettingsHeaders (0.61.5): - Folly (= 2018.10.22.00) - glog @@ -294,8 +286,6 @@ PODS: - React-Core/RCTLinkingHeaders (= 0.61.5) - React-RCTNetwork (0.61.5): - React-Core/RCTNetworkHeaders (= 0.61.5) - - React-RCTPushNotification (0.61.5): - - React-Core/RCTPushNotificationHeaders (= 0.61.5) - React-RCTSettings (0.61.5): - React-Core/RCTSettingsHeaders (= 0.61.5) - React-RCTText (0.61.5): @@ -319,6 +309,8 @@ PODS: - React-Core - RNCAsyncStorage (1.6.3): - React + - RNCPushNotificationIOS (1.2.2): + - React - RNDeviceInfo (0.21.5): - React - RNSentry (1.6.3): @@ -413,7 +405,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`) @@ -421,6 +412,7 @@ DEPENDENCIES: - ReactCommon/turbomodule/core (from `../node_modules/react-native/ReactCommon`) - rn-fetch-blob (from `../node_modules/rn-fetch-blob`) - "RNCAsyncStorage (from `../node_modules/@react-native-community/async-storage`)" + - "RNCPushNotificationIOS (from `../node_modules/@react-native-community/push-notification-ios`)" - RNDeviceInfo (from `../node_modules/react-native-device-info`) - "RNSentry (from `../node_modules/@sentry/react-native`)" - RNSound (from `../node_modules/react-native-sound`) @@ -528,8 +520,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: @@ -542,6 +532,8 @@ EXTERNAL SOURCES: :path: "../node_modules/rn-fetch-blob" RNCAsyncStorage: :path: "../node_modules/@react-native-community/async-storage" + RNCPushNotificationIOS: + :path: "../node_modules/@react-native-community/push-notification-ios" RNDeviceInfo: :path: "../node_modules/react-native-device-info" RNSentry: @@ -626,13 +618,13 @@ SPEC CHECKSUMS: React-RCTImage: 6b8e8df449eb7c814c99a92d6b52de6fe39dea4e React-RCTLinking: 121bb231c7503cf9094f4d8461b96a130fabf4a5 React-RCTNetwork: fb353640aafcee84ca8b78957297bd395f065c9a - React-RCTPushNotification: 6d0c0cd88a40465677e07a614f65ea88c1c3de24 React-RCTSettings: 8db258ea2a5efee381fcf7a6d5044e2f8b68b640 React-RCTText: 9ccc88273e9a3aacff5094d2175a605efa854dbe React-RCTVibration: a49a1f42bf8f5acf1c3e297097517c6b3af377ad ReactCommon: 198c7c8d3591f975e5431bec1b0b3b581aa1c5dd rn-fetch-blob: f525a73a78df9ed5d35e67ea65e79d53c15255bc RNCAsyncStorage: 3c304d1adfaea02ec732ac218801cb13897aa8c0 + RNCPushNotificationIOS: 4c97a36dbec42dba411cc35e6dac25e34a805fde RNDeviceInfo: e7c5fcde13d40e161d8a27f6c5dc69c638936002 RNSentry: ae1e005e4f2655775475445a9c49c1d343e8e3a7 RNSound: da030221e6ac7e8290c6b43f2b5f2133a8e225b0 @@ -655,6 +647,6 @@ SPEC CHECKSUMS: Yoga: f2a7cd4280bfe2cca5a7aed98ba0eb3d1310f18b YogaKit: f782866e155069a2cca2517aafea43200b01fd5a -PODFILE CHECKSUM: b31e45ca912ffe41b915cb3dbc892ca3049708b7 +PODFILE CHECKSUM: a8a1bb97c8a90297760ede8d3e44a272c9c337fe 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.h b/ios/ZulipMobile/AppDelegate.h index d86977f0770..43d1c2e998d 100644 --- a/ios/ZulipMobile/AppDelegate.h +++ b/ios/ZulipMobile/AppDelegate.h @@ -1,8 +1,9 @@ #import #import #import +#import -@interface AppDelegate : UIResponder +@interface AppDelegate : UIResponder @property (nonatomic, strong) UMModuleRegistryAdapter *moduleRegistryAdapter; @property (nonatomic, strong) UIWindow *window; diff --git a/ios/ZulipMobile/AppDelegate.m b/ios/ZulipMobile/AppDelegate.m index 0569fd556ba..799e7bc3916 100644 --- a/ios/ZulipMobile/AppDelegate.m +++ b/ios/ZulipMobile/AppDelegate.m @@ -7,7 +7,7 @@ #import #import #import -#import +#import #import #import #import @@ -93,34 +93,43 @@ - (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. +// iOS 10+ Required for localNotification event +- (void)userNotificationCenter:(UNUserNotificationCenter *)center +didReceiveNotificationResponse:(UNNotificationResponse *)response + withCompletionHandler:(void (^)(void))completionHandler +{ + [RNCPushNotificationIOS didReceiveNotificationResponse:response]; + completionHandler(); +} + +// iOS 4-10 Required for the localNotification event. - (void)application:(UIApplication *)application didReceiveLocalNotification:(UILocalNotification *)notification { - [RCTPushNotificationManager didReceiveLocalNotification:notification]; + [RNCPushNotificationIOS didReceiveLocalNotification:notification]; } -// Called when a notification is delivered to a foreground app. +// iOS 10.0+: Called when a notification is delivered to a foreground app. -(void)userNotificationCenter:(UNUserNotificationCenter *)center willPresentNotification:(UNNotification *)notification withCompletionHandler:(void (^)(UNNotificationPresentationOptions options))completionHandler diff --git a/jest.config.js b/jest.config.js index 7b16fdf9eb1..10aec5ea03f 100644 --- a/jest.config.js +++ b/jest.config.js @@ -8,6 +8,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 0a4f052d8c0..4827dd54e42 100644 --- a/jest/jestSetup.js +++ b/jest/jestSetup.js @@ -55,6 +55,18 @@ jest.mock('react-native', () => { 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 cbde9522454..0113501f8f7 100644 --- a/package.json +++ b/package.json @@ -35,6 +35,7 @@ "@react-native-community/async-storage": "^1.6.3", "@react-native-community/cameraroll": "^1.7.2", "@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.1.2", "@zulip/shared": "^0.0.2", diff --git a/src/notification/index.js b/src/notification/index.js index f8bf436f45c..0402891e615 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'; @@ -118,8 +119,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; @@ -263,53 +264,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 c572f4bf0aa..5d72e99d209 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1503,6 +1503,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.5.0" + resolved "https://registry.yarnpkg.com/@react-native-community/push-notification-ios/-/push-notification-ios-1.5.0.tgz#b41802b58d2388c8cb76cd219bbeec9ac9a4c06d" + integrity sha512-88Uwu6S8oRVnuMfBMGN+MtTyUjiVmMKwfObYrPmm+b2E2Aqk0WlZ4clfECukG8QIzv1pfELJZ5uZMVTYMI6klg== + dependencies: + invariant "^2.2.4" + "@rollup/plugin-babel@^5.2.0": version "5.2.0" resolved "https://registry.yarnpkg.com/@rollup/plugin-babel/-/plugin-babel-5.2.0.tgz#b87556d61ed108b4eaf9d18b5323965adf8d9bee"