Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multi-App Setup seems to fall back to the default app regardless of app entry point. #848

Closed
Ayiga opened this issue Feb 23, 2018 · 1 comment

Comments

@Ayiga
Copy link

Ayiga commented Feb 23, 2018

Issue

Environment

  1. Application Target Platform: Both
  1. Development Operating System: macOS High Sierra 10.13.3 (17D47)
  1. Build Tools: N/A
  1. React Native version: 0.53.3
  1. RNFirebase Version: 3.2.5
  1. Firebase Module: auth (likely affects all modules)

Greetings,

I'm attempting to use multiple Apps using the initializeApp method as outlined in the documentation on rnfirebase.io, but I'm getting stuck on Authentication. It seems as though, regardless of which App I am using it still falls back to the [DEFAULT] app.

screen shot 2018-02-23 at 4 26 08 pm

screen shot 2018-02-23 at 4 20 52 pm

screen shot 2018-02-23 at 4 24 45 pm

I've looked through the source to try and pin-point why this is occurring, and I believe the issue is caused by attempting to index the APP_MODULES by the App object itself. Since Javascript object's are only able to have string or Symbols as their keys, this forces the App to be converted to a string. With no implementation of the toString method this falls back to the default Object toString method, of '[object Object]'. Because of this, all created App Modules will be referencing the same app.

screen shot 2018-02-23 at 4 22 09 pm

The easiest way to detect this is to create a project with react-native-firebase, have a native app setup, and sign in to it. Now create a new app using initializeApp, and then call the onAuthStateChanged. You should get a result that returns the user for the '[DEFAULT_APP]' instead of null or a user for the second app.

It seems that the simplest solution to this issue is to add a toString method to the App class so that it returns the app's name.

  /**
   * toString returns the name of the app.
   *
   * @return {string}
   */
  toString() {
    return this._name;
  }

This is what I've done currently to fix the issue. Another possible solution would be to switch out the Javascript Object Literal for a Map. Although that may cause issues with the javascript Environment, and could require the runtime to have a polyfill. I'm not sure if it has this polyfill by default.

const APP_MODULES: Map<App, FirebaseModule> = new Map();

The last option that would work would be to modify the Javascript Object Literal so that it uses a string as a key, and modify the modules so that they use the app's name.
Lines to Modify 1

Lines to Modify 1

Lines to Modify 2

  /**
   *
   * @param statics
   * @param InstanceClass
   * @return {function()}
   * @private
   */
  appModule<M: FirebaseModule>(
    app: App,
    namespace: FirebaseNamespace,
    InstanceClass: Class<M>
  ): () => FirebaseModule {
    return (): M => {
      if (!APP_MODULES[app.name]) {
        APP_MODULES[app.name] = {};
      }

      if (
        isAndroid &&
        namespace !== 'utils' &&
        !INTERNALS.FLAGS.checkedPlayServices
      ) {
        INTERNALS.FLAGS.checkedPlayServices = true;
        app.utils().checkPlayServicesAvailability();
      }

      if (!APP_MODULES[app.name][namespace]) {
        APP_MODULES[app.name][namespace] = new InstanceClass(app, app.options);
      }

      return APP_MODULES[app.name][namespace];
    };
  },

There may be more ways to address this issue as well, but these seem like fairly simple changes.

As I said, I used the first method, but it's all personal preference. I can open a Pull Request with the method that best suits your style.

Please let me know,

Thanks,

~ Ayiga

@Salakar
Copy link
Member

Salakar commented Feb 24, 2018

Thanks for this well written issue report 💯

Have just pushed a fix for this, went with the toString() approach - thanks for all the suggested fixes.

Will publish a patch version to npm shortly.

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

No branches or pull requests

2 participants