ionic-team / ionic-framework

A powerful cross-platform UI toolkit for building native-quality iOS, Android, and Progressive Web Apps with HTML, CSS, and JavaScript.
https://ionicframework.com
MIT License
51.02k stars 13.51k forks source link

Custom route reuse strategy not supported #17673

Closed webmozart closed 1 year ago

webmozart commented 5 years ago

Bug Report

Ionic version: [x] 4.0.2

Current behavior: As described in #15688. When implementing a custom RouteReuseStrategy that tries to store and reuse specific routes, IonRouterOutlet bails out with the error:

incompatible reuse strategy

(see IonRouterOutlet::detach())

Expected behavior: I expect route reuse strategy that try to attach/detach routes to work as designed for Angular.

Steps to reproduce: Add the code below to a barebone Ionic application and navigate to any page.

Related code:

test-route-reuse-strategy.ts:

export class TestRouteReuseStrategy implements RouteReuseStrategy {

    private handle: DetachedRouteHandle = null;

    shouldReuseRoute(future: ActivatedRouteSnapshot, current: ActivatedRouteSnapshot): boolean {
        return true;
    }

    shouldAttach(route: ActivatedRouteSnapshot): boolean {
        return this.decide(route) && null !== this.handle;
    }

    shouldDetach(route: ActivatedRouteSnapshot): boolean {
        return this.decide(route);
    }

    store(route: ActivatedRouteSnapshot, handle: DetachedRouteHandle | null): void {
        this.handle = handle;
    }

    retrieve(route: ActivatedRouteSnapshot): DetachedRouteHandle | null {
        return this.handle;
    }

    private decide(route: ActivatedRouteSnapshot): boolean {
        // Simplified
        return true;
    }

}

app.module.ts:

    providers: [
        // ...
        { provide: RouteReuseStrategy, useClass: TestRouteReuseStrategy }
    ],

Other information: Previously reported in #15688 and closed, but this seems to prevail in master.

Ionic info:

Ionic:

   ionic (Ionic CLI)             : 4.10.2 (/home/snip/.config/yarn/global/node_modules/ionic)
   Ionic Framework               : @ionic/angular 4.0.2
   @angular-devkit/build-angular : 0.12.4
   @angular-devkit/schematics    : 7.2.4
   @angular/cli                  : 7.2.4
   @ionic/angular-toolkit        : 1.3.0

Cordova:

   cordova (Cordova CLI) : 8.0.0
   Cordova Platforms     : android 7.1.4
   Cordova Plugins       : cordova-plugin-ionic-keyboard 2.1.3, cordova-plugin-ionic-webview 3.1.2, (and 5 other plugins)

System:

   Android SDK Tools : 26.1.1 (/home/snip/Android/Sdk)
   NodeJS            : v8.9.4 (/home/snip/.nvm/versions/node/v8.9.4/bin/node)
   npm               : 6.8.0
   OS                : Linux 4.15
frankpapenmeier commented 5 years ago

+1 I also tried implementing a custom ReuseStrategy in Ionic 4.0.2 -> Whenever shouldDetach returns true, the app throws an error message.

liamdebeasi commented 5 years ago

Hi there,

Thanks for opening an issue with us! Would you be able to provide a repository with the code required to reproduce this issue?

Thanks!

webmozart commented 5 years ago

Sure, here you go: https://github.com/webmozart/ionic-custom-routing-strategy

AKosmachyov commented 5 years ago

@webmozart JFYI, you see this error because it was embedded in the source code of the framework. https://github.com/ionic-team/ionic/blob/617453ba439a7dc1f7d278fa82a0b79498ef14e2/angular/src/directives/navigation/ion-router-outlet.ts#L119-L130

liamdebeasi commented 5 years ago

Hi there,

This is definitely something that is on our radar. We've had to prioritize a few other things, but we hope to loop back to this soon. Didn't want you to think we were just ignoring you 🙂

Thanks!

agustiza commented 5 years ago

+1 on the need. It's a quite needed feature for complex applications and it's blocking us from completely migrating to Ionic 4.

daem0ndev commented 5 years ago

Until then, if you really need to use your own custom route reuse strategy, you can just opt out of using the ion-router-outlet component and just use the vanilla angular router-outlet component instead.

markgsmith64 commented 5 years ago

@daem0ndev - do you know how I can use the vanilla angular router-outlet component?

kettunen commented 4 years ago

Any progress on this?

raphrmx commented 4 years ago

Still nothing new on this important topic?

strikergeorge commented 4 years ago

I saw another answer in https://github.com/ionic-team/ionic/issues/15688 said issue fixed in Beta 12. I will have a try.

casper5822 commented 4 years ago

Is there any new about this bug? We cannot deploy our app without a custom strategy.

psenechal commented 4 years ago

Did a fix for this happen to make it into Ionic 5? This is really important for keeping Firebase read queries to an acceptable level when working with large datasets and attaching snapshot listeners.

simsonraj commented 4 years ago

Did anyone try with native angular router? I still get the same error even with it. Simply put with ionic we cant use Reuse strategy at all?

moscoso commented 4 years ago

I am having the same issue. Writing my own custom strategy, an error gets thrown by Ionic Angular when shouldDetatch gets called.

Unfortunate this has gone on this long, this is a key to larger apps. In general, i've been having a poor experience with the ionic/angular router, and the documentation is not fleshed out on the advanced details or topics.

moscoso commented 4 years ago

Regarding the poor experience, why does the split pane starter app with a menu on the side work with ion-router-outlet but not with router-outlet? Ion pages are competely incompatible with router-outlet which forces us to opt into ion-router-outlet

There are barely any differences being documented https://ionicframework.com/docs/api/router-outlet

moscoso commented 4 years ago

I am trying to use a custom strategy, it's not supported by ion-router-outlet, so I need to use regular router-outlet but it breaks my entire app layout.

If there is no progress to be made on this front then we this issue should be reopened:https://github.com/ionic-team/ionic/issues/17140#issuecomment-455769382_

We need a solution to either problem to at least making progress in a crucial router feature of an angular app.

ArjunAce commented 4 years ago

Any news on this? I too want to use a custom route reuse strategy but stuck due to this issue.

C-racker commented 4 years ago

Is there any progress so far

jashick commented 4 years ago

This is kind of a big deal... still no solution? I'm running ionic 5.2.3 and still having the issue.

reatailret commented 4 years ago

Dear contributors and developers, please, reply. Any way to use "reuse strategy" feature in future?

liamdebeasi commented 4 years ago

Hi everyone, this feature has not been implemented yet. We will updated this thread with any future updates, thanks.

liamdebeasi commented 4 years ago

@psenechal Do you have any more information regarding your Firebase use case?

reatailret commented 4 years ago

@psenechal Do you have any more information regarding your Firebase use case?

@liamdebeasi Example of use

Let see a typical example: Search Form Component loads /search-form route. We make a search Navigate to /search-results route that loads Search Results Component Click a result and navigate to /detail/1 that loads Detail Component Click Browser back button to return to navigate to the /search-results route that loads again the Search Result Component The step 4 will cause the Search Result Component to run again the same search, making a useless GET request to the API.

We want to cache and avoid total reloading of the Search Result Component (and preserve the scrolling) on the step 4 and only reload it when navigating from step 1 to step 2.

mhartington commented 4 years ago

@reatailret How is search result component scaffold out? Not looking for the exact code, but something like

class SearchComponent{
  constructor(...){...}

  ngOnInit(){...}

  ionViewDidEnter(){...}

}

More specifically, when are you making the search. The route reuse setup here has nothing to do with components being cached as you are describing

psenechal commented 4 years ago

Exactly what @liamdebeasi referred to...thanks for summing that up so beautifully.

The only thing I would add is that I have "list" pages that fetch a set of records from a collection in Firestore. When I do that, I typically use snapshotChanges so that a listener is created and any time the collection is updated, the records (and only the records that have changed) are then updated on my list page. When a component has to be reloaded, that listener is destroyed and a new one is created...along with pulling that list of records again which is unnecessary.

psenechal commented 4 years ago

VERY simplified version of what I would typically do...

class ListComponent{

  public listItems: ListItem[];

  constructor(
    private afStore: AngularFirestore
  ){}

  ngOnInit(){
    this.listItems = this.afStore.collection<ListItem>('listItem').snapshotChanges();
  }

  ionViewDidEnter(){...}
}

By getting the items in the OnInit method, it only runs when the component is initialized and the listener is created using the snapshotChanges() method. If I can use a custom route reuse strategy, I don't have to destroy this component and reinitialize it causing the OnInit method to run again which will get the items all over again and recreate the listener.

mhartington commented 4 years ago

So what you are describing is not related to the route reuse strategy. @psenechal if you could put together a very simplified demo, we can take a look, but this doesn't sound like it has anything to do with a route reuse strategy

StriderRanger commented 4 years ago

Another example where the Custom route reuse strategy might be useful is when reloading inside an Ionic tabs application. Sometimes we want a certain page/component to be always present inside a given tab, and prevent it from being destroyed.

This is a stackblitz example, with a full explanation in this link that was posted as a feature request.

reatailret commented 4 years ago

@reatailret How is search result component scaffold out? Not looking for the exact code, but something like

class SearchComponent{
  constructor(...){...}

  ngOnInit(){...}

  ionViewDidEnter(){...}

}

More specifically, when are you making the search. The route reuse setup here has nothing to do with components being cached as you are describing

@mhartington StackBlitz link, open and open console.

  1. click on "Start search", search page opened, in console "START SEARCH"
  2. click on Detail 1, detail page opened, in console "START DETAIL"
  3. click on browser "BACK" button , search page opened, in console NO "START SEARCH"

StackBlitz edit code link

  1. Change in CustomReuseStrategy.ts on line 4 url to something other, save project, open new window with project.
  2. click on "Start search", search page opened, in console "START SEARCH"
  3. click on Detail 1, detail page opened, in console "START DETAIL"
  4. click on browser "BACK" button , search page opened, in console "START SEARCH" DIFFERENCE HERE .
zrev2220 commented 4 years ago

Any update on this issue? I'm trying to use a custom RouteReuseStrategy in my Ionic app and can't use ion-router-outlet because of this issue (same problem @moscoso described here). As others have mentioned, it's blocking advanced functionality in larger apps.

mhartington commented 4 years ago

@zrev2220 if you have a use case for why you would need one, please share it.

jashick commented 4 years ago

I have a use case. I have a web only app with sidemen and 4 elements. The main page contains a large list of items. Every time I leave to go to a different sideman item I love the view. It recreates the entire page, and list. There is no way to keep a place of where they were. I can't understand why there isn't some method of reusing and storing views.

zrev2220 commented 4 years ago

@mhartington My use case: I have a page that previously used ion-tabs to display one of several subpages. The advantages with ion-tabs was:

  1. It could interface with the Angular router to determine which subpage to display.
  2. After displaying any page(s), those pages would not get destroyed until the parent page (containing the ion-tabs) was navigated away from, at which point the parent and all children that were created would all receive summary destruction.

I needed to use a different interface than that provided by ion-tabs to let the user select a subpage, so I have switched to using a combination of ion-segments and my own dropdown-style method of page selection, and a nested router outlet to hook into the router for displaying subpages. I currently have everything working except for no. 2 above.

If I use Angular's router-outlet instead of ion-router-outlet, then everything works, but then my subpages can no longer use Ionic's lifecycle hooks such as ionViewWillEnter. If ion-router-outlet supported a custom reuse strategy, then I would have everything working. The only other alternative is to refactor the child components to not rely on ionViewWillEnter, etc.

mhartington commented 4 years ago

@zrev2220 do you have a demo that we could look at? Seeing "nested router" already waves a bunch of red flags that say there are better approaches.

daem0ndev commented 4 years ago

To all those requesting this, I feel like you might need to explore the source code a bit further. The IonicRouteStrategy is actually not doing too much, and if you REALLY need your own custom route reuse strategy and you need it to support ion-router-outlet, simply make sure to always return false for shouldAttach / shouldDetach, and voila you dont get the exception. This is fine in the case of ion-router-outlet because it maintains a stack of components that were activated on that outlet which is very different than how angular's native router-outlet behaves where it only maintains the active component per outlet (not a stack). If anyone in the community wants free support on this with your application, im happy to help if you provide a repo and dm me, I really dont think there is an issue for @ionic here. @mhartington @zrev2220 @jashick @reatailret

zrev2220 commented 4 years ago

@mhartington In short, all I need is ion-tabs functionality with a custom mode of tab/page selection. This came around to a router reuse strategy when I tried getting child pages of my pseudo-tabs component to stay un-destroyed until that higher-level page is destroyed.

Clarification: My pseudo-tabs component uses a nested router outlet, not an entire router. From what I could tell from various Angular and Ionic docs, nesting outlets is not bad practice and is useful in varied situations.

If there's a better way to customize the tab selection mechanism with ion-tabs where I don't have to do as much wheel-reinventing, I'd be happy to give it a shot.

mhartington commented 4 years ago

@zrev2220 do you have a demo that we could look at?

Please provide a demo of what you are currently using so we can better understand.

zrev2220 commented 4 years ago

@mhartington Here's two StackBlitzes demo-ing kind of what I'm trying to do. They're in pure Angular without Ionic, keep in mind. The parent component represents my pseudo-tabs component, allowing you to view one of the children in a nested outlet based on the routing path.

https://stackblitz.com/edit/angular-tab-ish-route-reuse
👆 This one uses a custom reuse strategy (which is a bit messy) to keep the children alive while the parent lives, then destroy the children when the parent gets destroyed. This example works exactly they way I need it to, but I can't drop it into my Ionic app because of the "incompatible reuse strategy" issue.

https://stackblitz.com/edit/angular-tab-ish-no-route-reuse
👆 This one is the same as above but with the custom reuse strategy stuff pulled out. Hence it's much simpler, but the previous child dies when you navigate to a new one, which won't work for my use case.

daem0ndev commented 4 years ago

@mhartington In short, all I need is ion-tabs functionality with a custom mode of tab/page selection. This came around to a router reuse strategy when I tried getting child pages of my pseudo-tabs component to stay un-destroyed until that higher-level page is destroyed.

Clarification: My pseudo-tabs component uses a nested router outlet, not an entire router. From what I could tell from various Angular and Ionic docs, nesting outlets is not bad practice and is useful in varied situations.

If there's a better way to customize the tab selection mechanism with ion-tabs where I don't have to do as much wheel-reinventing, I'd be happy to give it a shot.

@zrev2220 I agree there is nothing wrong with nesting outlets, and its useful, but I think the key difference is that an ion-router-outlet specifically has some interesting implications and imo it should be used only for the main view routing because of how it maintains stacks to feel native and it provides the sweet animation between screens. I would argue that for nested outlets, those features may not be necessary or ideal (weird use case imo to have nested stacks with animations within a view for child routes). This is why nested outlets imo should be regular angular outlets. Now if you really just need the ionViewDidEnter capabilities, you can just switch to something more angular native so that it will work consistently regardless of what type of outlet that component is presented in. I.e., if a component is in a regular angular outlet, you could simply use the ngOnInit which would be essentially the same as ionViewDidEnter.

zrev2220 commented 4 years ago

@daem0ndev Makes sense to avoid ion-router-outlet in this case (and nested cases in general). I could certainly move away from the Ionic lifecycle hooks and switch to a normal router-outlet. The only issue with that is ngOnInit only runs when the component is created. Under ion-tabs, going back to a tab you've used before triggers ionViewWill/DidEnter, but not ngOnInit since it already init'd when you first go to the tab.

I can find ways around this, though. The more this discussion progresses and the more I have to write about what I'm doing :joy:, the more I'm realizing it'd probably be best to not rely on ionView* events and use a normal Angular outlet.

mhartington commented 4 years ago

@zrev2220 do you have a demo that includes Ionic in there? If the goal is to just be able to include some sub-components that are part of URL, you could always do so like this.

import { Component } from '@angular/core';
import { Router, ActivatedRoute } from '@angular/router';

@Component({
  selector: 'app-home',
  template: `
    <ion-header>
      <ion-toolbar>
        <ion-title> Blank </ion-title>
      </ion-toolbar>
    </ion-header>

    <ion-content>
      <ion-segment [(ngModel)]="segmentValue" (ionChange)="updateHash()">
        <ion-segment-button value="friends">
          <ion-label>Friends</ion-label>
        </ion-segment-button>

        <ion-segment-button value="enemies">
          <ion-label>Enemies</ion-label>
        </ion-segment-button>
      </ion-segment>
      <container-element [ngSwitch]="segmentValue">
        <div *ngSwitchCase="'friends'">Value is friends</div>
        <div *ngSwitchCase="'enemies'">Value is enemies</div>
      </container-element>
    </ion-content>
  `,
  styleUrls: ['home.page.scss'],
})
export class HomePage {
  segmentValue = 'friends';
  constructor(
    private router: Router,
    private route: ActivatedRoute
  ) {}
  ngOnInit(){
    const params = this.route.snapshot.queryParamMap.get('segment');
    this.segmentValue = params ? params : 'friends';
  }

  updateHash() {
    const navExtras = { queryParams: { segment: this.segmentValue } };
    this.router.navigate([], {...navExtras});
  }
}

To me, this is the correct way to handle situations where you want to mix segments with conditional rendering, not nested routes.

zrev2220 commented 4 years ago

@mhartington Thanks for the snippet. I had never seen ngSwitch before 😂 I agree this looks like a much better approach compared to getting the router involved like ion-tabs does. I'll give this a shot and come back with any questions I come up with.

zrev2220 commented 4 years ago

@mhartington Gave your ngSwitch idea a shot, and I like the way it works/feels. The only issue I have with it is that when I switch which component is rendered, the previous one is destroyed. The perk from ion-tabs I'm trying to replicate is how it doesn't destroy children until the parent is destroyed.

mhartington commented 4 years ago

ngSwitch is just sugar around ngIf, so this is to be expected. You could use [hidden] or [style.display] as alternatives.

I don't think you should keep the metaphor of tabs here in mind. Trying to force that metaphor on different design paradigms should be avoided.

Given the psuedo-setup you provided in your stackblitz, it shouldn't be an issue for those components to get destroyed as the segments are toggled.

ryaa commented 3 years ago

I'm also considering to extend IonicRouteStrategy to enable avoiding to destroy one of the components in my app. In my case, the app has a list component, from which the user can drill into the detail view. The problem is that the detail view contains a map which is built using google maps and map destroy is not supported (https://issuetracker.google.com/issues/35821412#comment32). Map instances in a single page application should be reused and not destroyed then recreated so custom route strategy can be very handy here....

mhartington commented 3 years ago

@ryaa Provide a sample please.

AlecMRogers commented 3 years ago

I don't understand why the control of component caching is so difficult, since it severely impacts the performance of an application. I figured I would weigh in on this issue as another voice, if not an entirely different use case.

I am fairly new to ionic, but I've been working with javascript and Cordova for a long time. The inability to use the same component instance every time a page is loaded using the router is deeply frustrating: performance is negatively affected if I have to create a complex UI every time the page is loaded (as happens when displaying maps with overlays, etc, as mentioned above).

I don't really need a stack of components at all; using ion-router would be fine, but it seems that much of ionic navigation and UI (ion-slide-nav, ion-tabs, etc) will have to be abandoned if I do so.

-- Update (after looking at the source code a bit) --

Can I create a subclass of ion-router-outlet that uses a custom ComponentFactory to return a static component instance? That has the benefit of being compatible with other UI elements and also (AFAIK) not interfering with the navigation stack (and I am navigating from root in most cases anyway).

hinddeep commented 3 years ago

Still facing the same issue. Following is the info of my build environment:

Ionic:

Ionic CLI : 6.13.0 (/usr/local/lib/node_modules/@ionic/cli) Ionic Framework : @ionic/angular 5.5.1 @angular-devkit/build-angular : 0.1100.3 @angular-devkit/schematics : 11.0.3 @angular/cli : 11.0.3 @ionic/angular-toolkit : 2.3.3

Capacitor:

Capacitor CLI : 2.4.4 @capacitor/core : 2.4.4

Utility:

cordova-res (update available: 0.15.3) : 0.15.2 native-run : not installed

System:

NodeJS : v14.15.5 (/usr/local/bin/node) npm : 6.14.11 OS : macOS Big Sur (Intel based MacBook)

hunterjenkins commented 3 years ago

A portion of my app has a chat feature. Currently, when a user clicks on conversation they route to a conversation-detail page, with a list of message cells. Every time they do this, the component redraws all of the message cells, which is not optimally performant because it renders the cells each time. Big messaging apps like iMessage, Slack, etc do not do this as they cache the rendered cells.

From what I gather, Angular solves this issue with the RouteReuseStrategy. Is this possible to apply the same routing strategy in Ionic? I've tried many examples and can't get it to work.

Any guidance / suggestions would be super helpful! 😄