Ionic-framework: [4.0.0-beta.16] Navigation breaks with latest beta build

Created on 19 Nov 2018  ·  27Comments  ·  Source: ionic-team/ionic-framework

Bug Report

Ionic Info
Run ionic info from a terminal/cmd prompt and paste the output below.

Ionic:

   ionic (Ionic CLI)             : 4.3.1 (/usr/local/npm_packages/lib/node_modules/ionic)
   Ionic Framework               : @ionic/angular 4.0.0-beta.16
   @angular-devkit/build-angular : 0.10.6
   @angular-devkit/schematics    : 7.0.6
   @angular/cli                  : 7.0.6
   @ionic/angular-toolkit        : 1.2.0

System:

   NodeJS : v10.7.0 (/usr/local/Cellar/node/10.7.0/bin/node)
   npm    : 6.4.1
   OS     : macOS

Describe the Bug
Page components do not get cached for re-use like they did in beta-15. This breaks the ion-back-button and any list/detail sort of pages since the list no longer is reused and is created from scratch.

Steps to Reproduce
See repo here. This is the started sidemenu project with a detail page added. To see expected behavior downgrade to beta-15

triage

Most helpful comment

Good news, i have read all the feedback very seriously, and next version will have a behaviour pretty much the same as beta.15, while fixing some bugs in the way.

All 27 comments

@AVatch Can you please change the title of this issue, as the navigation is broken not only for split-pane, but in general. Simple navigation between pages is broken as well - it seems, that StackController removes previous page when moving forward to another page.

Repo to test: https://github.com/mmlleevvyy/ionic-navbug-beta16

@manucorporat The latest commits related to tabs is the cause.

I have the same issues:

The back button disappears when the router.navigateByUrl function is occupied

it's not really a bug, ng router does not have a concept a "direction" like ionic does, previously we were doing so much magic, that lead to very error prone behaviour.

We have decided to do not animate to use stack navigation when "pure" angular routing is used:

<a href="/path">
<button [routerLink]="/path">

Instead you can use:

<ion-button href="/path">

or the [routerDirection] directive:

<button [routerLink]="path" routerDirection="forward">

We will continue experimenting and adjusting the default behaviour during this week. Leaving the issue open to continue discussing.

Thoughts?

I don't think this ticket is about animation, but about navigation in general.

If I, in a classic Master/Detail setup, click on an item in a list, I am taken to the Detail-page with no back button (Or back button uses defaultHref instead of linking to the previous page).

The main problem is how I navigate through the router inside the controller without losing the animation or the button back with router.navigate or router.navigateByUrl

Yeah @manucorporat I'm good with providing the router animation, what I am saying is if you look at the DOM, you'll notice that the when you navigate to any page, the previous component gets removed. Previously that component will be preserved so when you go back you can return from where you left off and not re-instantiate the component. This was the case in beta-15 and no longer in beta-16

Does that makes sense?

As a side note to your comment, most of my use case of navigating involved calling router.navigate([...]) in the component. Can routerDirection be passed in that way? There are a lot of scenarios, (redirect to home page on login for example) which require a programmatic redirect so being able to pass these params if they are now required will be very helpful.

@AVatch If you want to use the stack, you should not use Angular Router but Ionic NavController instead.
It does provide all the methods you need (navigateForward, navigateBack, navigateRoot, etc) to specify the navigation direction and therefore keep (or not) your previous component(s) instantiated in the stack.

@TomDemulierChevret ok sure but like in the documentation it says that the NavController is not bound to the route, so again the documentation advises to use this for subroutine like in dialogs/modals.

Part of the messaging w/ ionic v.4 was while being framework agnostic we could also make use of the latest angular builds, including the router module.

In any case, this issue was not about the animation, but rather the change in how navigating handles components. At the moment even the starter apps don't navigate properly because the components get destroyed rather than saved in the background when navigating away from them, so let's stick to that :)

@manucorporat Adding routerDirection works, but... not when used with a[routerLink], cause routerLink prepares "href" as external link:
https://github.com/angular/angular/blob/20729b33781786c44def273b172f426f265b629b/packages/router/src/directives/router_link.ts#L245

And in HrefDelegate you exclude external links:
https://github.com/ionic-team/ionic/blob/0cd052680159004d7965378ce096f3d4f7b86037/angular/src/directives/navigation/href-delegate.ts#L36

The solution is to call angular's RouterLinkWithHref#onClick instead of router.navigateByUrl. I can make a PR, if you agree.

Edit: I just tested ion-item with [routerLink] and... it doesn't work, that is it does navigate to given view, but not using ionic's StackController etc. It is because, when routerLink is not used with "a" tag, then href will not be set at all, so HrefDelegate will always get url as null/undefined. I will make a PR.

@AVatch What do you mean NavController is not bound to the route ?
It literraly uses urls to navigate and calls Angular Router internally.

See : https://github.com/ionic-team/ionic/blob/master/angular/src/providers/nav-controller.ts

@TomDemulierChevret I was going off of the documentation https://beta.ionicframework.com/docs/api/nav

It's nav not NavController my bad.

I just tested with beta-16 and can confirm that NavController does keep the stack intact (eg components are not destroyed if navigating forward).

@TomDemulierChevret Can you share your code? I just want to compare it to see where the issue is on my end then.

Thanks again!

Had some time to look over this again. When navigating using the NavController provided by ionic, the stack is intact, however using @angular/router does not maintain the stack.

If that's how you guys intend to move forward that's fine I can close this issue. I suppose once it gets some more documentation it won't be confusing, this just came out of nowhere, so it feels broken :)

Here is the repo where I tested the routing
https://github.com/AVatch/nav-break-example/blob/master/src/app/list/list.page.ts#L39

this.navController.navigateForward(['/list', i]);

Navigation is largely broken for me as well on beta 16.

Getting a lot of router-outlet destroyed errors randomly whereas with beta 15 everything was working as expected.

_The related issue below has moved to a separate issue: #16516._

Note: My definition of "reusing" a Page is when "forward" navigating to a URL and instead of pushing a new Page onto the StackController's "stack" it re-uses an instance of the Page that is already present in the stack.

I've been having issues with StackController as well but on Beta 15. I am using the Angular router imperatively in TypeScript but I believe I was experiencing them as well using the new NavController (which uses the Angular router).

Here's a related behavior I've observed:

When you use the router to "forward" navigate to a Page, if the URL (excluding query parameters) is the same as a Page that is already in the "stack," the StackController will remove all of the previous Pages in the stack back to the first occurrence of the matching URL/Page. It then re-uses that first occurrence of the Page in the stack. And so then when you use ion-back-button, which seems to ignore browser history and only uses the stack, the Page you go back to is not what you would expect--it's the Page prior to the first occurrence of the Page in the stack.

  • Note: Since the Android and browser back button seems to ignore the StackController's "stack," I've tried working around this issue to some success by creating my own custom back button component that just uses Angular's Location.back() when you click it. This tends to result in a lot of destroyed/re-initializing of Pages whose state you can attempt to preserve by storing off data in a shared service that is keyed with a unique ID (such as a UUID) present in the URL.

How StackController re-uses Pages has actually been tripping me up in another way; it almost seems like the opposite of the OP's problem but has a workaround that may help with the OP's problem. I can open up a separate issue for this but since StackController behavior might be adjusted due to this issue I wanted to make sure there was awareness of this other behavior.

If I use the Angular router to "forward" navigate to a Page with the same URL as one in the StackController's stack and I have navigation parameters that I store in a shared service (as opposed to the URL) and key the parameter storage in the service off of a URL query parameter such as a UUID (a query parameter--not using the matrix URL notation as noted at the end of this section), because the Page is being re-used due to a matching URL instead of being considered as different, the new query parameter value is not being used and the Page pulls the previous parameters out of the shared service due to using the old query parameter value.

Perhaps the solve is to work around it by just putting the UUID in as a matrix URL notation parameter and effectively opt-out of any StackController reuse of Pages. One upside is that "forward" navigating to the same Page/URL would use the correct parameters. Additionally, with all of the Pages in the stack remaining wired up (never destroyed or reused), when you navigate back to them they are exactly the way you left them when you navigated away, which is how Ionic 3 worked. One downside is that it's non-default behavior that differs from how Ionic seems to think it should work in Ionic 4. Additionally, although it's like Ionic 3, you lose an optimization in which all Pages stay wired-up in the DOM as you continue to forward navigate through the app which may potentially increase the amount of memory that's used by the app.

Update

I've upgraded to Beta 16. I see what the OP was seeing. Only one Page seems to ever be left in the DOM/stack. So for Beta 16, my comments above only apply, apparently, to the NavController/cases where the StackController stack is maintained.

We have situations where we depend on the previous Page still being around, such as navigating to a new Page to select a value and then invoking a callback function from the previous Page to do something with the selected value. Now it's broken. Also same issue as the OP where when navigating back the state of the Page is reset because it's re-created which is more aggressive than the destroying/re-initializing of Pages that I mentioned above.

As some have alluded to, it seems to be because there is no longer a direction when using the Angular router imperatively. Ionic has said the recommendation is to favor the Angular router over the NavController. We've just migrated from the new NavController to the Angular router and now this has changed. Is the recommendation to move back to the new NavController?

So for discussion purposes @manucorporat , with beta 16, is this to say you would have to choose between using the Ionic Nav Controller (and getting back button stack - great for iOS/Android) or using the Angular Router (e.g. routerLink and router.navigateByUrl - great for Web users who have a browser back button)?

I had been thinking that moving to Angular Router would still support the Ionic back button (for use with iOS/Android). On a side note, the Angular router PWA user experience seems a lot faster without router animations.

There still seem a ton of issues even with beta.16 routing/navigationand the work that went into it.
Especially when you look at ion-tabs in conjunction with eg.

  • „outside“ (of tabs) routing, causing the ion-back-button to jump back to the tab‘s main page, not the actual tab that was shown before navigating outside the tab context
  • relative navigation (vs full blown navigateUrl()) to drill down into nested tab pages, which freakingly only works once then refuses to route at all for that tab (route debugging/tracing doesn‘t show anything at that point)

The above is important for more complex, tabs including apps, that reuse page components in various contexts, eg the same 2nd or 3rd level page component (like a list of items) in different tabs.

Here is my current workaround to fix the broken back button (tabs -> non-tab page routing & back):

this._router.events.pipe(
  filter(event => event instanceof NavigationStart)
).subscribe((route: NavigationStart) => {
  // 
  // If route destination is "broken/incomplete", use last known open tab
  if (route.url === '/app/tabs' && this.lastOpenTab) {
    this._router.navigateByUrl(this.lastOpenTab);
  } else {
    // 
    // Fix part 2, keep track of last open tab
    if (route.url.match('/app/tabs')) {
      this.lastOpenTab = route.url;
    }
  }
});

Yeah, I noticed this too. This issue combined with #16451 is keeping me on beta-15, which is fine since it was a good release 👍 Perfection takes time.

Good news, i have read all the feedback very seriously, and next version will have a behaviour pretty much the same as beta.15, while fixing some bugs in the way.

@KevinKelchen could you open a new issue explaining that?

@manucorporat I have a quick question about the routing. I updated to Beta.16 and switched everything to the Ionic router. Are there any performance gains/losses to using the Ionic router over the Angular one?

Thanks, @manucorporat! I was thinking that might need to be the case. 🙂 I have created #16516.

@ireade there is not a Ionic router, it's the same angular router, but we added the "direction" concept, it's just a super small wrapper, still 100% angular router under the hood.

Beta.17 should fix this issue!

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

Was this page helpful?
0 / 5 - 0 ratings