angular / angular

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

Refine RouteReuseStrategy #27290

Open thorn0 opened 5 years ago

thorn0 commented 5 years ago

🚀 RouteReuseStrategy 2.0

Relevant Package

This feature request is for @angular/router

Description

The current RouteReuseStrategy API is totally confusing and probably broken.

Describe the solution you'd like

In order for this API to become usable, at least the following problems have to be solved:

Describe alternatives you've considered

Have you considered any alternative solutions or workarounds? The current API makes you use a lot of workarounds, and still you cannot be sure whether you've done everything right. E.g. see [my attempt to implement](https://stackblitz.com/github/thorn0/angular-router-experiments?file=src%2Fapp%2Fcustom-reuse-strategy.ts) the greedy reuse strategy.
jasonaden commented 5 years ago

This is a great summary of issues. Thanks for it!

We have some recent requirements where it looks like we need to provide better APIs allowing the developer to decide when to destroy components created by the router. This is specifically for mobile where we need to support transitions with gestures such as dragging from the side of the app left-to-right to go back, and both views need to be on screen at the same time. We have a way to do this with current APIs, but similar to what you described there's a problem when destroying.

Can you (or anyone else watching this issue) provide some use cases on how you would like to use any updates to these APIs? Getting those use cases listed helps us work out a better API more quickly and can help get an issue resolved faster.

thorn0 commented 5 years ago

It would be really great if there were a working example of a route reuse strategy that tries reusing as much as it can. Having such a reference / starting point, it should be simple to build more selective reuse strategies.

I tried to implement such a strategy using the current API, but I couldn't. The closest result I've managed to achieve is here. It seems to work, but try navigating dashboard → pages/A → dashboard → pages/A. You'll see that the parent PagesComponent instance on the pages/A page is not reused. There seems to be no way to solve this while it looks like a pretty basic use case: caching the whole component hierarchy on the page, not the innermost route only. But imagine we managed to find a solution somehow. Now, what if we go dashboard → pages/A → dashboard → pages/B? Should the instance of PagesComponent stored after visiting pages/A be reused for pages/B or should a new one be created? I guess both use cases are valid and need to be supported.

As for use cases, the most common ones must be all kinds of lists. Lists might contain search results, sophisticated filters, scrollable areas, and other complex state that we don't want to lose on navigation. Returning back to the list, the user often expects it 1) to be in the same state it was left in, 2) to appear instantly. That's where reusable routes should shine.

jasonaden commented 5 years ago

@thorn0 Okay, thanks for the update here. I'll take a look into this a bit further and post an update here.

FDIM commented 5 years ago

I've worked on such feature for old ui router (pre v1) and had this PR open: https://github.com/angular-ui/ui-router/pull/2333 - it has some details / problems there (e.g. scroll position).

My idea there was pretty much how @thorn0 described. I would also expect if parent component / route is cached it should be reused when navigation affecting only child component happens.

Additionally it was also possible to control when cache should be cleared inside the component and I think such behavior would be best and most flexible (e.g. you could easily start a timer inside detach handler). So, in this case how about passing an extra option (cleanup function) to ngOnDetach ? or maybe even better provide ComponentCache value that would let you do the same? Clearing cached component should also make sure that all child components are released from cache as well if there are more levels.

GuilhermeBCC commented 5 years ago

I have a problem that can be added to this list. When you detach a page/component all Custom Elements simply disappear (of course they are destroyed when deattach from DOM). That way they should be recreated somehow when the page/component is reattached.

FDIM commented 5 years ago

Could you clarify what is a custom element in your use case?

GuilhermeBCC commented 5 years ago

web components (https://w3c.github.io/webcomponents/spec/custom/), Custom Elements created with Angular Elements.

pcurrivan commented 5 years ago

I've also been working on a reuse strategy that tries to reuse as much as possible. Below are my thoughts on why this is problematic with the current API. It's not impossible to make a reuse strategy that reuses as much as possible, but it does require some annoying-at-best hacks.

My starting point was the strategy posted by @dmitrimaltsev here. The main idea is to break each navigation into a sequence of navigations depending on how the parts of the target url are currently stored in the detached route cache. This is necessary because a detached subtree can only be reattached as a whole. (If route A is stored with subtree A/A1/A1.1, then to reattach A, you must reattach the whole subtree A/A1/A1.1.) You need roughly one extra navigation for each detached subtree you need to reattach.

However, in order to accomplish this (breaking a navigation into a sequence of navigations), one needs to be able to override the router's navigation behavior.

@dmitrimaltsev did it by dynamically changing redirects in the router config and creating a custom routerLink directive. The issues with that are 1) fiddling with redirects requires some boilerplate in the route configs, 2) routerLink and routerLinkActive are complicated and rewriting them is not ideal, 3) controlling navigation via interacting dynamic redirects and custom router links is overly complicated, and 4) routing initiated by other means than the custom router link, e.g. changing the url when hash-based routing is enabled, will bypass the custom navigation behavior.

The only other option I am aware of by which a developer can override the navigation behavior is by adding a route guard to every single route. However this is a mess because 1) it involves a lot of boilerplate on the route config, and 2) you have to somehow distinguish the initial, user-generated navigation request from the "real" navigation steps generated by the route guard.

Whichever way you manage to override the navigation behavior to properly reattach detached route subtrees, this strategy of using a sequence of navigations has the flaw that you are actually doing a sequence navigations. i.e. the url and view actually will flicker as the algorithm goes through the steps of the navigation. There is no way I know of to do a navigation that reattaches a stored subtree but does not render anything.

To make a decent reuse-everything reuse strategy possible, you guys need to provide a way for a developer to change navigation behavior without hacks like custom router links or guards applied to all routes. As part of that you also need to provide a way to reattach a route as part of a navigation process without rendering anything. Or alternatively just do all this internally within the router and provide an API option to reuse routes.


Here's a sketch of the algorithm for navigating and reattaching any stored routes along the way.

nav(targetUrl): Promise {
  if (targetUrl === getCurrentUrl()) // navigation is done
    return render();
  const firstStoredSubtree = getFirstStoredSubtree(targetUrl);
  if (firstStoredSubtree == null)
    return navigate(targetUrl); // no more stored subtrees, so just complete the navigation
  else
    return navigateWithoutRendering(getFullPath(firstStoredSubtree)).then(() => nav(targetUrl)); // reattach subtree and then try navigation again
}

Here's an example run of the algorithm. The route naming scheme should be self-explanatory.

stored subtrees: A/A1/A1.1, A2/A2.1, A2.2/A2.2.1
navigating from B -> A2.2.1 (full path: A/A2/A2.2/A2.2.1), the steps are:
1) Try nav. A is stored with subtree A/A1/A1.1, so navigate without rendering to full path A/A1/A1.1.  
2) Try nav again. A2 is stored with subtree A2/A2.1, so navigate without rendering to full path A/A2/A2.1.
3) Try nav again. A2.2 is stored with subtree A2.2/A2.2.1, so navigate without rendering to full path A/A2/A2.2/A2.2.1.
4) Try nav again. Current route equals target route, so render.

(Currently there is no such thing as navigateWithoutRendering, so the above run would cause the view and url to flicker from A1.1 to A2.1 to A2.2.1)

Serginho commented 5 years ago

We do you plan to do this?

sedll7809 commented 5 years ago

hi thorn0 I use your code, all is normal, but I don't know how to delete the snapshot when I close TAB, please tell me how to delete the snapshot.

RouteReuseStrategy 2.0

Relevant Package

This feature request is for @angular/router

Description

The current RouteReuseStrategy API is totally confusing and probably broken.

Describe the solution you'd like

In order for this API to become usable, at least the following problems have to be solved:

  • 25521: Introduce ngOnAttach/ngOnDetach lifecycle hooks

  • 20114: impossible to store/retrieve siblings AND ALSO a non-sibling

  • 20072: RouteReuseStrategy doesn't reuse the parent tree components

  • Why is retrieve called before shouldAttach?
  • There is no 'official' way to properly dispose cached routes. DetachedRouteHandle is said to be 'opaque', but to dispose a route one has to use the content of this structure: (handle as any).componentRef.destroy(). Does this call suffice to dispose a route? Why not add a public API for this? Also a (handle as any).componentRef.hostView.destroyed check has to be used in retrieve because of #20072 to avoid reattaching a destroyed component.
  • Documentation. More than one sentence for every method and a clear description of their call sequence are needed.
  • Code examples (working).
  • Ship alternative ready-to-use implementations with the router, not only DefaultRouteReuseStrategy. E.g. a greedy strategy that tries to cache and reuse all the routes.

Describe alternatives you've considered

Have you considered any alternative solutions or workarounds? The current API makes you use a lot of workarounds, and still you cannot be sure whether you've done everything right. E.g. see my attempt to implement the greedy reuse strategy.

RouteReuseStrategy 2.0

Relevant Package

This feature request is for @angular/router

Description

The current RouteReuseStrategy API is totally confusing and probably broken.

Describe the solution you'd like

In order for this API to become usable, at least the following problems have to be solved:

  • 25521: Introduce ngOnAttach/ngOnDetach lifecycle hooks

  • 20114: impossible to store/retrieve siblings AND ALSO a non-sibling

  • 20072: RouteReuseStrategy doesn't reuse the parent tree components

  • Why is retrieve called before shouldAttach?
  • There is no 'official' way to properly dispose cached routes. DetachedRouteHandle is said to be 'opaque', but to dispose a route one has to use the content of this structure: (handle as any).componentRef.destroy(). Does this call suffice to dispose a route? Why not add a public API for this? Also a (handle as any).componentRef.hostView.destroyed check has to be used in retrieve because of #20072 to avoid reattaching a destroyed component.
  • Documentation. More than one sentence for every method and a clear description of their call sequence are needed.
  • Code examples (working).
  • Ship alternative ready-to-use implementations with the router, not only DefaultRouteReuseStrategy. E.g. a greedy strategy that tries to cache and reuse all the routes.

Describe alternatives you've considered

Have you considered any alternative solutions or workarounds? The current API makes you use a lot of workarounds, and still you cannot be sure whether you've done everything right. E.g. see my attempt to implement the greedy reuse strategy.

lijolijojohn commented 5 years ago

I have a problem that can be added to this list. When you detach a page/component all Custom Elements simply disappear (of course they are destroyed when deattach from DOM). That way they should be recreated somehow when the page/component is reattached.

I also face the same issue. The angular custom elements are not getting cached when using RouteReuseStrategy. I use this code. Please help me. I am fully stuck here. All other components are getting cached except the custom elements. export class CustomRouteReuseStrategy implements RouteReuseStrategy { // Specify the routes to reuse/cache in an array. routesToCache: string[] = ["salesordertracker"]; storedRouteHandles = new Map<string, DetachedRouteHandle>(); // Decides if the route should be stored shouldDetach(route: ActivatedRouteSnapshot): boolean { return this.routesToCache.indexOf(this.getPath(route)) > -1; } //Store the information for the route we're destructing store(route: ActivatedRouteSnapshot, handle: DetachedRouteHandle): void { this.storedRouteHandles.set(this.getPath(route), handle); } //Return true if we have a stored route object for the next route shouldAttach(route: ActivatedRouteSnapshot): boolean { return this.storedRouteHandles.has(this.getPath(route)); } //If we returned true in shouldAttach(), now return the actual route data for restoration retrieve(route: ActivatedRouteSnapshot): DetachedRouteHandle { return this.storedRouteHandles.get(this.getPath(route)) as DetachedRouteHandle; } //Reuse the route if we're going to and from the same route shouldReuseRoute(future: ActivatedRouteSnapshot, curr: ActivatedRouteSnapshot): boolean { if(future.component && (future.component).name == "SalesProjectComponent" ){ return false; } return future.routeConfig === curr.routeConfig; } // Helper method to return a path, // since javascript map object returns an object or undefined. private getPath(route: ActivatedRouteSnapshot): string { let path = ""; if (route.routeConfig != null && route.routeConfig.path != null) path = route.routeConfig.path; return path; }

}
Da13Harris commented 4 years ago

My use case is for a PWA that has it's own internal dynamic tabs. Within each dynamic tab, there are component instances (pages) that need to be kept independent of each other. These page components also instantiate their own services in order to keep their state independent of each other.

The custom route reuse strategy is used to store and retrieve the DOM tree for whichever component is routed to within each tab. For example, you can open a tab and route to something like example.com/tab/1/fiscal/receipts, do some work, then open a new tab, route to something like example.com/tab/2/contact/publications, then switch back and forth between the two tabs. In order to maintain state, those components and their children are stored and retrieved using the custom route reuse strategy.

Initially there were a lot of challenges building out a workable solution with the current API, particularly due to the way each route segment gets processed separately in a very imperative way. If I'm being honest, I'd strongly prefer if the route reuse strategy could be executed in a single pass with knowledge of the entire route hierarchy. That way we could potentially leverage the whole route tree in a more declarative way to get the desired behavior.

Another problem we faced was due to what seems to be a bug where retrieve sometimes gets called without the protection of the shouldAttach method. We worked around that by calling shouldAttach again before returning from the retrieve method. Yuck!

Despite these initial issues, we were ultimately able to achieve our primary goals. 🎉

Once we got there, however, we started (and continue) to face the next difficult challenge. When the route is retrieved and reattached, it seems that any child services that rely on dependency injection are unable to hold state data. I believe this is due to issue #20072, because new instances of the services seem to be created when they are reattached. This is unfortunate, because it makes us even more reliant on route handlers to keep state in our components, rather than simply putting it in a shared service. We also usually end up having to avoid further nesting of child routes, because state data that can't be held in the services also can't be passed in through the child <router-outlet>.

We keep managing workarounds, but would love to see some of these issues addressed.

Thanks for reading!

TheNemus commented 4 years ago

This is a great summary of issues. Thanks for it!

We have some recent requirements where it looks like we need to provide better APIs allowing the developer to decide when to destroy components created by the router. This is specifically for mobile where we need to support transitions with gestures such as dragging from the side of the app left-to-right to go back, and both views need to be on screen at the same time. We have a way to do this with current APIs, but similar to what you described there's a problem when destroying.

Can you (or anyone else watching this issue) provide some use cases on how you would like to use any updates to these APIs? Getting those use cases listed helps us work out a better API more quickly and can help get an issue resolved faster.

My use case is to cache components from lazy-loaded module, and use routes transition animations: if the component is stored, the transition goes very smooth. If I need to re-create the component each time its route has activated, the animation becomes laggy.

christoph-hue commented 1 year ago

We use a tabbed-approach to allow navigating between multiple views without reloading each view entirely on tab-activation. to accomplish this, we use a route-reuse-strategy. when I use a customElement in one of the views, the customElement is rendered correctly only on first navigation to this view. navigating somewhere else and then going back to the previous view makes the customElement disappear completely. see this Stackblitz-sample for a minimal reproduction.

so... yes, please fix this!

da201501245 commented 1 year ago

We use a tabbed-approach to allow navigating between multiple views without reloading each view entirely on tab-activation. to accomplish this, we use a route-reuse-strategy. when I use a customElement in one of the views, the customElement is rendered correctly only on first navigation to this view. navigating somewhere else and then going back to the previous view makes the customElement disappear completely. see this Stackblitz-sample for a minimal reproduction.

so... yes, please fix this!

is there any workaround for this?

da201501245 commented 1 year ago

I have a problem that can be added to this list. When you detach a page/component all Custom Elements simply disappear (of course they are destroyed when deattach from DOM). That way they should be recreated somehow when the page/component is reattached.

I also face the same issue. The angular custom elements are not getting cached when using RouteReuseStrategy. I use this code. Please help me. I am fully stuck here. All other components are getting cached except the custom elements. export class CustomRouteReuseStrategy implements RouteReuseStrategy { // Specify the routes to reuse/cache in an array. routesToCache: string[] = ["salesordertracker"]; storedRouteHandles = new Map<string, DetachedRouteHandle>(); // Decides if the route should be stored shouldDetach(route: ActivatedRouteSnapshot): boolean { return this.routesToCache.indexOf(this.getPath(route)) > -1; } //Store the information for the route we're destructing store(route: ActivatedRouteSnapshot, handle: DetachedRouteHandle): void { this.storedRouteHandles.set(this.getPath(route), handle); } //Return true if we have a stored route object for the next route shouldAttach(route: ActivatedRouteSnapshot): boolean { return this.storedRouteHandles.has(this.getPath(route)); } //If we returned true in shouldAttach(), now return the actual route data for restoration retrieve(route: ActivatedRouteSnapshot): DetachedRouteHandle { return this.storedRouteHandles.get(this.getPath(route)) as DetachedRouteHandle; } //Reuse the route if we're going to and from the same route shouldReuseRoute(future: ActivatedRouteSnapshot, curr: ActivatedRouteSnapshot): boolean { if(future.component && (future.component).name == "SalesProjectComponent" ){ return false; } return future.routeConfig === curr.routeConfig; } // Helper method to return a path, // since javascript map object returns an object or undefined. private getPath(route: ActivatedRouteSnapshot): string { let path = ""; if (route.routeConfig != null && route.routeConfig.path != null) path = route.routeConfig.path; return path; }

}

I am also stuck here can you please help me if you solve problem anyway?

raegen commented 8 months ago

A bit late to the party but, having had my own brawl with this stuff years ago, thought might be worth adding a mention. Back then I published a package that addresses some of the aforementioned points. https://www.npmjs.com/package/@raegen/mountable The thing is, it's 3 years old and I'm pretty sure some of the stuff involved doesn't work any longer, but, I believe it might be useful for ideas/references.

huajian123 commented 2 months ago

@da201501245 is there any workaround for this?

huajian123 commented 2 months ago

@christoph-hue is there any workaround for this?

zhuzhuxia5875102 commented 2 months ago

@da201501245 is there any workaround for this?

zhuzhuxia5875102 commented 2 months ago

@christoph-hue is there any workaround for this?