fix(app): react-native 0.74 bridgeless mode compatibility

[gabrieldonadel]

Description

When trying to use @react-native-firebase/app with the new architecture and bridgeless mode on (react-native version 0.74.0-rc.3), the user is faced with an unhandled promise rejection

Very similar case to https://github.com/react-native-netinfo/react-native-netinfo/pull/717

---

This error happens because of the following code inside nativeModule.js

https://github.com/invertase/react-native-firebase/blob/b40d44c57e81a4110cbb4e6204c400cb8f78b00a/packages/app/lib/internal/registry/nativeModule.js#L62-L79

Currently we cannot copy functions directly from the React Native's NativeModules object, given that the module object is a host object.

After this change everything works as expected and the compatibility layer allows @react-native-firebase/app to be used with bridgeless mode on

Release Summary

Add support for react-native 0.74 bridgeless mode

Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • [ ] Yes
• My change supports the following platforms;
  • [x] Android
  • [x] 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
  • [x] No

Test Plan

Tested running on Android and iOS with:

• new arch, bridgeless off
• new arch, bridgeless on
• old arch

:fire:

[vercel[bot]]

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	May 20, 2024 0:47am

[CLAassistant]

All committers have signed the CLA.

[gabrieldonadel]

hey @mikehardy, can you take a look at this PR when you have some time?

[mikehardy]

Something not quite right with the wrapped methods after this change when running locally, investigating...

RNFBAppCheckModule.isTokenAutoRefreshEnabled was called with 2 arguments but expects 1 arguments. If you haven't changed this method yourself, this usually means that your versions of the native code and JavaScript code are out of sync. Updating both should make this error go away.

From experience, I know that the method wrapping is mostly to automatically inject the idea of which "firebase app" you are using on the native side (denoted by a string which we use to do lookups in a "firebase app name" --> "firebase app native object" map on native side

The extra argument is that native firebase app name. This is a problem in the opposite direction though, where it got 2 arguments (possibly a wrapped method...) when it expected one. Anyway, I've worked through similar issues I can figure this one out I'm certain, but there might be a little delay while I do so

[github-actions[bot]]

Hello 👋, this PR has been opened for more than 2 months with no activity on it.

If you think this is a mistake please comment and ping a maintainer to get this merged ASAP! Thanks for contributing!

You have 15 days until this gets closed automatically

[fobos531]

Not stale

[johnf]

I tried this patch as well. My app loads but I get the following error

 (NOBRIDGE) ERROR  Error getting token [TypeError: this.native.getToken is not a function (it is undefined)]

[mikehardy]

Yeah - this is just not ready yet and I have not been able to identify exactly why yet, which is extraordinarily frustrating as the change looks perfect.

In testing, it looks like the switch from making a new object vs spreading the native methods into it to using the native object directly results in our post-processing-wrap of the functions not working on the object it should be working on. Basically the wrapping fails. Why? I don't know yet

[skinnynpale]

+1

[harisbaig100]

I applied it as a patch but still still (No Firebase App '[DEFAULT]') error persists

[birdofpreyru]

Here is a that works for me on Android / New Arch / [email protected].

diff --git a/node_modules/@react-native-firebase/app/lib/internal/registry/nativeModule.js b/node_modules/@react-native-firebase/app/lib/internal/registry/nativeModule.js
index 03f001c..bd0a484 100644
--- a/node_modules/@react-native-firebase/app/lib/internal/registry/nativeModule.js
+++ b/node_modules/@react-native-firebase/app/lib/internal/registry/nativeModule.js
@@ -65,7 +65,8 @@ function nativeModuleWrapped(namespace, NativeModule, argToPrepend) {
     return NativeModule;
   }
 
-  const properties = Object.keys(NativeModule);
+  let properties = Object.keys(Object.getPrototypeOf(NativeModule));
+  if (!properties.length) properties = Object.keys(NativeModule);
 
   for (let i = 0, len = properties.length; i < len; i++) {
     const property = properties[i];

[mikehardy]

Hey @birdofpreyru that looks fantastic locally, it works on rn73 which indicates it is backwards-compatible where the previous patch wasn't.

Separately I've done a lot of work cleaning up the tests so that CI can go green (it's been red on iOS in CI for a while though it was passing locally)

With those changes rebased in here, the original patch reverted, and your patch added on, I think it might work

Can you confirm that the final diff state of this branch (just a -1 / +2) is what you have working in your testing of new arch on rn74?

[mikehardy]

Hmm 🤔 this works correctly for me when I apply it on top of #7774 and run it in old architecture mode

but on new architecture mode my local iOS simulator says it's loading the bundle from metro then it just hangs - I think it is a local problem though - does it work for everyone here on iOS + new arch bridgeless?

android new arch bridgeless works

[mikehardy]

going to merge since it appears to be backwards compatible at least, and seems to work - if there is still a problem we can fix-forward