feat: Added Expo config plugin #5480

barthap · 2021-07-05T11:02:07Z

Description

Expo released development clients and rolled out an interface called config plugins. These allow users to add native modules which aren't included in Expo Go.

This PR adds a config plugin, which allows users for painless setup of RNFB, without touching any native code. Until now, I've been maintaining the plugin as a separate npm package: with-rn-firebase and had some positive feedback. Now it's time to integrate it into the upstream library 😄 For more context, see the discussion at #5386.

Basically I've copied my code from https://github.com/barthap/with-rn-firebase, but I've split it into separate plugins for each affected package to avoid additional config like setting enableCrashlytics etc.

RNFB repo related changes:

Added build:plugin and lint:plugin npm scripts for affected packages
The build:plugin is run during npm prepare phase
Added plugin/__tests__ directories with some basic tests, they work when running npm run tests:jest
I had to modify global tsconfig.json to add esModuleInterop: true - otherwise the plugin tests failed. I haven't seen any regressions and hope it's not breaking - please correct me if it is.
Added dependency @expo/config-plugins and devDependency with plugin tsconfig base.
Added packages/**/plugin/build dir to eslint and prettier ignore - it's a generated directory
Added some values to sdkVersions section in app/package.json

For the plugin to work, the package should have a app.plugin.js file in its root folder - in this case it only delegates to the plugin/build directory. The build directory is gitignored and generated during npm build:plugin from TypeScript files inside plugin/src.

The plugins

As for now, to reflect current with-rn-firebase functionality, I've added plugins for 3 packages:

App:
Its purpose is to copy google-services.json and GoogleServices-Info.plist into correct places, modify AppDelegate.m, and add Gradle dependency and plugin - basically the installation steps from the homepage.

The Google Services files by default are copied from the paths specified by expo.ios|android.googleServicesFile in app.json.
Crashlytics:
Installs the required Gradle dependencies for Android. It doesn't yet support the optional release NDK installation step.
Performance Monitoring:
Installs the required Gradle dependency for Android.

Usage / How it works

When using Expo, replace the yarn add/npm install with expo install:

expo install @react-native-firebase/app
expo install @react-native-firebase/crashlytics

Then in the app.json the plugins section should be updated automatically. When not using expo install, you have to add the plugins on your own`.

Remember to configure the expo.android.googleServicesFile and expo.ios.googleServicesFile in app.json. The paths are relative to the project root.

The app.json file may look like this (non-related parts omitted):

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

Further steps

After this PR is merged:

Update the docs Expo section and possibly the installation steps for the packages: app, perf, crashlytics

These were not included in my original plugin, they're not implemented yet:

Investigate the need of config plugins for the FCM module (see the discussion in Missing notification icon config in AndroidManifest.xml barthap/with-rn-firebase#6)
Add option to install NDK Crashlytics in release build
Investigate the need of adding plugin for other packages. Afaik there are some configuration options requiring the Podfile changes, which are not trivial for config plugins.

Related issues

Discussion #5386

Release Summary

app, crashlytics, perf: added Expo config plugins

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__ (added tests for config plugins)
I have updated TypeScript types that are affected by my change.
This is a breaking change;
- Yes
- No

Test Plan

Steps that anyone should be able to reproduce:

Installed npm RNFB packages to a separate directory and applied my plugin changes - I've attached the packages to this post:
patched-packages.zip
In another directory expo init app_name_here - selected managed template

You can use npx crna -t with-dev-client instead to try it out with Expo development client
Run expo prebuild on this project
git commit your changes
yarn link all 3 RNFB packages into Expo app project
Add plugins section to app.json. If you have Expo Tools VSCode extension, the intellisense should propose them automatically
Copy or create blank google-services.json and GoogleService-Info.plist (e.g. in app root dir) and specify the expo.android.googleServicesFile and expo.ios.googleServicesFile fields in app.json.
git commit again to have a checkpoint to easy reset
Run expo prebuild and observe the git diff. Experiment with different combinations of plugins in app.json

You can build and run your app with expo run:ios / expo run:android

vercel · 2021-07-05T11:02:12Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

react-native-firebase – ./

🔍 Inspect: https://vercel.com/invertase/react-native-firebase/4sgzUQNvJ7TLSq9fyd56kL3NkRFR
✅ Preview: https://react-native-firebase-git-fork-barthap-expo-co-ae921b-invertase.vercel.app

react-native-firebase-next – ./website_modular

🔍 Inspect: https://vercel.com/invertase/react-native-firebase-next/2MZ4wNHK3hbYGd58CvHYJRZv9dXb
✅ Preview: Failed

[Deployment for 964c489 failed]

CLAassistant · 2021-07-05T11:02:14Z

All committers have signed the CLA.

GertSallaerts · 2021-07-06T09:43:23Z

From the best practices in Expo's config plugins documentation:

Leverage built-in plugins: Account for built-in plugins from the prebuild config. Some features are included for historical reasons, like the ability to automatically copy and link Google services files defined in the Expo config. If there is overlap, then maybe recommend the user uses the built-in types to keep your plugin as simple as possible.

Wouldn't it then make more sense to direct users to fill in the config.android.googleServicesFile and config.ios.googleServicesFile entries in their app.json?

Or if you really want to provide defaults do something like this in your plugin to still leverage the existing functionality somewhat:

const DEFAULT_ANDROID_GOOGLE_SERVICES_PATH = './google-services.json';
const DEFAULT_IOS_GOOGLE_SERVICES_PATH = './GoogleService-Info.plist';

const withRnFirebase: ConfigPlugin<PluginProps> = (config) => {

  config.android = config.android || {};
  config.android.googleServicesFile =
    config.android?.googleServicesFile ||
    DEFAULT_ANDROID_GOOGLE_SERVICES_PATH;

  config.ios = config.ios || {};
  config.ios.googleServicesFile =
    config.ios?.googleServicesFile ||
    DEFAULT_IOS_GOOGLE_SERVICES_PATH;

  return withPlugins(config, [
    // iOS
    withFirebaseAppDelegate,
  
    // Android
    withBuildscriptDependency,
    withApplyGoogleServicesPlugin,
  ]);
};

mikehardy · 2021-07-06T13:56:02Z

Sorry I haven't had time to review yet - however! I built some infrastructure in the past which may help - if you follow the details link on the patches github actions check, you'll find a "patches.zip" artifact that is a full set of patch-package compatible files from last release to current main branch + this PR, so anyone can test it easily. I don't use Expo so I would love to see reports of success from anyone that tries it

I'll do a quick scan to make sure you're not a rogue bitcoin miner abusing Github free services (I'm joking) then approve the CI run so it generates the patches

codecov · 2021-07-06T14:00:06Z

Codecov Report

Merging #5480 (4e83a62) into master (ee4b760) will decrease coverage by 3.94%.
The diff coverage is 59.82%.

❗ Current head 4e83a62 differs from pull request most recent head 964c489. Consider uploading reports for the commit 964c489 to get more accurate results

@@             Coverage Diff              @@
##             master    #5480      +/-   ##
============================================
- Coverage     71.46%   67.53%   -3.93%     
- Complexity        0      980     +980     
============================================
  Files            96      199     +103     
  Lines          4292     9639    +5347     
  Branches        923     1504     +581     
============================================
+ Hits           3067     6509    +3442     
- Misses         1128     2717    +1589     
- Partials         97      413     +316

barthap · 2021-07-08T07:00:12Z

@mikehardy Thanks, it helped a bit :) In the Test Plan section I prepared an instruction for anyone who wants to test it and prepared ready-to-go patched packages. Regarding other CI tests - i had to add plugin/build directories to eslint ignore, that test should pass now. Other tests passed.

@GertSallaerts yes I considered that. Honestly, most functionality of the @rnfb/app plugin is also built in expo-cli, and partially in expo-firebase-core. And both approaches have some pros and cons. I decided to duplicate the functionality, because that makes RNFB more flexible and independent from potential expo-cli changes. Of course, I don't mind relying on the built-in plugin, but in that case the app plugin has little purpose to exist. cc @EvanBacon what do you think?

Edit: Ok, it is clear now (see Evan's review below). We should rely on the built-in Expo config fields (expo.ios.googleServicesFile and expo.android.googleServicesFile) and I made them mandatory - the plugin displays an error when they're not set.

And this is also the reason why the copying functionality should be implemented here - the expo-cli built-in one is treats these values as optional and just skips copying if they're absent.

packages/app/plugin/src/index.ts

packages/app/plugin/src/android/constants.ts

packages/app/plugin/src/ios/appDelegate.ts

packages/app/plugin/tsconfig.json

packages/crashlytics/plugin/tsconfig.json

EvanBacon · 2021-07-08T20:15:07Z

packages/app/plugin/src/index.ts

+   * Custom location of `google-services.json`,
+   * relative to project root
+   */
+  androidGoogleServicesPath?: string;


For now, maybe just rely on the Expo CLI built-in values since they could be used across Google Sign-in and other packages. This will reduce initial complexity in the config plugin as well.

mikehardy

That's a lot better for the review + fixup, thanks @EvanBacon @barthap

I want to be clear that while I am certainly Expo-curious, and I am really excited that RNFB will now be more easily integrated into Expo, I will need help with user support and fixups in the area.

All of my support is based on knowledge of react-native built from npx react-native init and going from there 😅 . So, I will really need help in this area with any documentation or user support you can provide when the inevitable user questions arise 🙏 🙏

That actually makes me a bit nervous, but "fear of more users with legitimate questions" is no reason to hold up a PR in my opinion, so I'll merge this and let's go!

barthap · 2021-07-22T15:39:55Z

I will need help with user support and fixups in the area.

In such cases, just ping me or somebody at Expo, we'll try to help 😄

Salakar · 2021-07-23T08:12:32Z

Thanks for this @barthap - would you also be able to update https://github.com/invertase/react-native-firebase/blob/master/docs/index.md#expo - it's quite out of date 😅

barthap · 2021-07-23T08:16:20Z

Yeah, that's my plan 😄 I'll open a PR probably on Monday

Salakar · 2021-07-23T08:20:49Z

Yeah, that's my plan 😄 I'll open a PR probably on Monday

Thanks!

Ehesp · 2021-07-28T09:05:26Z

Thanks for the help here @barthap @EvanBacon 👍

* Preparations & Added the `app` plugin * Add perf monitoring plugin * Add crashlytics plugin * eslint ignore `plugin/build` dirs * Add tests READMEs

richardwu · 2021-08-11T22:46:28Z

Thanks all for pushing this upstream.

For other packages like remote-config, it seems like there are no additional steps on the native side, so AFAIK a config plugin is unnecessary. To confirm: should this be as simple as yarn add @react-native-firebase/remote-config if I am using EAS Build?

richardwu · 2021-08-11T23:25:37Z

Thanks all for pushing this upstream.

For other packages like remote-config, it seems like there are no additional steps on the native side, so AFAIK a config plugin is unnecessary. To confirm: should this be as simple as yarn add @react-native-firebase/remote-config if I am using EAS Build?

Oops, I just saw the comment regarding packages without native code linking need not a plugin.

On a side note: is it necessary to include com.google.firebase:firebase-iid:17.0.2 in the config plugin for @react-native-firebase/app? I'm running into this issue when using expo-notifications and a commentor mentioned that including that dependency fixed the issue: https://forums.expo.dev/t/cant-get-expotoken-in-expo-notification-due-to-failed-resolution-of-lcom-google-firebase-iid-firebaseinstanceid/53265/4

barthap · 2021-08-12T06:10:27Z

I haven't dug into the issue itself, but if this dependency is needed for FCM / Notifications, the proper place for it would be the expo-notifications plugin or @react-native-firebase/fcm (plugin doesn't exist yet).

If you really need to include that dependency for notifications to work, the easiest workaround would be to write your own plugin (I replied in the forum thread).

mikehardy · 2021-08-12T14:44:33Z

Worth noting, iid is a dead module for firebase, it should not be included as a direct dependency anywhere with current versions, as far as I know

techpanga · 2021-10-04T03:29:25Z

Do we have config plugin for messaging?

barthap · 2021-10-04T05:55:57Z

No, but if you mean to customize notification icons, please take look at this discussion: barthap/with-rn-firebase#6 - you can "steal" expo-notifications plugin for that 😉

mikehardy · 2021-10-04T12:08:58Z

Unrelated, looks like the dependency (expo-plugin) has a semver major to adopt. Is there anything we need to do here? Will it transitively cause a semver major here? Hadn't considered that this dependency might cause us to do majors, hopefully avoidable but if not so be it, I have another breaking change in the wings if so

barthap · 2021-10-04T14:27:49Z

honestly I don't know, cc @EvanBacon

mikehardy · 2021-10-04T16:16:30Z

when I tried to look (it came out a couple weeks ago) I have to say as an "expo-plugins" consumer, I was completely unable to find any CHANGELOG or commit diff or anything on it (I'm not even sure where the code is? I couldn't find it in the repo?) so it wasn't a smooth-n-easy consumer situation. If the package.json for it could actually point to the real location of expo-plugins (vs the general expo/expo repo) and there was a real changelog, I could have just done the migration...

brentvatne · 2021-10-04T18:00:32Z

the changelog for config-plugins lives inside of the expo-cli repo (where config-plugins lives also): https://github.com/expo/expo-cli/blob/master/CHANGELOG.md

the major version bump was due to the "extend base mods" feature landing, which adds to the public api. we bumped the major version because this shifted around the internals in a meaningful way and we are not entirely sure on the extent to which developers may have been depending on undocumented apis.

mikehardy · 2021-10-04T18:40:05Z

Sweet! Thanks for the CHANGELOG pointer, that will help. Thanks also for the change insight, I don't think we were doing anything fancy at all in here, so this is likely safe for us to ingest then.

techpanga · 2021-10-05T02:56:59Z

No, but if you mean to customize notification icons, please take look at this discussion: barthap/with-rn-firebase#6 - you can "steal" expo-notifications plugin for that 😉

@barthap , I am looking for imageUrl in notifications...

https://firebase.google.com/docs/cloud-messaging/ios/send-image
https://firebase.google.com/docs/cloud-messaging/android/send-image

Can this plugin be extended to include the image in notifications. (not for the icon)

Preparations & Added the app plugin

605bf56

vercel bot deployed to Preview July 5, 2021 11:02 View deployment

App plugin fixes

79c402e

barthap changed the title ~~feat: Added Expo config plugin~~ [WIP] feat: Added Expo config plugin Jul 7, 2021

barthap added 3 commits July 7, 2021 08:09

Add perf monitoring plugin

c439bf1

Add crashlytics plugin

66393b2

eslint ignore plugin/build dirs

5ddc787

vercel bot deployed to Preview July 7, 2021 06:13 View deployment

Fix plugin entry filenames

2fb4020

vercel bot deployed to Preview July 7, 2021 06:59 View deployment

barthap commented Jul 8, 2021

View reviewed changes

packages/app/plugin/src/index.ts Show resolved Hide resolved

packages/app/plugin/src/android/constants.ts Outdated Show resolved Hide resolved

barthap changed the title ~~[WIP] feat: Added Expo config plugin~~ feat: Added Expo config plugin Jul 8, 2021

barthap marked this pull request as ready for review July 8, 2021 07:04

EvanBacon reviewed Jul 8, 2021

View reviewed changes

packages/app/plugin/src/ios/appDelegate.ts Show resolved Hide resolved

EvanBacon reviewed Jul 8, 2021

View reviewed changes

packages/app/plugin/tsconfig.json Outdated Show resolved Hide resolved

EvanBacon reviewed Jul 8, 2021

View reviewed changes

packages/crashlytics/plugin/tsconfig.json Outdated Show resolved Hide resolved

EvanBacon reviewed Jul 8, 2021

View reviewed changes

barthap added 2 commits July 9, 2021 07:35

Update tsconfig to Node 12

632e41c

Simplify Google-Services resolution

19947ab

vercel bot deployed to Preview July 9, 2021 06:33 View deployment

Move dep versions to app package.json

d788738

vercel bot deployed to Preview – react-native-firebase July 12, 2021 14:08 View deployment

vercel bot had a problem deploying to Preview – react-native-firebase-next July 12, 2021 14:08 Failure

Update warning headers

6e654c0

mikehardy approved these changes Jul 22, 2021

View reviewed changes

mikehardy added Workflow: Pending Merge Waiting on CI or similar and removed Workflow: Needs Review Pending feedback or review from a maintainer. labels Jul 22, 2021

mikehardy merged commit 832057c into invertase:master Jul 22, 2021

mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Jul 22, 2021

barthap deleted the expo-config-plugin branch July 23, 2021 15:45

barthap mentioned this pull request Jul 26, 2021

docs: Updated Expo installation steps #5548

Merged

8 tasks

mikehardy mentioned this pull request Aug 4, 2021

using plugin with Expo react native app transistorsoft/react-native-background-fetch#354

Closed

mikehardy mentioned this pull request Aug 21, 2021

Error with @react-native-firebase/auth and Expo: "does not contain a valid config plugin" #5629

Closed

mikehardy mentioned this pull request Oct 10, 2021

Would you like to collaborate on this feature in the main notifee repo? nicko170/with-notifee#1

Open

mikehardy mentioned this pull request Nov 19, 2021

[🐛] Expo standalone application crashes in the splash screen when you use the Firebase module #5869

Closed

This comment has been minimized.

Sign in to view

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Added Expo config plugin #5480

feat: Added Expo config plugin #5480

barthap commented Jul 5, 2021 •

edited

Loading

vercel bot commented Jul 5, 2021 •

edited

Loading

CLAassistant commented Jul 5, 2021 •

edited

Loading

GertSallaerts commented Jul 6, 2021 •

edited

Loading

mikehardy commented Jul 6, 2021

codecov bot commented Jul 6, 2021 •

edited

Loading

barthap commented Jul 8, 2021 •

edited

Loading

EvanBacon Jul 8, 2021

mikehardy left a comment

barthap commented Jul 22, 2021

Salakar commented Jul 23, 2021

barthap commented Jul 23, 2021

Salakar commented Jul 23, 2021

Ehesp commented Jul 28, 2021

richardwu commented Aug 11, 2021

richardwu commented Aug 11, 2021 •

edited

Loading

barthap commented Aug 12, 2021

mikehardy commented Aug 12, 2021

techpanga commented Oct 4, 2021

barthap commented Oct 4, 2021

mikehardy commented Oct 4, 2021

barthap commented Oct 4, 2021

mikehardy commented Oct 4, 2021

brentvatne commented Oct 4, 2021

mikehardy commented Oct 4, 2021

techpanga commented Oct 5, 2021

This comment has been minimized.

feat: Added Expo config plugin #5480

feat: Added Expo config plugin #5480

Conversation

barthap commented Jul 5, 2021 • edited Loading

Description

The plugins

Usage / How it works

Further steps

Related issues

Release Summary

Checklist

Test Plan

vercel bot commented Jul 5, 2021 • edited Loading

react-native-firebase – ./

react-native-firebase-next – ./website_modular

CLAassistant commented Jul 5, 2021 • edited Loading

GertSallaerts commented Jul 6, 2021 • edited Loading

mikehardy commented Jul 6, 2021

codecov bot commented Jul 6, 2021 • edited Loading

Codecov Report

barthap commented Jul 8, 2021 • edited Loading

EvanBacon Jul 8, 2021

Choose a reason for hiding this comment

mikehardy left a comment

Choose a reason for hiding this comment

barthap commented Jul 22, 2021

Salakar commented Jul 23, 2021

barthap commented Jul 23, 2021

Salakar commented Jul 23, 2021

Ehesp commented Jul 28, 2021

richardwu commented Aug 11, 2021

richardwu commented Aug 11, 2021 • edited Loading

barthap commented Aug 12, 2021

mikehardy commented Aug 12, 2021

techpanga commented Oct 4, 2021

barthap commented Oct 4, 2021

mikehardy commented Oct 4, 2021

barthap commented Oct 4, 2021

mikehardy commented Oct 4, 2021

brentvatne commented Oct 4, 2021

mikehardy commented Oct 4, 2021

techpanga commented Oct 5, 2021

This comment has been minimized.

barthap commented Jul 5, 2021 •

edited

Loading

vercel bot commented Jul 5, 2021 •

edited

Loading

CLAassistant commented Jul 5, 2021 •

edited

Loading

GertSallaerts commented Jul 6, 2021 •

edited

Loading

codecov bot commented Jul 6, 2021 •

edited

Loading

barthap commented Jul 8, 2021 •

edited

Loading

richardwu commented Aug 11, 2021 •

edited

Loading