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

Component lifecycles differs on nativescript-angular and angular2 on web. #374

Closed
m-abs opened this issue Jul 27, 2016 · 30 comments
Closed

Comments

@m-abs
Copy link
Contributor

m-abs commented Jul 27, 2016

I ran into this problem and I'll try to explain it here.

With nativescript-angular the lifecycle hook ngOnDestroy isn't called when we navigate away from a page as it would be on web. Navigating back reuses the old component with nativescript-angular but a new instance is created on web.

I've based an example on Nathan Walker's angular2-seed-advanced:
https://github.com/m-abs/angular2-seed-advanced/tree/ngondestroy-not-triggered

I've two component for my routes:
HomeComponent
AboutComponent

Each component have a instanceNo, so we can identify multiple instances of the component.

Action Web TNS
Launch application HomeComponent<1>.ngOnInit() is called HomeComponent<1>.ngOnInit() is called
Click on 'ABOUT' HomeComponent<1>.ngOnDestroy() followed by AboutComponent<1>.ngOnInit() AboutComponent<1>.ngOnInit()
Click on 'HOME' AboutComponent<1>.ngOnDestroy() followed by HomeComponent<2>.ngOnInit() HomeComponent<2>.ngOnInit()
Click back HomeComponent<2>.ngOnDestroy() followed by AboutComponent<2>.ngOnInit() HomeComponent<2>.ngOnDestroy()
Click back again AboutComponent<2>.ngOnDestroy() followed by HomeComponent<3>.ngOnInit() AboutComponent<1>.ngOnDestroy()

As you can see the two behaves very differently.

On web:

  • Navigate to a new page, the current component is destroyed and a new is created.
  • Navigating hitting back, the current component is destroyed and a new is created.

With nativescript-angular:

  • Navigate to a new page, creates the new component but leaves the current component as is.
  • Navigating hitting back, the current component is destroyed and the old component is reused.

If you want to try for yourself, checkout my branch ngondestroy-not-triggered and run:

For web:

npm run start 

For nativescript-angular with android:

npm run start.livesync.android

Shouldn't the two behave the same?
So components are destroyed when we navigate from a page and (re)created when we navigate to the page?

@vakrilov
Copy link
Contributor

vakrilov commented Aug 1, 2016

@m-abs You are indeed correct. When doing a page navigation in native app the page/component you are navigation from is not actually destroyed. When you navigate back to it the component is not created again. The original component is shown instead.

Can you please elaborate on you scenario and why this is a problem?

There are also a couple of other ways to plug into the app navigation:

  1. Subscribe to the Router.events observable
  2. Use route guards

@m-abs
Copy link
Contributor Author

m-abs commented Aug 2, 2016

@vakrilov
Thank you for the suggest, I'll try it when I get back to work :)

I'll can try to elaborate, sorry for the long text :)

Our project is based on angular2-seed-advanced, so we share code between the {N} -version and the web-version of our project.
The project is an audiobook-player that shows HTML-content synced to the audio during playback.
We render the HTML-content with our content-viewer-component. On web we use the component as is and in {N} it runs in a small ng2-webapp inside a web-view.
Our page-component sets up events and subscriptions to make this work in our ngOnInit() hook and they’re removed in our ngOnDestroy()hook.

Since the page-component lives on after navigation from it, the subscriptions and events created in ngOnInit() will still be active, this means that the web-view/content-viewer-component still does work that is now unneeded.
We also risk having two instances of the web-view/content-viewer-component running at the same time. If the user navigates to the page again without going back, since that won’t reuse the component.

I also think the behavior goes against what the ng2-docs imply.

From: https://angular.io/docs/ts/latest/guide/lifecycle-hooks.html#!#hooks-overview

hook Purpose
ngOnInit() Initialize the directive/component after Angular initializes the data-bound input properties.
ngOnDestroy() Cleanup just before Angular destroys the directive/component. Unsubscribe observables and detach event handlers to avoid memory leaks.

From: https://angular.io/docs/ts/latest/guide/router.html#!#route-parameters

Observable params and component re-use

In this example, we subscribe to the route params Observable. That implies that the route params can change during the lifetime of this component.

They might. By default, the router reuses a component instance when it re-navigates to the same component type without visiting a different component first. The parameters can change between each re-use.

We can of cause use the Router.events observable as a workaround, but I’d much rather just rely on the hook ng2 components have for it.
I’d also prefer the same behavior (where possible) on both web and in {N}, otherwise it can be surprising for the developers.

I can see it’s done intentionally here: https://github.com/NativeScript/nativescript-angular/blob/master/nativescript-angular/router/page-router-outlet.ts#L112
And I do understand the benefits of caching components like that, but what about larger app with many pages where the user don’t necessarily navigates back?
Don't we risk it accumulates old components and thus causing the leaks the ng2-docs mention?

@dvabuzyarov
Copy link

+1

@ignaciolarranaga
Copy link

Maybe some intermediate approach is when using the paged navigation indicate which is the expected reusability of the components (I would say destroy as default and preserve as alternative).

@sivsushruth
Copy link

Any updates on this ? Very urgent!

@sivsushruth
Copy link

We are using it in a soon to be launched app and we have already invested too much time with nativescript to go back now. This issue is going to be a dealbreaker. I am willing to help if someone can let me know what do do.

@m-abs
Copy link
Contributor Author

m-abs commented Sep 29, 2016

Our designer wants a three tabbed interface, where the navbar is always visible at the bottom of the screen and the user can tap on each tab.
So the user can always forward navigate to any of our three tabs.
The way the router caches pages at the moment and since child-routes doesn't work right on back (see #477) we'll will end up with memory leaks it we don't find a solution.

I've been considering using an JS-hack to work around this by making the components constructor return the first component, but this is not a solution I like.

@Component({...})
export class PageComponent {
  private static singleton: PageComponent;
  constructor(...) {
    if (PageComponent.singleton) {
      return PageComponent.singleton;
    }

     PageComponent.singleton = this;
  }
}

There're a bunch of problems with this solution, like potentially breaking DI, since the first components injections will always be used. and ngOnInit would get called multiple times.

I've also considered making a private fork of the ns-router that doesn't have caching feature, but I really don't want to maintain it.

@vakrilov vakrilov self-assigned this Sep 30, 2016
@sivsushruth
Copy link

Any way to temporarily disable cache folks ? This issue is really the breaking point and we need to launch our app ASAP. Can someone please help out. Thanks in advance

@m-abs
Copy link
Contributor Author

m-abs commented Oct 6, 2016

@sivsushruth
If you use a router-outlet and managed back-buttons yourself, pages aren't cached. (Since it's not a NativeScript Page switch).

@m-abs
Copy link
Contributor Author

m-abs commented Oct 6, 2016

@sivsushruth
Unfortunately you need to capture back-button on android (like this: https://github.com/m-abs/angular2-seed-advanced/blob/child-router-android-back-button/nativescript/app/pages/app/app.component.ts#L58-L69) and add your own NavigationButton to the ActionBar.

@sivsushruth
Copy link

@m-abs The issue however is, I need caching for a few pages, is it possible to have router-outlet inside page-router-outlet ?

@m-abs
Copy link
Contributor Author

m-abs commented Oct 6, 2016

@sivsushruth
That's what we do.

We've created a few child routes like this: https://github.com/m-abs/angular2-seed-advanced/blob/child-router-android-back-button/src/client/app/components/home/home.routes.ts
And since we're using child-routes, we needed to add an router-outlet to our HomeComponent: https://github.com/m-abs/angular2-seed-advanced/blob/child-router-android-back-button/src/client/app/components/home/home.component.tns.html#L23

Routes under /home/ in our example are not cached.

@tviel
Copy link

tviel commented Oct 17, 2016

+1 for ng2 lifecycle or additional hooks to determine cache state of page. This would be helpful to subscribe / desubscribe from observables (e.g. if you navigate by observable return on pages, you get hit by race conditions to which page you actually will be navigated).

@m-abs
Copy link
Contributor Author

m-abs commented Oct 22, 2016

@tviel You can listen to router.events, you can't use ActivatedRoute, because it seems to stop emitting events after first navigate away from the component...

Could we please make this behavior optional? It makes sense to cache some pages but not others.

@m-abs
Copy link
Contributor Author

m-abs commented Oct 26, 2016

@tviel Update on my last comment.

Router.events didn't work for us, but I can listen to Page.unloadedEvent and Page.loadedEvent to see if a component is unloaded.

@gergderkson
Copy link

@m-abs I was also able to workaround this using :

        this.page.on(Page.unloadedEvent, event => {
            this.ngOnDestroy();
        })

But it definitely seems like overhead.

@vakrilov
Copy link
Contributor

vakrilov commented Nov 2, 2016

Let's do a little summary so far

Subscriptions And CleanUp

When using page-navigation ngOnInit() will be called the first time the component is navigatedTo and ngOnDestroy() will be called when navigating back from the page with the component (when it is removed form the navigation stack). When navigating forward(deeper in the app) those hooks will not be called. The component instance will be preserved as it is still connected to a page that lives inside the native navigation stack.

However, you might want prevent it from updating while not shown. So far I think @m-abs solution of using page events would be the simplest. You might call the ngOnInit()/ngOnDestroy() hooks directly, but it's good idea to guard them with a flag to prevent double execution.

Using ActivatedRoute and Params

Normally, in Angular you would inject ActivatedRoute and read parameters from it. Your component will be reused if you do a subsequent navigations to the same route while only changing the params. That's why params and data inside ActivatedRoute are observables.

In NativeScript when navigating back to an existing page your component will not be re-created. Angular router will still create an new instance ActivatedRoute and put all params in it, but you cannot get hold of it trough injection as no constructor is called.

The Solution: In NativeScript you can inject PageRoute which has an activatedRoute: Observable<ActivatedRoute> inside it. Each time a new ActivatedRoute instance is created for this re-used component it will be pushed in this observable, so you can still get you params:

So, instead of:

class MyComponent {
  id: number;
  constructor(private route: ActivatedRoute) {
    this.route.params
      .forEach((params) => { this.id = +params['id']; });
    }
}

You do:

class MyComponent {
  id: number;
  constructor(private pageRoute: PageRoute) {
    // use switchMap to get the latest activatedRoute instance
    this.pageRoute.activatedRoute
      .switchMap(activatedRoute => activatedRoute.params)
      .forEach((params) => { this.id = +params['id']; });
  }
}

Why So Complicated

Native platforms preserve the views in the native navigation stack when navigating deeper. So we need to preserve the components that are connected to those views too. This is done in the <page-router-outlet>. However, there are some limitations in terms of how Nativescript plugs into the router live-cycle explained in angular/angular#7757. I hope in future version of angular these will be resolved so we can narrow the gap between using <page-router-outlet> and the stock <router-outlet>

@phattranky
Copy link

phattranky commented Apr 8, 2017

@m-abs

Thanks :D

So great, use loadedEvent, unloadedEvenet to call ngOnInit and ngOnDestroy solved all problems

@bunnyvishal6
Copy link

Awesome! Page.unloadedEvent on ngOnInit solved my problem. @m-abs Thanks alot.

@nicodemuz
Copy link

It still doesn't work for me. Any idea where I'm doing wrong?

import { Component, OnDestroy } from "@angular/core";
import { ActivatedRoute } from "@angular/router";
import { ListingService } from "../../shared/listing/listing.service";
import { Listing } from "../../shared/listing/listing";
import { RouterExtensions, PageRoute } from "nativescript-angular/router";
import "rxjs/add/operator/switchMap";
import {Page} from "ui/page";

@Component({
  selector: "listing-detail",
  providers: [ListingService],
  templateUrl: "components/listingDetail/listingDetail.html",
  styleUrls: [
    "components/listingDetail/listingDetail-common.css",
    "components/listingDetail/listingDetail.css"
  ]
})
export class ListingDetailComponent {
  public id: number;
  public listing: Listing;
  constructor(
    private pageRoute: PageRoute,
    private page: Page,
    private listingService: ListingService,
    private routerExtensions: RouterExtensions
  ) {
    // Get listing ID
    this.pageRoute.activatedRoute
      .switchMap(activatedRoute => activatedRoute.params)
      .forEach((params) => { this.id = +params['id']; });
  }

  ngOnInit() {
    this.listingService.get(this.id).subscribe(loadedListing => {
      this.listing = loadedListing;
    });

    this.page.on(Page.unloadedEvent, event => {
        this.ngOnDestroy();
    })
  }

  public onBackButtonTap() {
    this.routerExtensions.back();
  }

  ngOnDestroy() {

  }
}

@bunnyvishal6 @m-abs able to help?

@m-abs
Copy link
Contributor Author

m-abs commented Jul 1, 2017

@MrNicodemuz
I think the problem is you don't call ngOnInit() after you change the id. So you don't load the new data.

Sent from my OnePlus ONEPLUS A3003 using FastHub

@nicodemuz
Copy link

@m-abs many thanks it's working now! :)

@jdnichollsc
Copy link

@m-abs @gergderkson Guys, my app is closed after call this.ngOnDestroy();
Any help is really appreciated

@evertonrobertoauler
Copy link

Hi, I solved the issue, triggering change detection in my root component, every time any navigation ends.

this.router.events.subscribe(e => e instanceof NavigationEnd && this.changeDetectionRef.detectChanges());

@jnorkus
Copy link

jnorkus commented Oct 16, 2018

This is one of the most annoying "features" of NS Angular and it doesn't seem like anyone's looking to improve it. We've spent so much time debugging issues where resources haven't been released and streams are just piling up because ngOnDestroy is not getting called.

The biggest problem here is inconsistency - the constructor and ngOnInit is getting called every time the component is navigated to. But when navigating out, you're only left with the router event and page unloaded event.

Getting rid of ng events and just using Page events would seem like a good solution but the problem is - the page is unloaded every time app goes to background. Some of the app logic needs to keep running in the background, so this is not a good solution either.

@NickIliev
Copy link

Closing the issue - for anyone interested in how the native mobile events are working and should be used alongside the Angular ones please read this comment

@Stanteq
Copy link

Stanteq commented Sep 26, 2019

I wrote an article about this. I solved it in generic way with the decorator.

@MitchTalmadge
Copy link

Man, so much for code re-use. Now I have to create an entirely separate tns controller just to use the PageRoute instead of ActivatedRoute. What a joke.

@flodaniel
Copy link

This remains a major limitation / "feature" :/

@aosi87
Copy link

aosi87 commented Oct 12, 2020

Why if Im writing a wizard, i need to move forward and backward, and sometimes move, skip, or jump to pages/views and limit user capabilities of moving back to pages (i could set a new root here, but not my case) not catched or in any state. Also if i complete the wizard i should be able to ngDestroy all the previous views/pages to do a clean new wizard setup. Current logic is not allowing me to do that, and i do not wanna recreate the wheel or change the native behavior, is there a way to trigger those events or angular cycle hooks (init, destroy, etc) using nativescript?

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