Skip to content

Conversation

MonchiLin
Copy link
Contributor

@MonchiLin MonchiLin commented Sep 21, 2023

Description

This PR adds support for Firebase Messaging in Expo projects via a config plugin. This is mainly achieved by setting the appropriate keys (com.google.firebase.messaging.default_notification_icon and com.google.firebase.messaging.default_notification_color) in the AndroidManifest of the Android project during the build stage. The addition of this plugin will automatically enable Firebase Messaging support for developers using @react-native-firebase/messaging in their Expo projects which have the necessary notification configuration.

Displaying notifications correctly on Android requires setting a pure white icon. By default, the system uses the app's own icon as the notification icon, which usually isn't purely white. Therefore, it's necessary to specify a notification icon. If no notification configuration is detected from the Expo configuration, a friendly warning will be printed.

Related issues

Release Summary

Added support for Firebase Messaging in Expo projects via a config plugin.

Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
    • Yes
  • My change supports the following platforms;
    • Android
    • iOS
  • My change includes tests;
    • e2e tests added or updated in packages/\*\*/e2e
    • jest tests added or updated in packages/\*\*/__tests__
  • I have updated TypeScript types that are affected by my change.
  • This is a breaking change;
    • Yes
    • No

Test Plan

I created the repo to test the feature.

https://github.com/MonchiLin/firebase-messaging-test-app

@vercel
Copy link

vercel bot commented Sep 21, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-firebase ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 21, 2023 11:19am
react-native-firebase-next 🛑 Canceled (Inspect) Sep 21, 2023 11:19am

@CLAassistant
Copy link

CLAassistant commented Sep 21, 2023

CLA assistant check
All committers have signed the CLA.

@MonchiLin MonchiLin changed the title feat: Adding support for Firebase Messaging via Expo config plugin. feat(app/messaging): Adding support for Firebase Messaging via Expo config plugin. Sep 21, 2023
@MonchiLin MonchiLin changed the title feat(app/messaging): Adding support for Firebase Messaging via Expo config plugin. feat: Adding support for Firebase Messaging via Expo config plugin. Sep 21, 2023
Copy link
Collaborator

@mikehardy mikehardy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for posting this!

A couple questions - is it possible to go with a no-code solution wherein the standard Expo plugins for adding elements to AndroidManifest.xml are used? I assume those exist? Code here (especially Expo-related code which evolves rapidly) adds a non-trivial maintenance burden so I'd prefer a docs solution vs a code solution

If not, then 2) shouldn't this be in the messaging area? (as opposed to app)

@MonchiLin
Copy link
Contributor Author

MonchiLin commented Sep 23, 2023

Thanks for posting this!

A couple questions - is it possible to go with a no-code solution wherein the standard Expo plugins for adding elements to AndroidManifest.xml are used? I assume those exist? Code here (especially Expo-related code which evolves rapidly) adds a non-trivial maintenance burden so I'd prefer a docs solution vs a code solution

If not, then 2) shouldn't this be in the messaging area? (as opposed to app)

I. To my knowledge, if you do not use Expo bare, you can only modify the AndroidManifest.xml through Expo plugins.

I've checked BuildProperties, and it does not provide a way to modify AndroidManifest.xml.

II. You've made a good point, and it's something I've considered. There are two reasons why I didn't place this part of the code in the Messaging module:

  1. I noticed that the Getting Started section of the documentation is the only part that mentions how to use it with Expo. I think if I add it to the Messaging module, developers who don't notice the documentation on how Messaging works with Expo might miss this information. (Yes, I would also add a PR for Docs if the code could be merged).

  2. The current way to use Expo plugins is as follows:

{
  "expo": {
    "android": {
      "googleServicesFile": "./google-services.json"
    },
    "ios": {
      "googleServicesFile": "./GoogleService-Info.plist"
    },
    "plugins": [
      "@react-native-firebase/app",
      "@react-native-firebase/perf",
      "@react-native-firebase/crashlytics"
    ]
  }
}

If it were added to the Messaging module, it would become:

{
  "expo": {
    "android": {
      "googleServicesFile": "./google-services.json"
    },
    "ios": {
      "googleServicesFile": "./GoogleService-Info.plist"
    },
    "plugins": [
      "@react-native-firebase/app",
      "@react-native-firebase/perf",
      "@react-native-firebase/crashlytics",
      "@react-native-firebase/messaging"
    ]
  }
}

I think this might increase the cognitive load somewhat, but I'm not sure if what I'm doing is correct. Since you've raised doubts, I think what I did may not be intuitive. Now I have moved the code to the Messaging module.

Following your advice, I've noticed that continuing to commit in this PR would pollute the commits, so I've created a new PR. [https://github.com//pull/7369]

Have a good day.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants