feat(router): add support for custom route reuse strategies #13124

vsavkin · 2016-11-29T00:56:45Z

No description provided.

vicb · 2016-11-29T22:26:45Z

modules/@angular/router/src/route_reuse_strategy.ts

+export type DetachedRouteTree = typeof Object;
+
+/**
+ * @whatItDoes Provides a way to customize when and activated routes get reused.


and -> an / gets

vicb · 2016-11-29T22:27:41Z

modules/@angular/router/src/route_reuse_strategy.ts

+  /**
+   * Determines if this route (and its subtree) should be detached to be reused later.
+   */
+  abstract shouldDettach(route: ActivatedRouteSnapshot): boolean;


detach (single t)

vicb · 2016-11-29T22:30:29Z

modules/@angular/router/src/router.ts

@@ -287,6 +288,16 @@ type NavigationParams = {
  promise: Promise<boolean>
 };

+export class DefaultRouteReuseStrategy implements RouteReuseStrategy {


Add a comment describe the strategy, ie "// Reuses only when the same route is activated"

vicb · 2016-11-29T22:30:59Z

modules/@angular/router/src/router.ts

@@ -326,6 +337,8 @@ export class Router {
   */
  urlHandlingStrategy: UrlHandlingStrategy = new DefaultUrlHandlingStrategy();

+  routeReuseStrategy: any = new DefaultRouteReuseStrategy();


vicb · 2016-11-29T22:40:28Z

modules/@angular/router/src/router.ts

+      route: TreeNode<ActivatedRoute>, parentOutletMap: RouterOutletMap): void {
+    const outlet = getOutlet(parentOutletMap, route.value);
+    const componentRef = outlet.detach();
+    this.routeReuseStrategy.set(route.value.snapshot, <any>{componentRef, route});


is <any> required here ?

vicb · 2016-11-29T22:45:55Z

modules/@angular/router/src/directives/router_outlet.ts

@@ -67,6 +67,13 @@ export class RouterOutlet implements OnDestroy {
    return this._activatedRoute;
  }

+  detach(): ComponentRef<any> {
+    this.activated = null;
+    return this.activated;


needs test (& fix)

vicb · 2016-11-29T22:46:52Z

modules/@angular/router/src/directives/router_outlet.ts

+    return this.activated;
+  }
+
+  reattach(ref: ComponentRef<any>) { this.activated = ref; }


Need to update the activated route ?

vicb · 2016-11-29T22:51:02Z

modules/@angular/router/src/router.ts

+
+        if (this.routeReuseStrategy.shouldAttach(future.snapshot)) {
+          const stored = (<any>this.routeReuseStrategy.get(future.snapshot));
+          outlet.reattach(stored.ref);


componentRef ?

vicb · 2016-11-29T22:52:21Z

modules/@angular/router/src/router.ts

@@ -1151,6 +1191,11 @@ class ActivateRoutes {
  }
 }

+function advancedActivatedRouteNodeAndItsChildren(node: TreeNode<ActivatedRoute>): void {


advanceActivatedRouteNodeAndItsChildren (drop the first d)

vicb · 2016-11-29T23:00:39Z

modules/@angular/router/src/create_router_state.ts

    return new TreeNode<ActivatedRoute>(value, children);

+  } else if (!!routeReuseStrategy.get(curr.value)) {


useless !!

not sure to get the logic here

vicb · 2016-11-29T23:11:38Z

modules/@angular/router/src/route_reuse_strategy.ts

+ *
+ * @experimental
+ */
+export type DetachedRouteTree = typeof Object;


finer type ?

Introduce the inner type

Could we make it more explicit ?
Would it help with the cast DRT -> DRTI ?

export interface DRT {} export interface DRTI extends DRT { // ... }

vicb · 2016-11-29T23:13:30Z

modules/@angular/router/test/integration.spec.ts

@@ -31,6 +31,7 @@ describe('Integration', () => {
     fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
       const fixture = createRoot(router, RootCmp);

+


nit: revert

vicb · 2016-11-29T23:15:58Z

modules/@angular/router/test/integration.spec.ts

@@ -2392,6 +2393,108 @@ describe('Integration', () => {
         })));
    });
  });
+
+  describe('Custom Route Reuse Strategy', () => {
+    class AttacheDetachReuseStrategy implements RouteReuseStrategy {


Attache no e but may be reuseStrategy ?

vicb · 2016-11-29T23:17:54Z

modules/@angular/router/test/integration.spec.ts

+          return false;
+        } else {
+          let equal = true;
+          Object.keys(future.params).forEach(k => {


let equal = Object.keys(future.params).every(...)

vicb · 2016-11-29T23:18:37Z

modules/@angular/router/test/integration.spec.ts

+      }
+    }
+
+    it('should support attaching & detachmenting fragments',


"detachmenting" :)

vicb · 2016-11-30T02:09:48Z

modules/@angular/router/src/router.ts

@@ -287,6 +288,20 @@ type NavigationParams = {
  promise: Promise<boolean>
 };

+
+/**
+ * Does not detach any subtress. Reuses routes as long as their route config is the same.


vicb · 2016-11-30T02:13:21Z

modules/@angular/router/src/create_router_state.ts

+function createNode(
+    routeReuseStrategy: RouteReuseStrategy, curr: TreeNode<ActivatedRouteSnapshot>,
+    prevState?: TreeNode<ActivatedRoute>): TreeNode<ActivatedRoute> {
+  if (prevState && routeReuseStrategy.shouldReuseRoute(curr.value, prevState.value.snapshot)) {


vicb · 2016-11-30T02:14:01Z

modules/@angular/router/src/create_router_state.ts

    return new TreeNode<ActivatedRoute>(value, children);

+  } else if (routeReuseStrategy.retrieve(curr.value)) {
+    const tree: TreeNode<ActivatedRoute> = (<any>routeReuseStrategy.retrieve(curr.value)).route;


use the internal type ?

vicb · 2016-11-30T02:15:40Z

modules/@angular/router/src/create_router_state.ts

+    throw new Error('Cannot reattach ActivatedRouteSnapshot created from a different route');
+  }
+  if (curr.children.length !== result.children.length) {
+    throw new Error('Cannot reattach ActivatedRouteSnapshot with a different number of children');


not sure about this check ?

vicb · 2016-11-30T02:18:58Z

modules/@angular/router/src/directives/router_outlet.ts

@@ -67,11 +67,26 @@ export class RouterOutlet implements OnDestroy {
    return this._activatedRoute;
  }

+  detach(): ComponentRef<any> {
+    this.location.detach(0);


I would not specify the optional 0 (same for insert)
Makes me think there are more than 1 view.

vicb · 2016-11-30T02:19:56Z

modules/@angular/router/src/directives/router_outlet.ts

+  attach(ref: ComponentRef<any>, activatedRoute: ActivatedRoute) {
+    this.activated = ref;
+    this._activatedRoute = activatedRoute;
+    this.location.insert(ref.hostView, 0);


assert length == 0 ?

vicb · 2016-11-30T02:21:20Z

modules/@angular/router/src/route_reuse_strategy.ts

+export type DetachedRouteTree = typeof Object;
+
+/**
+ * @internal


add a comment (on DetachRouteTree ?) opaque vs internal

vicb · 2016-11-30T02:24:32Z

modules/@angular/router/src/router.ts

+      route: TreeNode<ActivatedRoute>, parentOutletMap: RouterOutletMap): void {
+    const outlet = getOutlet(parentOutletMap, route.value);
+    const componentRef = outlet.detach();
+    this.routeReuseStrategy.store(route.value.snapshot, <any>{componentRef, route});


remove <any> ?

vicb · 2016-11-30T02:28:08Z

modules/@angular/router/src/router.ts

-        this.activateChildRoutes(futureNode, null, outletMap);
+
+        if (this.routeReuseStrategy.shouldAttach(future.snapshot)) {
+          const stored: DetachedRouteTreeInternal =


can we say const stored = <DetachedRouteTreeInternal>this.routeReuseStrategy.retrieve(future.snapshot); ?

vicb · 2016-11-30T03:28:24Z

modules/@angular/router/src/create_router_state.ts

+  } else if (routeReuseStrategy.retrieve(curr.value)) {
+    const tree: TreeNode<ActivatedRoute> = (<any>routeReuseStrategy.retrieve(curr.value)).route;
+    setFutureSnapshotsOfActivatedRoutes(curr, tree);
+    return tree;


question: should we store null at this point ?
/bookmark 1

vicb · 2016-11-30T03:30:21Z

modules/@angular/router/src/router.ts

+    if (this.routeReuseStrategy.shouldDetach(route.value.snapshot)) {
+      this.detachAndStoreRouteSubtree(route, parentOutletMap);
+    } else {
+      this.deactiveRouteAndOutlet(route, parentOutletMap);


/ref bookmark 1
or store null here so that we never re-use an outdated value ?

vicb · 2016-11-30T03:31:16Z

modules/@angular/router/src/directives/router_outlet.ts

@@ -67,11 +67,26 @@ export class RouterOutlet implements OnDestroy {
    return this._activatedRoute;
  }

+  detach(): ComponentRef<any> {
+    this.location.detach(0);


if (this.activated) ...

vsavkin · 2016-11-30T07:22:03Z

Got an lgtm from vicb

vicb · 2016-11-30T17:21:29Z

modules/@angular/router/src/directives/router_outlet.ts

+  attach(ref: ComponentRef<any>, activatedRoute: ActivatedRoute) {
+    this.activated = ref;
+    this._activatedRoute = activatedRoute;
+    this.location.insert(ref.hostView);


assert empty before insert ?

rmcsharry · 2017-01-08T17:44:02Z

I see that there is a basic blog post about this here, but it's rather basic and seems to indicate the the reuse strategy is implemented application wide.

Has anyone written a more simple guide than this? Or a way to implement reuse for one specific component only?

tjaartvanderWalt · 2017-01-11T11:01:31Z

@rmcsharry I am also looking for a more detailed post/example. The implementation found here breaks when I have paths that contain child paths, I have opened an issue 13869. I have also posted the issue on SO

m98 · 2017-11-18T13:57:28Z

This would be nice if the document could have an example about how to properly use RouteReuseStrategy

For example, can that be helpful for such situation:

Router:

{path: 'news/:id', component: NewsComponent},  // to read one news
{path: 'news/:id/image/:image-id', component: NewsComponent},  // to see one news image by the id

Now if you click on the image (ex: news/123/image/5) the image opens by the logic we have in the component, but the component also re-initialize which means it will rerun the component methods which will retrieve news data over HTTP request.

How can I use RouteReuseStrategy for this case?

sandangel · 2017-11-19T22:06:09Z

Don’t know why they miss this in documentation.

FKholbutayev · 2018-10-19T08:43:39Z

Facing the same issue @m98, do they have any updates?

angular-automatic-lock-bot · 2019-09-14T05:21:49Z

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.}

googlebot added the cla: yes label Nov 29, 2016

vsavkin force-pushed the router_caching branch from 98098e5 to cf5e581 Compare November 29, 2016 18:45

vsavkin changed the title ~~wip~~ feat(router): add support for custom route reuse strategies Nov 29, 2016

vsavkin added the action: review The PR is still awaiting reviews from at least one requested reviewer label Nov 29, 2016

vsavkin assigned vicb Nov 29, 2016

vicb reviewed Nov 29, 2016

View reviewed changes

modules/@angular/router/test/integration.spec.ts

}

}

it('should support attaching & detachmenting fragments',

Copy link

Contributor

vicb Nov 29, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"detachmenting" :)

vsavkin force-pushed the router_caching branch from cf5e581 to d301b19 Compare November 30, 2016 02:05

vicb reviewed Nov 30, 2016

View reviewed changes

vsavkin force-pushed the router_caching branch 2 times, most recently from eecd53a to 42cf06f Compare November 30, 2016 07:21

feat(router): add support for custom route reuse strategies

42cf06f

vsavkin added pr_state: LGTM and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Nov 30, 2016

vsavkin merged commit 42cf06f into angular:master Nov 30, 2016

vicb reviewed Nov 30, 2016

View reviewed changes

zoechi mentioned this pull request Apr 5, 2017

Programmatically prevent the destruction of components when requested #5275

Closed

FKholbutayev unassigned vicb Oct 19, 2018

angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 14, 2019

		return new TreeNode<ActivatedRoute>(value, children);

		} else if (!!routeReuseStrategy.get(curr.value)) {

		@@ -31,6 +31,7 @@ describe('Integration', () => {
		fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
		const fixture = createRoot(router, RootCmp);

feat(router): add support for custom route reuse strategies #13124

feat(router): add support for custom route reuse strategies #13124

Uh oh!

Conversation

vsavkin commented Nov 29, 2016

Uh oh!

vicb Nov 29, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vsavkin commented Nov 30, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rmcsharry commented Jan 8, 2017

Uh oh!

tjaartvanderWalt commented Jan 11, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

m98 commented Nov 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

vicb Nov 29, 2016 •

edited

Loading

tjaartvanderWalt commented Jan 11, 2017 •

edited

Loading

m98 commented Nov 18, 2017 •

edited

Loading