angularfire icon indicating copy to clipboard operation
angularfire copied to clipboard

provideFirebaseApp(fn) should allow fn to take an injector as an argument

Open majido opened this issue 3 years ago • 4 comments

I think the function argument that provideFirebaseApp() accepts should have the following signature fn: (injector: Injector) => IFirebaseApp instead of fn: () => IFirebaseApp

Additional Context

The function argument that provideFirebaseApp() accepts, is expected to not take any arguments [1] but in practice when it is invoked it is passed an injector [2].

Passing an injector makes sense since our firebase initialization logic that needs to run inside that function may have some dependencies (like options in our usecase explained below). I suggest the signature for provideFirebaseApp() to change into

function provideFirebaseApp(fn: (injector?: Injector) => IFirebaseApp, ...deps: any[]): ModuleWithProviders<FirebaseAppModule>

It seems that the original authors were aware of this usecase because:

  1. The actually passes the injector instance when invoking the provided function.
  2. provideFirebaseApp actually accepts a list of dependencies deps and adds them as the module dependencies.

[1] https://github.com/angular/angularfire/blob/d724d81eafe003d2ec18728998cec7682aaaaeec/src/app/app.module.ts#L71 [2] https://github.com/angular/angularfire/blob/d724d81eafe003d2ec18728998cec7682aaaaeec/src/app/app.module.ts#L48

Version info

Angular: 12.0.3

Firebase: 9.1.0

AngularFire: 7.1.1

Other (e.g. Ionic/Cordova, Node, browser, operating system):

How to reproduce these conditions

Here is a sample snippet code where I am trying to pass in firebase options using a DI token. This is very useful if my options are fetched dynamically from a server.

export const MY_FIREBASE_OPTIONS_TOKEN = new InjectionToken<FirebaseOptions>(
  'my-firebase-options'
);

@NgModule({
  imports: [
    CommonModule,
   // ******* THIS IS WHERE THE ISSUE LIES ******
    provideFirebaseApp((injector: any) => {
      const firebaseOptions = injector.get<FirebaseOptions>(
        MY_FIREBASE_OPTIONS_TOKEN
      );
      return initializeApp(firebaseOptions);
    }, MY_FIREBASE_OPTIONS_TOKEN),
    // Use 'session' auth state persistence which means the firebase session
    // only persist in the current session or tab, and will be cleared when the
    // tab or window in which the user authenticated is closed. More info:
    // https://firebase.google.com/docs/auth/web/auth-state-persistence
    provideAuth(() =>
      initializeAuth(getApp(), {
        persistence: browserSessionPersistence,
      })
    ),
  ],
  declarations: [],
})
export class FirebaseTestModule {}

elsewhere in my AppModule I can do:

@NgModule({
  imports: [FirebaseTestModule],
  providers: [
    {
      provide: MY_FIREBASE_OPTIONS_TOKEN,
      // You can even use an APP_INITIALIZER provider to fetch this config from server.
      useValue: {
        apiKey: 'my_key',
        authDomain: 'my_domain',
      },
    },
  ],
})
export class AppModule {}

However this would produce the following typescript error: error TS2345: Argument of type '(injector: Injector) => FirebaseApp' is not assignable to parameter of type '() => FirebaseApp'.

Steps to set up and reproduce See: https://stackblitz.com/edit/angular-ivy-psv1mw?file=src%2Fapp%2Ffirebase-test%2Ffirebase-test.module.ts

Sample data and security rules N/A

Debug output

N/A

Expected behavior

I should be able to use injector to access my dependencies.

Actual behavior

No clean way to access injector instance.

Workaround

I have found a workaround to satisfy both typescript while also gaining access to the injector.

  // Workaround: use a rest spread argument to both appease typescript by
  // matching the function signature while also giving us access to the passed
  // in injector argument.
  provideFirebaseApp(...args: any) => {
    const injector = args[0] as Injector;
    const firebaseOptions = injector.get<FirebaseOptions>(
      MY_FIREBASE_OPTIONS_TOKEN
    );
    return initializeApp(firebaseOptions);
  }),

majido avatar Dec 08 '21 15:12 majido

This issue does not seem to follow the issue template. Make sure you provide all the required information.

google-oss-bot avatar Dec 08 '21 15:12 google-oss-bot

Update the issue to match the expected bug report fields.

majido avatar Jan 04 '22 18:01 majido

Ran into this today. I noticed the typings were also wrong for a few other calls like provideStorage(), provideAuth(), etc.

https://github.com/angular/angularfire/blob/d724d81eafe003d2ec18728998cec7682aaaaeec/src/storage/storage.module.ts#L57

Should be a fairly straightforward PR and would be useful for setting up emulators and injecting other tokens without having typescript yell at you.

dereekb avatar Mar 24 '22 22:03 dereekb

Created a PR.

I think I got all the modules. Just searched "fn: () =>" and dropped in injector to all the *.module.ts files where it matched.

dereekb avatar Mar 25 '22 06:03 dereekb