TrilonIO/angular-application-insights

Multiple imports

worldspawn opened this issue · 1 comments

Hi,

So I wanted to define a module (ModuleB) that imports (among) other things appinsightsmodule and then have my app module import that. My lazy loaded modules also import the same module.

Now ModuleB implements forRoot to stop a instance being made of every service for each lazy module. If I import AppInsightsModule here I really should be getting these errors 'ApplicationInsightsModule is already loaded. Import it in the AppModule only'

Rather the service instance that is passed to the Module should be checked to see if its already initialised or simply assume that it is if there is a parent module.

I wrote my own module to import and it seems to be working fine, though not a lot of testing has happened yet.

Note it provides the AppInsightsService in the forRoot.

@NgModule({
  imports: [ CommonModule ],
  declarations: [],
  exports: [],
  providers: [ AppInsightsService ]
})
export class ApplicationInsightsModule {

  constructor (
    @Optional() @SkipSelf() parentModule: ApplicationInsightsModule,
    appInsightsService: AppInsightsService
  ) {
    if (!parentModule) {
      appInsightsService.init();
      // throw new Error(
      //   'ApplicationInsightsModule is already loaded. Import it in the AppModule only');
    }


  }

  static forRoot(config: AppInsightsConfig): ModuleWithProviders {
    return {
      ngModule: ApplicationInsightsModule,
      providers: [
        { provide: AppInsightsConfig, useValue: config },
        AppInsightsService
      ]
    };
  }
}

My understanding of forRoot is not great but I think that because you prevent it from being imported more than once its kind of redundant to implment forRoot at all

@worldspawn You're completely correct, I don't even know how I didn't notice that yet!!
If you can get a PR in for that we could get it released in a newer version of the package - I'm sure it'd help lots of others.

Good catch on this one 💯