[messaging] ReactNativeFirebaseMessagingReceiver vs FirebaseMessagingService

[sfuqua]

In poking around at how this package works while cross-referencing with Google Firebase SDK documentation, I was surprised to see that ReactNativeFirebaseMessagingService.onMessageReceived is not implemented, deferring to ReactNativeFirebaseMessagingReceiver - a BroadcastReceiver handling Intents with the com.google.android.c2dm.intent.RECEIVE action.

Specifically, that action seems to no longer be publicly documented as far as I can tell and seems to be (from Google's perspective?) an internal implementation detail of the Firebase SDK at this point.

This led me to wonder if there are any long term supportability concerns about leveraging this Intent or if there's some sort of understanding with the Firebase team w.r.t. this package? I may be reading too much into not being able to find public Google docs on it anymore, but I found it interesting that react-native-firebase doesn't use onMessageReceived which is the official "blessed" API for handling RemoteMessages, despite its documented shortcomings around apps running in the background.

[mikehardy]

Probably historic! react-native-firebase has been around quite some time at this point, the majority of firebase's life itself I think, but we are a bit behind in ingesting upstream change at the moment, especially with respect to structural items (crashlytics init style, iOS init style), this - if it were true I have not investigated sorry - would fit that general theme of "functionality still working / structure may be a bit deprecated" so I would not be too surprised.

Without implying it is an obligation, we'd love to have a PR that switched us to a more blessed/current/documented API for this area though given this one of the most important native integration points, and thus one of the most important reasons someone would choose to use these modules vs firebase-js-sdk special care for the current installed base to make sure it's compatible is important

Otherwise - in the absence of a PR - since you've just scoured the docs, logging an enhancement issue with points to the current documentation and affected code area makes it a much more digestible work ticket for anyone in the future to triage / verify it's a thing, then perhaps tackle

Either way, considering this as a sort of code review / ecosystem comparison, I definitely appreciate the review - both these ecosystems move pretty quickly and results in code rot quicker than I'd personally like, and more than I can handle myself all the time.

[sfuqua]

The complication in all of this is I don't think it's a straightforward swap over to onMessageReceived due to the nuance in how the Android Firebase SDK does (or doesn't) call that method. Lots of discussion in the ecosystem around how the FirebaseMessagingReceiver only calls back into app code for "data only" notifications or when the app is running in the foreground, and the current rn-firebase implementation using a BroadcastReceiver and the c2dm.intent.RECEIVE action seems to work around that problem.

I think this SDK issue is tangentially related to the discussion: firebase/firebase-android-sdk#2639

As-is, moving rn-firebase to use onMessageReceived may technically be a breaking change since I believe the current BroadcastReceiver is invoked in all scenarios (the same way the SDK itself is), whereas onMessageReceived gets skipped over for notification payloads while the app is backgrounded, for better or worse.

I don't think I have a recommendation for this package yet - I consider this sort of a weird design choice of the Firebase SDK, personally, and I'm crossing my fingers to try and get some traction on the linked issue as it seems to have been causing the community grief for a few years. I'm planning to poke at this a bit more next week and figure out whether with recent versions of the SDK it's possible to override FirebaseMessagingService.handleIntent - it seems like that would work but is a gray area in terms of supported API surface.