feat(ios): make didRegisterForRemoteNotificationsWithDeviceToken handle Data and String push tokens

[bibyzan]

This PR enables passing strings to the PushNotifications registration event callback. This will enable user projects to pass any 3rd party token to the same event so they don't need to check which platform it is in JS to get the right kind of token.

This also includes a change to the push notifications firebase guide that adds the necessary lines to the user AppDelegate.swift to receive their FCM token in JS instead of the APNS one if they choose.

I have successfully built and tested this code locally to verify that it solves this problem (FCM token is passed through the same event on iOS and android). @jcesarmobile Let me know if there's anything about this implementation that might be inconsistent with the rest of the repo or some other reason why it shouldn't be merged.

[priyankpat]

You can test it by installing your forked repository locally by running npm i ../capacitor/ios, assuming your directory placement is as follow:

-- root
--- capacitor (forked repo)
--- test-project (test repo)

[bibyzan]

Hi @PR1YANKPAT3L I tried what you said but I need a package.json in that iOS directory for it to install. Perhaps I'm still missing a configuration or build step?

[bibyzan]

I was missing the steps outlined on this readme (https://github.com/ionic-team/capacitor/blob/master/example/README.md) which helped me install my local version of capacitor and test this

[atmosuwiryo]

Awesome, succeeded send request notification on my iOS device.

[jcesarmobile]

Thanks, but we won't be adding a new didRegisterForRemoteNotificationsWithFCMToken, that might confuse people into thinking that Capacitor supports FCM out of the box without any of the changes you've documented in the guide.

Maybe you can make minimal changes in didRegisterForRemoteNotificationsWithDeviceToken to handle both the Data (APSNs token) and the String (FCM or any other service token that has String format)

[bibyzan]

Good suggestion @jcesarmobile, I've removed the didRegisterForRemoteNotificationsWithFCMToken hook and enum case as well as adding a case to didRegisterForRemoteNotificationsWithDeviceToken that casts the token to a string if it fails to first cast it to a Data object. I also changed the push notification guide to use the regular CAPNotifications.DidRegisterForRemoteNotificationsWithDeviceToken instead of the FCM one that I removed.

I just tested this locally on a real device with FCM in my test project and confirmed the token being passed back this gets notifications 👍

[bibyzan]

(Will this remove the PR list saying I'm still pending a change?) guess not.

[bobkosse]

Is there some progress on this change request?

[bibyzan]

Is there some progress on this change request?

Not sure, I'm waiting to see if these changes are good but for some reason it still says in the PR list that it's waiting more changes from me 😒 I think its an issue with GitHub when you're not a member of the repo you can't change waiting changes status when it really needs review

[REPTILEHAUS]

+1 for this

[Karrlllis]

+1, pls approve this CR

[josh-m-sharpe]

Possibly unpopular opinion: Support for Firebase-related things directly in capacitor is out of scope.

I see that your changes here are only detecting very minor differences in the notification payload (or something like that), and not including pods, or firebase codez in capacitor... but it also includes a recommendation for how to use firebase in capacitor and IMO that functionality is best left to a plugin, which can better maintain changes across versions of capacitor and firebase. Capacitor's plugin infrastructure also makes the necessary changes to app-level code simpler. E.g. capacitor-fcm app-level code changes is basically zero.

That all said, the current capacitor guide that describe firebase push notifications definitely needs updating since it flat out doesn't work without capacitor-fcm.

[bibyzan]

but it also includes a recommendation for how to use firebase in capacitor and IMO that functionality is best left to a plugin

@josh-m-sharpe it doesn't seem like you've fully reviewed the changes I made here. This PR enables the use of any 3rd party push service to pass to the users JS PushNotifications registration event as a string. The only recommendation I made was by proposing a fix to the Firebase Push notifications tutorial which gives the user all they need to use FCM in that context and not have to manage an entire plugin just for one piece of functionality and resolves the confusion with that guide.

My main thing here is trying to minimize additional bloat in people's codebase's with plugins that barely do anything. No issues with capacitor-fcm, but it really isn't another plugin I personally want to manage over time just for a single method that I can put in my own AppDelegate.swift in less than 10 lines. If people want to install it for its other uses like subscribing to topics that's fine but I don't think a lot of people use those, including me. Capacitor will still work the same way if this gets merged, and APNS will be the default. But now people will have the option to make their token callbacks agree if they want to use a service like FCM and not have divergent platform token logic

[josh-m-sharpe]

I guess my point is capacitor is responsible for managing core iOS things. In this case, APNs, and that it does well. It is not responsible for providing 3rd-party functionality. As of this writing, none of the core plugins are 3rd party related. This change breaks that precedent.

I suppose I also disagree with you about wanting to manage a 3rd party plugin that "barely does anything". On the contrary, capacitor-fcm implementation is near zero whereas your approach requires that I now maintain 10 lines of code in my AppDelegate. I'm now responsible for staying on top of changes to capacitor's push notification API as well as changes to FCM. I don't want to do that. That's what plugins are for. It's much easier to update the plugin than dork around in swift-land.

[bibyzan]

As of this writing, none of the core plugins are 3rd party related. This changes breaks that precedent.

How does this break that precedent? My proposal doesn't add anything 3rd party related to PushNotifications.swift but giving the option to change your projects default behavior.

I'm now responsible for staying on top of changes to capacitor's push notification API as well as changes to FCM. I don't want to do that. That's what plugins are for. It's much easier to update the plugin than dork around in swift-land.

You do have a point there on why you would want to use a plugin, but in cross-platform-land if you're really trying to avoid thinking about swift-land then your gonna have a bad time debugging iOS!

Perhaps the guide could be changed to give the option of using the plugin to pass the token to the same callback, or just paste the code for the token if you don't want the plugin.

[jcesarmobile]

Verified that FCM token is received if following the new steps on the guide, and if nothing is changed, the APNs token is received keeps working as before, so merging.
Thanks for the PR!

[Unkn0wn0x]

I would like to see some more documentation on this, especially in the HowTo for sending / receiving Push Notifications via FCM as some important parts are missing.

https://capacitor.ionicframework.com/docs/guides/push-notifications-firebase#add-initialization-code

There a few things missing:

Pod Firebase/Messaging in the Podfile. Full Podfile could look like:

platform :ios, '12.0'
use_frameworks!

# workaround to avoid Xcode caching of Pods that requires
# Product -> Clean Build Folder after new Cordova plugins installed
# Requires CocoaPods 1.6 or newer
install! 'cocoapods', :disable_input_output_paths => true

def capacitor_pods
  # Automatic Capacitor Pod dependencies, do not delete
  pod 'Capacitor', :path => '../../node_modules/@capacitor/ios'
  pod 'CapacitorCordova', :path => '../../node_modules/@capacitor/ios'
  pod 'CordovaPlugins', :path => '../capacitor-cordova-ios-plugins'
  # Do not delete
end

def my_custom_pods
  pod 'Firebase/Messaging'
end

target 'App' do
  capacitor_pods
  my_custom_pods
end

And running the following commands to ensure, that the dependency is installed, up to date and available:

1. cd ios/app/
2. pod repo update
3. sudo gem install cocoapods
4. sudo gem install cocoapods-dependencies
5. pod dependencies
6. pod update
7. pod install

---

Import of FirebaseMessaging and FirebaseInstanceID into AppDelegate.swift and additional code to handle it the right way, full AppDelegate.swift could look like:

import UIKit
import Capacitor
import Firebase
import FirebaseMessaging
import FirebaseInstanceID

@UIApplicationMain
class AppDelegate: UIResponder, UIApplicationDelegate {

  var window: UIWindow?

  func application(_ application: UIApplication, didFinishLaunchingWithOptions launchOptions: [UIApplication.LaunchOptionsKey: Any]?) -> Bool {
    // Override point for customization after application launch.
    FirebaseApp.configure()
    return true
  }

  func applicationWillResignActive(_ application: UIApplication) {
    // Sent when the application is about to move from active to inactive state. This can occur for certain types of temporary interruptions (such as an incoming phone call or SMS message) or when the user quits the application and it begins the transition to the background state.
    // Use this method to pause ongoing tasks, disable timers, and invalidate graphics rendering callbacks. Games should use this method to pause the game.
  }

  func applicationDidEnterBackground(_ application: UIApplication) {
    // Use this method to release shared resources, save user data, invalidate timers, and store enough application state information to restore your application to its current state in case it is terminated later.
    // If your application supports background execution, this method is called instead of applicationWillTerminate: when the user quits.
  }

  func applicationWillEnterForeground(_ application: UIApplication) {
    // Called as part of the transition from the background to the active state; here you can undo many of the changes made on entering the background.
  }

  func applicationDidBecomeActive(_ application: UIApplication) {
    // Restart any tasks that were paused (or not yet started) while the application was inactive. If the application was previously in the background, optionally refresh the user interface.
  }

  func applicationWillTerminate(_ application: UIApplication) {
    // Called when the application is about to terminate. Save data if appropriate. See also applicationDidEnterBackground:.
  }

  func application(_ app: UIApplication, open url: URL, options: [UIApplication.OpenURLOptionsKey : Any] = [:]) -> Bool {
    // Called when the app was launched with a url. Feel free to add additional processing here,
    // but if you want the App API to support tracking app url opens, make sure to keep this call
    return CAPBridge.handleOpenUrl(url, options)
  }

  func application(_ application: UIApplication, continue userActivity: NSUserActivity, restorationHandler: @escaping ([UIUserActivityRestoring]?) -> Void) -> Bool {
    // Called when the app was launched with an activity, including Universal Links.
    // Feel free to add additional processing here, but if you want the App API to support
    // tracking app url opens, make sure to keep this call
    return CAPBridge.handleContinueActivity(userActivity, restorationHandler)
  }

  override func touchesBegan(_ touches: Set<UITouch>, with event: UIEvent?) {
    super.touchesBegan(touches, with: event)

    let statusBarRect = UIApplication.shared.statusBarFrame
    guard let touchPoint = event?.allTouches?.first?.location(in: self.window) else { return }

    if statusBarRect.contains(touchPoint) {
      NotificationCenter.default.post(CAPBridge.statusBarTappedNotification)
    }
  }

  #if USE_PUSH

  func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data) {
    Messaging.messaging().apnsToken = deviceToken
    InstanceID.instanceID().instanceID { (result, error) in
      if let error = error {
          NotificationCenter.default.post(name: Notification.Name(CAPNotifications.DidFailToRegisterForRemoteNotificationsWithError.name()), object: error)
      } else if let result = result {
          NotificationCenter.default.post(name: Notification.Name(CAPNotifications.DidRegisterForRemoteNotificationsWithDeviceToken.name()), object: result.token)
      }
    }
  }

  #endif

}

Now Xcode should be satisfied (no errors or warnings for the AppDelegate.swift, the build should go through and PushNotifications.addListener('registration', ...) should return a FCM token (instead of an APNS token) when calling PushNotifications.register().

Hope it helps.

Cheers
Unkn0wn0x

[enzoyu5488]

InstancceID is deprecated. I used

[rolandorojas]

It seems that a few of the methods referenced in the AppDelegate.swift section of this guide are deprecated.

This is how I got it working in the didRegisterForRemoteNotificationsWithDeviceToken part:

  func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data) {
    Messaging.messaging().apnsToken = deviceToken;
    
    NotificationCenter.default.post(name: Notification.Name(CAPNotifications.DidRegisterForRemoteNotificationsWithDeviceToken.name()), object: Messaging.messaging().fcmToken)
  }