React-redux-firebase: feat(core): remove extending of firebase

[robwafle]

Good Morning,

I believe I am reporting a bug, but I'm not sure if this is a bug with react-redux-firebase v3.0.x or react-native-firebase v6, or perhaps I'm just doing this wrong.

Bug:
Attempted to assign to a Readonly Property in ReactReduxFirebaseProvider -> createFirebaseInstance.js line 1, char 2483.

Bug Description:
I'm following the example here: https://react-redux-firebase.com/docs/integrations/react-native.html But I replaced the v5 react-native-firebase components with v6 react-native-firebase components,

When I do this, I get an error here. I believe firebase._ may be the readonly property?
firebase._ = merge(defaultInternals, firebase._) // eslint-disable-line no-param-reassign

To Reproduce:
Please download this git repo I created using the starter instructions here:
https://invertase.io/oss/react-native-firebase/quick-start/new-project

And, then I tried to follow the previously mentioned example above.
https://github.com/robwafle/test-rnfb6-rrf

Expected Behavior:
I am hoping that I will be able to use both components together.

I am currently using these dependencies:
"@react-native-firebase/app": "6.0.1",
"@react-native-firebase/firestore": "^6.0.1",
"react": "16.8.6",
"react-native": "0.60.5",
"react-redux": "^7.1.1",
"react-redux-firebase": "^3.0.2",
"redux": "^4.0.4",
"redux-firestore": "^0.9.0",
"redux-logger": "^3.0.6",
"redux-starter-kit": "^0.7.0",
"remote-redux-devtools": "^0.5.16"

I was able to get this to work with v5 of react-native-firebase and v2 of react-redux-firebase but my code only ran and built for a few hours, then I started getting some error about chrono and there was this "ugly" workaround where I had to remove the rnfirebase components from my pod, run pod install, and add the components back in, and then run pod install again and then do NOT clean my build folder unless I want to start over..

Here it is:
https://github.com/invertase/react-native-firebase/issues/1610

Lastly, I tried the instructions at the following link to see if I could "hack my way around it" and/or create my first contribution to open source, but it didn't work for me. I found a stack overflow article saying "The npm link command doesn't work because React Native packager doesn't support symlinks." (https://stackoverflow.com/questions/44061155/react-native-npm-link-local-dependency-unable-to-resolve-module) and the url to the instructions is: https://github.com/prescottprue/react-redux-firebase/blob/master/docs/contributing.md

Am I missing something? (I'm pretty sure I'm missing a FEW things at this moment) But, since this the name of this repository starts with "react" I was hopeful that the instructions would work.

I look forward to any help and/or direction to get this resolved.

Thx!

Rob

[prescottprue]

Believe this is a duplicate of this issue on redux-firestore. It is due to the newer version of react-native-firebase not allowing for object extension like older versions and the firebase SDK do. It seems like it would be a pretty big change to handle this, also not exactly sure why they did it when the Firebase SDK doesn’t have this limititation

[Ehesp]

I'll ping @Salakar on this one as he re-wrote most of the internals. We do have an extendApp function: https://github.com/invertase/react-native-firebase/blob/master/packages/app/lib/FirebaseApp.js#L63

I'll get Mike to take a look anyhow.

[prescottprue]

@Ehesp thanks! It is awesome to know there is an extendApp since that may be a good fallback.

[ricmatsui]

Having the same issue. For what it's worth, I made a custom build of react-redux-firebase v2 with extendApp instead of defineProperty/assign and early testing looks fine for now:

https://github.com/ricmatsui/react-redux-firebase/commit/0b2edcd66f782ca0092529173bd175cc3c3529b8 and then creating the Redux store with the default app

reactReduxFirebase(firebase.app(), { enableRedirectHandling: false })

Not sure if this would be a breaking change, but it might be good to consider not extending firebase objects and instead using a secondary data structure, for example a shared WeakMap with firebase objects as keys:

const firebaseInternals = new WeakMap();
...
const enhancedFirebase = // construct a proxy-like object with default internals + helpers
firebaseInternals.set(firebase, enhancedFirebase);

but I'm not as sure about the changes to the API for example withFirebase where components are given an enhanced firebase object. (I only use firebaseConnect right now)

[prescottprue]

@ricmatsui Thanks for reporting! I like the idea of a separate object/map to store the methods, and actually think that is the way things will go (since I wasn't sure that extendApp would work as expected). The reason the standalone object/map wasn't done originally it that there are other unmodified Firebase statics/methods copied over as well. One breaking change for this switch would be to have those non-extended methods and statics pulled directly from the Firebase SDK instead of from the extended version.

With all of that in mind, I am open to a PR if we want to start the process

[bcgilliom]

BTW - I give a 👍 to move away from extending the fb object, but extend app seems to work as a stopgap 🤷‍♂

[illuminist]

It also would be great for reducing confusion between normal firebase instance and extended instance.

Few more things to consider is:

• The extended instance should be able to initialize outside of react render tree, so the library should act like 'redux-firebase' instead of 'react-redux-firebase', same as 'redux-firestore'. It's currently can be done with calling createFirebaseInstance and vice versa.
• As react-native-firebase has different way to access firebase features, ie. has to directly import the feature instance from respective package, could it be better if a user provide each feature instance separately instance of internally accessing via firebase[featureName]()? And with a new way to initialize redux firebase instance

for example

/** function initialize call createFirestoreInstance but will also manage an instance,
  * to be access via getFirestore function, while createFirestoreInstance will
  * only create an instance. Should be called around firebase.initializeApp call
  */
reduxFirestore.initialize(firebase.firestore(), { /* config */ })

/** same internal as reduxFirestore.initialize but with more features to input
  */ 
reactReduxFirebase.initialize({
  auth: firebase.auth(),
  database: firebase.database(),
  firestore: reduxFirestore.getFirestore(), // just for rrf internal use such as profile from firestore
}, { /* config */ })

[Salakar]

Heads up that v6.3.0 of RNFirebase is out which should now be working with react-redux-firebase - please try it out and report back.

[prescottprue]

@Salakar Thanks for the heads up!

[prescottprue]

As mentioned by @Salakar, the v6.3.0 version of react-native-firebase should fix this issue. For all looking for it, this is the commit that removes Object.freeze.

I am going to close this since updating should make this no longer an issue. If anyone updates to v6.3.0 of react-native-firebase and this is still and issue, please reach out.