Destroying lazy loaded modules doesn't unload them

[kewde]

I'm submitting a...

[x] Feature request

Current behavior

Currently the lazy loaded module sets itself to the destroyed state. When we revisit the lazy route, it still loads the already-destroyed module. It doesn't unset itself.

Destroying it for a second time results in:

'Error: The ng module LazyModule has already been destroyed.'

Expected behavior

I expect lazy modules to unload themselves once destroyed, re-initializing themselves when they're needed again.

What is the motivation / use case for changing the behavior?

Our application has to manage multiple users. Our services build up a toxic cache of data that belongs to user Alice. When we switch to user Bob, we call destroy() on the lazy loaded module and reload the module for Bob.

[kewde]

I'ved digged in the code a bit.

When the lazy route is visited for the first time, it will take the string or callback of loadChildren and load the lazy module.
Once the lazy module is loaded, it will append an instance of LoadedRouterConfig, named _loadedConfig to that lazy route.

If the _loadedConfig is present, it assumes the module is loaded and will retrieve it.
Deleting the _loadedConfig will cause it to re-initialize the module.

---

So I was scouting for a work around and I have one problem and one blank:

• I can't access the _loadedConfig, it seems like the routes get loaded into an InjectionToken ROUTES with useValue, but the routes in my own application, which I inserted don't show the internal objects (_loadedConfig etc). Does useValue clone them?
• I wonder that when you run .get() on an Injector if it prefers non-destroyed providers/modules over destroyed ones? It seems to not care, because I destroyed my module and it is still returning services that are (semi-fully) functional.

I find it really weird that the instances aren't getting truly destroyed, they're set in a destroyed state and destroy() is called on the underlying providers but my application seems mostly unaffected (except for whatever happens in ngOnDestroy).

[mlc-mlapis]

@kewde ... what is the idea to destroy already loaded modules? Because the effect would be ... loading it once again from a server. Angular has no public API for destroying already loaded modules.

JavaScript has GC and it cleans any memory object which is not already referenced. You can't access private methods and properties from Angular internal API.

[kewde]

@mlc-mlapis

It wouldn't destroy the module as in requiring a reload, but it would destroy the instance and all the data in it.

Angular does have a public API for destroying NgModuleRef's (https://angular.io/api/core/NgModuleRef). I expected that if I get a reference to the lazy loaded NgModule and destroyed it, it would recreate a new instance when revisiting the route. With no data left from the previous session.

"You can't access private methods and properties from Angular internal API.".
Heh, it's JS, we all know that's not really true at runtime ;)

[mlc-mlapis]

@kewde ... you would have a problem in AOT app ...

"You can't access private methods and properties from Angular internal API.".
Heh, it's JS, we all know that's not really true at runtime ;)

Ah, you are right. I forgot about it. And you mean that once instantiated singleton services are kept permanently?

Angular does have a public API for destroying NgModuleRef's

[kewde]

@mlc-mlapis

Ah, you are right. I forgot about it. And you mean that once instantiated singleton services are kept permanently?

The route isn't aware that the NgModule is destroyed (it still sits there in the array, just marked as destroyed), so the DI is always returning that module and its providers (read: lazy-singleton at the lazymodule level, so not really an app wide singleton). The data structure is completely wiped though.

I have some custom initialization logic in the constructor of my lazy module, but that constructor isn't being called anymore because no new instance is being made.

[kewde]

@NgModule({
  imports: [
    CommonModule,
    user_routing, // ModuleWithProviders = RouterModule.forChild(Routes[])
     /*
          The UserModule holds a lot of lazy-wide singleton services.
          A lot of data is in them that must never be mixed up between users
          but is still shared between many components.
     */
    UserModule, 
  ],
  exports: [
    RouterModule
  ],
  declarations: [
    UserRouterComponent
  ],
  entryComponents: [
    UserRouterComponent
  ]
})
export class LazyUserModule implements OnDestroy {
  constructor(
    private _route: ActivatedRoute,
    private _userService: UserService
  ) {
    console.log('LazyUserModule launched!');
    this._route.paramMap.subscribe((params: ParamMap) => {
      // get the user from the route
      const user =  params.get('user');
      // set the user in the singleton UserService
      this._userService.set(user);
    });
  }

  ngOnDestroy() {
    console.log('LazyUserModule destroyed!');
  }
 }

This is the log you'll see:

LazyUserModule launched!
... move to different user: Alice ...
LazyUserModule destroyed!
... move to different user: Bob ...
'Error: The ng module LazyModule has already been destroyed.'

This is the log I expected:

LazyUserModule launched!
... move to different user: Alice ...
LazyUserModule destroyed!
LazyUserModule launched! // instance 2
... move to different user: Bob ...
LazyUserModule destroyed! // instance 2

[mlc-mlapis]

@kewde ... ah I see now, so the behavior is somehow similar to using ActivatedRoute on a component level when you use product/:id routes ... and when the component instance is kept when you route between product/X and product/Y ... and when it's also not a good idea to call ngOnDestroy hook manually.

I didn't know that it's also possible to use ActivatedRoute on a module level ... by the same way as on a component.

[kewde]

@mlc-mlapis
The ActivatedRoute trick didn't actually work, but listening to Router.events does work 👍
But yes, even on the module level you get to enjoy DI.

[mlc-mlapis]

@kewde ... yep, we DI on modules. That's fine.

But I am still thinking about the real used cases with NgModuleRef.destroy() when new instances are not created and when trying to use it you get an error.

[BenDevelopment]

Our application has to manage multiple users. Our services build up a toxic cache of data that belongs to user Alice. When we switch to user Bob, we call destroy() on the lazy loaded module and reload the module for Bob.

@kewde can you explain me how to call destroy() on a lazy loaded module ? I cannot find any documentation about that.

[kewde]

@BenDevelopment

We get our lazy module:

private lazy: NgModuleRef<LazyModule>,

Then we destroy it:

this.lazy.destroy()

---

Then to set the user based on the route:

export class LazyModule implements OnDestroy {
  constructor(
    private _router: Router,
    private _rpc: RpcService // this service is _provided_ by the lazy module!
  ) {
    console.log('LazyModule launched!');
    // Not the prettiest code, but it listens to all router events
    // and if one includes the wallet parameter, it will grab it
    // and set the rpc wallet.
    const loadUser = this._router.events.subscribe((event: any) => {
      if (event.snapshot && event.snapshot.params['user']) {
        this._rpc.user = event.snapshot.params['user'];
        loadUser.unsubscribe();
      }
    });
  }

[BenDevelopment]

Thank you @kewde
Is there something else to do to destroy a lazy loaded module? When I call module.destroy(); I get this error:

Error: ViewDestroyedError: Attempt to use a destroyed view: detectChanges
    at viewDestroyedError (https://localhost:44395/vendor.js:59863:12)
    at Object.debugUpdateRenderer [as updateRenderer] (https://localhost:44395/vendor.js:63334:15)
    at checkAndUpdateView (https://localhost:44395/vendor.js:62713:14)
    at callViewAction (https://localhost:44395/vendor.js:62949:21)
    at execEmbeddedViewsAction (https://localhost:44395/vendor.js:62912:17)
    at checkAndUpdateView (https://localhost:44395/vendor.js:62709:5)
    at callViewAction (https://localhost:44395/vendor.js:62949:21)
    at execComponentViewsAction (https://localhost:44395/vendor.js:62891:13)
    at checkAndUpdateView (https://localhost:44395/vendor.js:62714:5)
    at callWithDebugContext (https://localhost:44395/vendor.js:63601:25)

Any idea?

[kewde]

@BenDevelopment

Route to a location outside of the lazy module before destroying it.

onDestroy() {
  routeToViewOutsideOfLazyModule();
  this.lazy.destroy();
}

[artemverbo]

+1 would love to be able to completely destroy the lazy loaded module, we do run some initialization logic and "reprovide" or "shadow" some services in the lazy loaded module injector via useFactory, but seems like all these factory functions are memoized and never called again, tried to destroy lazy loaded module with the api route mentioned above, but no luck.

[DioLps]

I'm facing the same problem, there is some update about how to fix this?

[RyannGalea]

Looking into this at the moment.

I am currently lazy loading a complete Map Module which I would like to clear once logged out.

The reason behind this is because if the user re-logs in id like the MapModule can re-initialise all it's services etc without a page load.

Unless there is a way to force some sort of re-initialization of a lazy module but not have it as it's the default action on every route change.

Ryann.

[chenhaoftk]

We would like to re-initialize services provided in lazy module too. #28405 looks like what we need. Is the fix being planned?

[hvsharma63]

Looking into this at the moment.

I am currently lazy loading a complete Map Module which I would like to clear once logged out.

The reason behind this is because if the user re-logs in id like the MapModule can re-initialise all it's services etc without a page load.

Unless there is a way to force some sort of re-initialization of a lazy module but not have it as it's the default action on every route change.

Ryann.

We would like to re-initialize services provided in lazy module too. #28405 looks like what we need. Is the fix being planned?

Hello,
While I was researching the topic "How to access same route with different modules/components", I came across this Medium Article.
Now, I found one interesting code(function) which author has implemented.
By doing this, one can re-initalize the module. So that after user logs-in again, that module is initalized to initial state.
I know this a workaround, but by doing this I achieve what I needed to do.
I'm kinda rookie in angular, please let me know anyone if this is the correct way or not.

[angular-robot]

Just a heads up that we kicked off a community voting process for your feature request. There are 20 days until the voting process ends.

Find more details about Angular's feature request process in our documentation.

[ShamimIslam]

When you have a hammer, everything is a nail. Lazy loading is not for caching service data. Lazy loading is for loading code in smaller bundles.

You should be using design patterns like memo, visitor, command or a dictionary pattern in a service to store your data. Alternatively, you should use sessionStorage, or localStorage. Data is data. You just need a singleton service that controls. it. If you need different levels of access, you should partition the data into subobjects.

JS/TS DOES actually destroy objects when you use them properly. If you like, you can even add an interface to access the subobjects from secondary services so it seems like the data is partitioned separately (adapter, facade pattern).

The trick to solving these things is to use the hammer that needs the simplest nail. And if you feel you're going in circles from trying to use the hammer at hand, you should at least try to ask yourself if you have the right tool for the job.

Lastly, if you TRULY MUST reload the services, use a window.location.href to reload the application. That will dump the entire content and start over.

Thoughts?

P.S. The ONLY reason I would want to dump a lazy loaded module is to remove the unused code. Data should not be controlled by module for a single session unless extenuating circumstances require it. And even then, it's better to store that kind of data in an offline data store and retrieve it when needed.

[Mereo4]

When you have a hammer, everything is a nail. Lazy loading is not for caching service data. Lazy loading is for loading code in smaller bundles.

You should be using design patterns like memo, visitor, command or a dictionary pattern in a service to store your data. Alternatively, you should use sessionStorage, or localStorage. Data is data. You just need a singleton service that controls. it. If you need different levels of access, you should partition the data into subobjects.

JS/TS DOES actually destroy objects when you use them properly. If you like, you can even add an interface to access the subobjects from secondary services so it seems like the data is partitioned separately (adapter, facade pattern).

The trick to solving these things is to use the hammer that needs the simplest nail. And if you feel you're going in circles from trying to use the hammer at hand, you should at least try to ask yourself if you have the right tool for the job.

Lastly, if you TRULY MUST reload the services, use a window.location.href to reload the application. That will dump the entire content and start over.

Thoughts?

I disagree. The point here is to be able to control the lifecycle of modules (and services), Angular does not yet offer this ability.

The op gave here one use case, but I have another one that causes me issues too, I have to manually reset some service provided by a lazy module because they are not destroyed once loaded. We should be able to control this as in Dagger or other DI system.

[epieddy]

@ShamimIslam I globally agree with you about : "If you need to destroy a lazy loaded module to clear your data / avoid memory leak, your problem is elsewhere".

But I still want this feature because I have the following use case :

I have a huge "macro app" which assemble different tools for all the different job in the same company. Each employee is allowed to use some of the tools. I have a kind of plugin system (based among other thing on lazy loading via the router) where each tool is a plugin / NgModule and each time a user log in, based on the user permissions, I'm loading only the allowed tools. But when the user logout, I specifically need to unload all the tools because the next user to login might not have the same permissions and therefor will not load the same tools. In my case, it's not about clearing data, it's about "clearing / unloading code OR destroying instances" the next user will not be allowed to use.

[mlc-mlapis]

@epieddy There is the destroy() method that can be used for it.

abstract class NgModuleRef {
abstract injector: Injector
abstract componentFactoryResolver: ComponentFactoryResolver
abstract instance: T
abstract destroy(): void
abstract onDestroy(callback: () => void): void
}

#37095 (comment)

[epieddy]

@mlc-mlapis As @kewde explained here #24962 (comment), yes there is a destroy() method which destroys the Injector (and call the methods ngOnDestroy on all services inside this injector) + call the registered onDestroy callbacks on the NgModuleRef itself.

But the ngModule instance (or NgModuleRef) itself is not cleared as there is still a reference to it inside the router inside a LoadedRouterConfig as the module attribute =>

angular/packages/router/src/config.ts

Line 501 in dfb072a

constructor(public routes: Route[], public module: NgModuleRef<any>) {}

This attribute is never cleared and therefor the NgModuleRef is never gc'ed.

On the other hand, the NgComponentOutlet does clear the reference to the NgModule instance :

angular/packages/common/src/directives/ng_component_outlet.ts

Line 97 in dfb072a

this._moduleRef = null;

[epieddy]

To work around that, I'm currently using an ugly hack :

const home = this._router.config.find(route => route.path === `home`);

home.children = this._extensionService.destroyAllExtensions();

this._router.resetConfig(this._router.config);

// [...]

public destroyAllExtensions(): Routes {

    // [...]

    for (const [extensionId, { instance }] of this._extensions) {

      if (instance) {
        instance.onDestroy();
      }

      this._extensionDestroySubject$.next(extensionId); // my internal wiring
    }

    this._extensions.clear();  // my internal wiring
    this._extensionsInitializationListeners.clear();  // my internal wiring

    const routes: Routes = [];

   // The extension selection screen.
    routes.push({
      path: ``,
      component: this._extensionSelectorComponent,
    });

   // When the user is not logged in, I register a "Missing Extension" placeholder for this catchall route.
    routes.push({
      path: `:extensionId`,
      component: this._extensionMissingComponent,
    });

    return routes;
  }

By resetting the router config, I'm effectively destroying the references to the NgModule instances

[ShamimIslam]

Ok - so if the user is logging out, after the session is cleared, why can't you force the window to reload? Doesn't that solve the problem? setTimeout(()={window.location.href=restartLocation;},1)

…

On Thu, Jun 10, 2021 at 3:44 AM Eddy ***@***.***> wrote: @ShamimIslam <https://github.com/ShamimIslam> I globally agree with you about : "If you need to destroy a lazy loaded module to clear your data / avoid memory leak, your problem is elsewhere". But I still want this feature because I have the following use case : I have a huge "macro app" which assemble different tools for all the different job in the same company. Each employee is allowed to use some of the tools. I have a kind of plugin system (based among other thing on lazy loading via the router) where each tool is a plugin / NgModule and each time a user log in, based on the user permissions, I'm loading only the allowed tools. But when the user logout, I specifically need to unload all the tools because the next user to login might not have the same permissions and therefor will not load the same tools. In my case, it's not about clearing data, it's about "clearing / unloading code OR destroying instances" the next user will not be allowed to use. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24962 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXKQTS4MOGPIBOLATK5UOTTSBUMPANCNFSM4FKTMA2A> .

[epieddy]

Ok - so if the user is logging out, after the session is cleared, why can't you force the window to reload? Doesn't that solve the problem? setTimeout(()={window.location.href=restartLocation;},1)

@ShamimIslam

• Just because a solution works doesn't mean it is a good solution
• In the context of a webapp, reloading the window means loading and parsing once again all the init chunks of your app plus lazy loading all the remaining chunks. And the user has to wait for the app to start again. Also, you might have some context that you wish to preserve between users login and logout
• In my context, the Angular is packaged with electron as a "native desktop app" and launched in full screen. The point is that the users feel like they're using a native application as much as possible. If I have to reload the BrowserWindow every time a user logs out, it kind of kills the native app feeling
• Still in my context, I do indeed need to preserve some context between users session and I think that saving this context somehow (localStorage, main process, etc.) is an uglier hack than the one I've explained earlier

[ShamimIslam]

Ahh. See, now you've explained the why. So far it's been a "web app" - now it's a locally packaged electron solution. Got it. Thanks.

…

On Fri, Jun 11, 2021 at 11:14 AM Eddy ***@***.***> wrote: Ok - so if the user is logging out, after the session is cleared, why can't you force the window to reload? Doesn't that solve the problem? setTimeout(()={window.location.href=restartLocation;},1) @ShamimIslam <https://github.com/ShamimIslam> - Just because a solution works doesn't mean it is a good solution - In the context of a webapp, reloading the window means loading and parsing once again all the init chunks of your app plus lazy loading all the remaining chunks. And the user has to wait for the app to start again. Also, you might have some context that you wish to preserve between users login and logout - In my context, the Angular is packaged with electron as a "native desktop app" and launched in full screen. The point is that the users feel like they're using a native application as much as possible. If I have to reload the BrowserWindow every time a user logs out, it kind of kills the native app feeling - Still in my context, I do indeed need to preserve some context between users session and I think that saving this context somehow (localStorage, main process, etc.) is an uglier hack than the one I've explained earlier — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#24962 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAXKQTTEWJB5GARXXP7YVFTTSIR7HANCNFSM4FKTMA2A> .

[Akxe]

Hello everyone. If anyone of you thinks, that router should be able to automatically destroy modules, and therefore services of the route that is being unloaded, please vote for this comment.

To include the community in the feature request process, we open voting for 60 days. Anyone can cast a vote for the request with a thumbs-up (👍) reaction. When a feature request reaches 20 or more upvotes, we formally consider the feature request. Alternatively, the bot closes the request.

This voting is valid until 26.6.2021

[mlc-mlapis]

@Akxe There is probably a slight problem with the correct understanding of the instruction because it looks like voting should be done on the root of such a feature request and not on the bot comment. In this case, it means that all ten votes won't be counted. It is also a general problem in other feature requests, where people vote in the wrong places.

[Akxe]

@Akxe There is probably a slight problem with the correct understanding of the instruction because it looks like voting should be done on the root of such a feature request and not on the bot comment. In this case, it means that all ten votes won't be counted. It is also a general problem in other feature requests, where people vote in the wrong places.

Let's hope that angular devs will solve this easily understandable mistake people (including myself) made.

Ps: Voted for feature too now!

[Rush]

How can I vote for it? I discovered the issue after the voting ended!

[mlc-mlapis]

@Rush This issue is not closed so you can vote right now.

[Akxe]

@Rush Vote for it anyway. Actually vote for this one too. This one is required for the other one anyway. Whit this completed it might be possible to do it ourselves 😊

[atscott]

Closing as duplicate of #37095

[angular-automatic-lock-bot]

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}