angular / angular

Deliver web apps with confidence 🚀
https://angular.dev
MIT License
94.98k stars 24.84k forks source link

MODULE_INITIALIZER like APP_INITIALIZER #17606

Closed pantonis closed 2 months ago

pantonis commented 6 years ago

I'm submitting a ...


[ ] Regression (behavior that used to work and stopped working in a new release)
[ ] Bug report 
[X] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead see https://github.com/angular/angular/blob/master/CONTRIBUTING.md#question

I was wondering if like APP_INITIALIZER a MODULE_INITIALIZER can be implemented. I have a scenario where multiple lazy load modules exist. Each module has a service that has injected in its constructor a config of type ConfigA. ConfigA is fetched from server. This service is then injected into several module components. With APP_INITIALIZER I cannot do this since the same type ConfigA is used for all modules and a singleton will be created.

vicb commented 6 years ago

Could you please describe your use case more precisely ?

How do you lazy load your module ? If you use the router you might want to use a resolver to fetch data from the serve ?

pantonis commented 6 years ago

Hi,

Please see the sample code I created above that is implemented with resolver and is not working

@NgModule({
    providers: [
        StompServiceProvider,        
        ConfigResolve
    ]

})
export class FeatureModule { }

import { Http } from '@angular/http';
import { OverrideStompService } from "./services/override-stomp.service";
import { ActivatedRoute } from '@angular/router';

export let StompServiceProvider =
    {
        provide: OverrideStompService,
        useClass: OverrideStompService
    };

import { Http } from "@angular/http";
import { Injectable } from '@angular/core';
import { StompService, StompConfig } from "@stomp/ng2-stompjs";

@Injectable()
export class OverrideStompService extends StompService {
    constructor(stompConfig: StompConfig) {
        super(stompConfig);
    }   
}

import { Http, Response } from "@angular/http";
import { Injectable } from '@angular/core';
import { Resolve, ActivatedRouteSnapshot } from '@angular/router';
import { StompConfig } from '@stomp/ng2-stompjs';
import 'rxjs/add/operator/map';

@Injectable()
export class ConfigResolve implements Resolve<StompConfig>{
    constructor(private http: Http) {
    }

    resolve(route: ActivatedRouteSnapshot) {
        return this.http.get('app/config/brokerConfigFile.json')
            .map((response: Response) =>{ 
                debugger;
                return response.json()});        
    }
}
pantonis commented 6 years ago

APP_INITIALIZER works but I have to load all modules configurations in the RootModule. This raises security and isolation concerns as Feature module A will know the config of Feature module B, Feature Module A will know config of Feature module C and so on...

mlc-mlapis commented 6 years ago

... hmm, do you have a plunker that implements the resolver? Because in your code I can't se where you use the resolver service. It should be related to a route ... I can't see any route definition.

There should be something like:

{ 
    path: 'contact/:id',
    component: ContactsDetailComponent,
    resolve: {
      contact: 'contact'
    }
}
pantonis commented 6 years ago

Yes I Have this and forgot to add this in the code above. The problem here is that OverrideStompService is created before the resolver is called

mlc-mlapis commented 6 years ago

... for sure, it is logical. What to think using some identification of the module and then inject that value to the resolver service to know what module is processed and how it should be processed:

static forChild(): ModuleWithProviders {
     return {
          ngModule: MyLazyLoadedModule,
          providers: [
               {provide: ModuleConfig, useValue: {id:12, config: '...', ...} }
          ]
     };
}
pantonis commented 6 years ago

No I dont like the idea with passing module id etc.. I still think idea of MODULE_INITIALIZER is more clean and will also help other scenarios that I cant think of right now.

mlc-mlapis commented 6 years ago

Maybe yes, in some cases but actually you don't have it and doesn't seem that someting like that will get the support in near future.

As I know there should be something called module hook in Angular 5.x what will replace actual APP_INITIALIZER. The question is what exactly there will be implemented (one or more hooks on the module level and which, ...) and if it would be usable even for your case. There is a good chance that it will.

pantonis commented 6 years ago

@mlc-mlapis how do you know that it doesnt seem that will get the support in near future? I opened this ticket to write a suggestion what it would be good to have. Lets wait for the ng team to see what they will reply.

mlc-mlapis commented 6 years ago

Sure. The intention wasn't to say anything against, just a guess. 👀

leak commented 6 years ago

Is there any viable workaround? Basically APP_INITIALIZER functionality on sub module (1 level below app module) level?

mlc-mlapis commented 6 years ago

@leak ... and constructor on a module level is not enough for you?

leak commented 6 years ago

Constructor kicks in too early for me.

const appRoutes: Routes = [
    { path: '', loadChildren: './home/home.module#HomeModule' },
    { path: 'login', component: LoginPageComponent },
    { path: '**', component: NotFoundPageComponent }

Even when I'm loading the LoginPageComponent the Home module constructor is called.

What I need is some sort of life cycle hook which only kicks in when the Home module is actually active.

My actual use case: Loading a user profile. One way is through the login page, but when the auth is already good and the app is loading in another browser tab, I don't want to go through the login component again and rather just fetch the user profile from an api.

Maybe I'm completely on the wrong train, I just started digging into this issue.

mlc-mlapis commented 6 years ago

@leak ... oh, I thought constructor of HomeModule lazy loaded module ... it should be called only when that module is loaded, not earlier, certainly not when LoginPageComponent is instantiated.

leak commented 6 years ago

Just tested again to confirm:

export class HomeModule {
    constructor() {
        console.log("HomeModule ctor");
    }
}

Sadly it shows up on console when I start the application on /login.

mlc-mlapis commented 6 years ago

@leak ... ah, you have that module as a default route ... so no surprise that it is loaded immediately ... { path: '', loadChildren: './home/home.module#HomeModule' } but then its sense as a lazy loaded module is just problematic = doesn't have a sense.

leak commented 6 years ago

Nice catch. Leaves me short of the ctor option though. Guess I have to resort to dabbling with routing events...

hoeni commented 5 years ago

I also like the Idea of having a MODULE_INITIALIZER similar to APP_INITIALIZER very much, because in the module constructor there is no way to wait for async operations, while APP_INITIALIZER may return a promise that is resolved before going on.

BenDevelopment commented 5 years ago

I'm looking for the same feature, I've to run some initialization code when the lazy module loads. APP_INITIALIZER doesn't seems to works with lazy module.

cdutta commented 5 years ago

I am looking forward to a similar feature.

Woodsyla commented 5 years ago

+1 for MODULE_INITIALIZER. I need to populate the store in my feature module with query parameters from the url, currently I am doing it using APP_INITIALIZER but this means the root module needs to know the correct action to dispatch to the feature which feels wrong.

An initialiser hook when the lazy module is loaded seems a reasonable request.

bhavnaberi commented 5 years ago

MODULE_INITIALIZER is a good way to keep the separation of concerns. Example: in my application, there are 10 lazy loaded modules and each module has their configurations. Loading all configurations on App_Load will increase app load time. Also, if a user never loads a particular lazy loaded module, then all configurations loaded for these modules will be useless.

MODULE_INITAILIZER is very much required.

christofferfleat commented 5 years ago

Would also be good for lazy loading global styles/scripts when navigating to a lazy loaded module, global styles/scripts that are only necessary for that particular module.

blemaire commented 5 years ago

Oh no, I was hoping this was available (my root module needs to load some local config for authentication) then, I need to load extra data on child modules,MODULE_INITIALIZER would have been the perfect solution for this

mirogrenda commented 4 years ago

Also from me a +1 for MODULE_INITIALIZER

mdnaserhassan commented 4 years ago

+1 for MODULE_INITIALIZER

kalpeshhpatel commented 4 years ago

+1 for MODULE_INITIALIZER

hoeni commented 4 years ago

As I can see, there are two limitations, that can't be realized currently by an angular app (using the module constructor):

nivrith commented 4 years ago

+1 for MODULE_INITIALIZER

hoeni commented 4 years ago

Is there anything we can do to give this issue some core developer attention? Though I don't have knowledge of Angular internals (yet :-), I can't image that this would be a huge one to implement. On the other hand it would be so helpful on crafting a better application architecture in many use cases.

hareesh996 commented 4 years ago

+1 for MODULE_INITIALIZER

BhishmaS commented 4 years ago

+1 for MODULE_INITIALIZER

Alexzjt commented 4 years ago

3 years after 3years. Is there any related work doing now?

vdjurdjevic commented 4 years ago

Another option would be to support ngOnInit and ngOnDestroy for modules.. Take a look at this issue: #10244

dontboyle commented 4 years ago

+1 for MODULE_INITIALIZER

kristofdegrave commented 4 years ago

If it should be possible if the following code would be added in the RouterConfigLoader. https://github.com/angular/angular/blob/1e9eeafa9e3fbfad5e94d697b8079cf649db626e/packages/router/src/router_config_loader.ts#L35-L44 to

return moduleFactory$.pipe(switchMap((factory: NgModuleFactory<any>) => {
      if (this.onLoadEndListener) {
        this.onLoadEndListener(route);
      }

      const module = factory.create(parentInjector);
       const initStatus = module.injector.get(ApplicationInitStatus);
       initStatus.runInitializers();
       return from(initStatus.donePromise.then(() => {
             return new LoadedRouterConfig(flatten(module.injector.get(ROUTES)).map(standardizeConfig), module);
         }));
}));    

With that in place, it would be possible to add the ApplicationInitStatus Provider inside the module, along with APP_INITIALIZER providers

@NgModule({
 providers: [
    {
      provide: ApplicationInitStatus,
      useClass: ApplicationInitStatus,
      deps: [[new Optional(), APP_INITIALIZER]]
    },
    {
      provide: APP_INITIALIZER,
      useFactory: () => {
        // DO Something
      }
    }
  ]
})
export class Feature

If the ApplicationInitStatus isn't added as a provider, the root one is used, and since this one is already initialized nothing will happen. If provided, only the APP_INITIALIZER defined here will run.

RyannGalea commented 4 years ago

+1 for MODULE_INITIALIZER

jamalsoueidan commented 4 years ago

+1 for MODULE_INITIALIZER

JoseStall commented 4 years ago

+1 for MODULE_INITIALIZER

armanozak commented 3 years ago

+1 for MODULE_INITIALIZER

johnc-ftl commented 3 years ago

While I also think a MODULE_INITIALIZER would be really useful (because it would allow asynchronous initialization in lazy-loaded modules), I believe most of the use-cases for a module initializer could be satisfied if there were support for async factory functions. If async factory functions were supported, classes requiring the async module initialization logic could just take a dependency on a type that is provided by an async factory.

Alternatively, if there were a MODULE_INITIALIZER, it would be possible to complete async initialization for dependencies in the MODULE_INITIALIZER.

atscott commented 3 years ago

It doesn't seem like there's been any discussion here about using canActivate as your module initializer at the root of the forChild module.

RouterModule.forChild([
  {path: '', canActivate: [MyInitializerGuard], children: [...]}
]);

If you think of canActivate as "can this route activate yet?" rather than simply "is it allowed to activate at all?", this conceptually makes sense. This guard would also satisfy the need for an asynchronous blocker to activating any components within the module. This approach seems like it would solve most, if not all, of the use-cases here.

johnc-ftl commented 3 years ago

@atscott - that's a good point, and certainly a useful piece to hack around no module initializer or async factory functions.

However, Angular allows you to lazy-load modules independent from routing (I implemented this just last week to reduce initial request size and overhead). While it's probably less common than lazy-loaded modules in routing, it's an important use case.

Also, the semantics are quite different. A route guard is "can this route be activated" vs "run me on initialization". For example, one would expect module initialization to run once only, whereas the route guard would run every time it is accessed. Yes, you can work around the semantic and behavioral differences, but I think there's still a strong case for async factory functions (or MODULE_INITIALIZER, if you prefer).

atscott commented 3 years ago

@johnc-ftl Absolutely good points. Agreed that these are things that cannot be solved in the router. It may still be a good option for many who have commented here but won’t work for all. We need to be sure the problem and solution are appropriately scoped. For example, the solution provided in #36084 does not solve your use-case and may not necessarily provide additional value beyond canActivate. We should probably close that one then since it doesn’t solve the root problem and is only a partial solution that expands the public API unnecessarily.

jochelo commented 3 years ago

+1 for MODULE_INITIALIZER

wndproc1 commented 3 years ago

+1 for MODULE_INITIALIZER

Alex-hv commented 3 years ago

+1 for MODULE_INITIALIZER

callebstrom commented 3 years ago

+1 for MODULE_INITIALIZER

Djay77 commented 3 years ago

+1 for MODULE_INITIALIZER

Really need this classic feature.

LPCmedia commented 3 years ago

+1 but the limitations of https://github.com/angular/angular/issues/23279 seem to apply here as well. If we can ensure APP_INITIALIZERS are resolved and we can pass values to forRoot and modules...would be awesome.