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

NativeScript router outlet implementation, component caching/reuse, and private APIs #7757

Closed
hdeshev opened this issue Mar 24, 2016 · 24 comments
Labels
area: router feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature freq2: medium state: Needs Design
Milestone

Comments

@hdeshev
Copy link

hdeshev commented Mar 24, 2016

Introduction

NativeScript's navigation framework uses a concept of a frame that can load different pages similar to the way frames/iframes work in a browser. The only difference is that a page that you navigate away from doesn't really get unloaded, but gets shown back immediately when the user navigates back to it.

Custom Router Outlet

To support this scenario and make it easy for users to navigate between pages and use animated transitions, we decided to build our own router outlet component (referred to as page-router-outlet from now on) which caches components you navigate away from, and restores them once you go back to the original page.

The Hack

That works fine so far, but it requires a hack on our end -- we need to explicitly set the parent router's _childRouter private field when navigating back to a component or that has been loaded before to avoid breaking subsequent navigations. Here's the offending code.

Solutions

We are looking for better ways to do this, something that will use supported API and doesn't break in future releases. I am not familiar with the future plans for the routing framework, but at this point even a public setChildRouter method looks good enough as it would guarantee that the page-router-outlet implementation does not break with future Angular 2 releases.

Steps to reproduce and a minimal demo of the problem

Requires a mobile app, but the issue isn't really mobile specific. One would probably face the same issue when building a routing solution that uses frames/iframes to load component content or implement a scheme that caches components and reuses them on back navigation.

Other information
I discussed this with @tbosch in person, and it would be great to have @btford's feedback as well.

Similar issues have been discussed before:

Ending this wall of text with a link to the detailed description of the way routing works in NativeScript.

@btford
Copy link
Contributor

btford commented Mar 25, 2016

Here's my take on it.

We want to add some sort of lifecycle management for a routable component that explains what to do with a component when it's removed due to a navigation, and also what to do when a component would be instantiated.

Here's one (admittedly kinda terse) way to add such an API:

@Routes([
  { path: '/a', name: 'A', component: A },
  { path: '/b', name: 'B', component: B }
])
@ChildRouteLifecycleManager(MyChildRouteLifecycleManager)
class App {}

class MyChildRouteLifecycleManager implements ChildRouteLifecycleManagerI {
  cache: Map<Type, ComponentRef> = {};

  // called when a component would be destroyed as part of a
  // navigation.
  onRouteAway(instr: Instruction, comp: ComponentRef): void {
    this.cache.set(instr.component.componentType, comp);
  }

  // called when a component would be instantiated as part of a
  // navigation.
  // returns ComponentRef, or null. null means "instantiate a new
  // component as normal." returning a ComponentRef means
  // "reattach this ref instead of instantiating a new component"
  onRouteTo(instr: Instruction): ComponentRef {
    return this.cache.get(instr.component.componentType) || null;
  }
}

Whenever App has a route change, after all the CanActivate, CanDeactivate, etc. hooks have run, during the "commit" phase of routing, we'd ask the provided ChildRouteLifecycleManager what to do with the component that's going to be removed, and what to do about getting a new instance.

We'd use a decorator here to scope this behavior just to App's immediate child routes. Components should not be reused like this across hierarchy changes. For example, navigating from A -> B -> C to A -> D -> C should not result in grandchild component C being cached and reused.

This would allow users to programmatically define their own caching behavior (local/global LRU cache, evict the cache when memory is low, or even using different caches for different platforms based on capabilities).

@vakrilov
Copy link

Providing such way to manage the lifecycle of components during navigation will be useful and will probably implement the need of the custom page-router-outlet. Which is welcome.

The other issue that needs to be addressed, is with the child-router when reusing a cached component.

Here is the scenario:
We have navigated to a component which has child router-outlet. We navigate away and then come back using the cached component instance. However, the child router-outlet inside the cached instance is already registered to a child router instance which was lost during the forward navigation. So to make it work, we need a way to bring back this child router instance when reusing the cached component-ref.
Currently, we are caching the old child router instance and setting it back in a really hacky way. It would be great if we had a way to do that in a more legal manner.

The thing to keep in mind when caching components is that you don't get a chance to reuse the DI as the constructor is already executed.

@hdeshev
Copy link
Author

hdeshev commented Mar 30, 2016

@btford, to add to what @vakrilov said above, I was interested in details concerning the ChildRouteLifecycleManager decorator solution you outlined above:

  • Will this land in Angular master any time soon? Will it make it for the 1.0 release?
  • Will it eliminate the need for our router outlet to set the _childRouter field on back navigation? Perhaps by doing it automatically for us. 😀
  • Will we be able to automatically register a lifecycle manager from the router outlet implementation so that we don't have to make all client apps add an extra line like @ChildRouteLifecycleManager(NativeScriptRouteLifecycleManager) to their route definitions?

@vakrilov
Copy link

Router v3.0.0 Update

We have migrated our custom router outlet to the v3 router. However, we are facing new set of challenges.

Background

For NativeScript we have implemented <page-router-outlet> that works in sync with the native navigation. The way that works is:

  • When navigating forward the current components are not destroyed they are kept on the current page. A new page is created to host the new component and the application navigates to that page.
  • When navigation backwards the current components are destroyed. However, instead of creating new component instance, native back navigation is executed which shows the previous page and the cached components that were on it.

Here is the page-router-outlet implementation spike. And here is a more detailed info about angular routing in NativeScript.

Although, there are NativeScript specifics, the essence here is persisting and reusing component instances when navigating. This question has been raised in several other threads (#6634, #5275, #7965) and other implementation might face the same problems.

Problems and Challenges

When navigating backwards - no new components are created. Instead they are revived form the cache.

ActivatedRoute

There is no way to re-inject the new instance of ActivatedRoute passed to the outlet activate() method. So any subscribers for the activated-route inside the component will be listening to an outdated activated route instance.

To overcome this we have created PageRoute class which contains an Observable<ActivatedRoute> which is updated on every back navigation. The PageRoute is injected in the component on initial navigation so that it has a way of getting hold of the actual activatedRoute/params.

If there is build-in angular way of persisting components - it will have to take care of updating the ActivatedRoute instance that has been already injected in a persisted component.

RouterOutletMap

There is no way to re-inject the routerOutletMap. In fact child outlets inside the cached component have already registered to the outlet-map injected into them the first time the component was navigated to. So we need cache the initial instance of the outlet map and restore its state into the instance that was given to the activate() method on the subsequent navigation.

Obviously not a sustainable solution. However, an API to merge/update RouterOutletMap might solve the problem.

We would appreciate any thoughts on that. Perhaps @vsavkin might have some opinion if we are going in the right direction or the future version of the router will help us clear some of the hacks we currently have in the code.

@VladimirAmiorkov
Copy link

Hi,

It may have been said before but here is the issues I have faced. They are not NativeScript only issues but are easily observed in {N}.

Background

Lets say you have an application that will have an infinite navigation or at least unknown level deep one. For example you have a list that shows items, each time you click on an item it navigates forward to a Page which again shows a list of items etc.

The way one may handle such navigation is to create an "List" component that will be 'reused' for each navigation and when an item is the Page is pressed simply navigate to it via this._router.navigate(['/list', "Level 2", "newItems"]); and pass the necessary args. The same component will use its ngOnInit to subscribe to the ActivatedRoute in order to retrieve the passed params and use them to make calls to a backend for the items it needs to show in a List. Finally we need to make sure the "List" component is never reused as it will interfere with the application navigation stack. This in RC1 was done with overriding the CanReuse of the "List" component:

routerCanReuse(nextInstruction: any, prevInstruction: any): boolean {       
     return false;      
}

Problems and Challenges

Ever since the RC2, RC3 releases the above approach no longer possible since CanReuse was removed.

Solutions

Currently we are making "dummy" components which is identical copies of the "List" component but simply have a different class name and use them to declare additional routes in the RouterConfig. This is not optional as it does not allow for infinite navigation as desired.

It would be great if there is a way to override the default 'should reuse' condition of each Component. This will make it possible to create a Component that is never or only conditionally reused rather than count on Angular to automatically resolve if the component should be reused.

@vicb
Copy link
Contributor

vicb commented Jul 19, 2016

@VladimirAmiorkov do you think we should re-open this issue ?

/cc @vsavkin

@VladimirAmiorkov
Copy link

@vicb As this issues describes multiple concerns and challenges I think each of those should be addressed. From my experience the major one is the non availability of overriding if and when and component will be reused. This is a showstopper for scenarios where you would like to share the code of a component to multiple "pages" but want to achieve fill navigation history. The old CanReuse functionality was exactly what I was expecting to see in a angular component.

You could reopen this issue or I can create a new one it doesn't matter all that matters if such functionality is or will be available in angular 2.

@tbosch tbosch reopened this Jul 26, 2016
@tbosch
Copy link
Contributor

tbosch commented Jul 26, 2016

/cc @mhevery

@vsavkin vsavkin self-assigned this Jul 27, 2016
@vsavkin
Copy link
Contributor

vsavkin commented Aug 1, 2016

This is my thinking on the issue.

Current State:

When performing navigation, the router does the following:

parseUrl => applyRedirects => recognize => createRouterState => runGuards => activate

This is the signature of createRouterState:

function createRouterState(curr: RouterStateSnapshot, prevState: RouterState): RouterState {...}

Currently, the createRouterState is not customizable. As a result, you cannot "reuse" activated routes.

Should Reuse:

We were planning to implement support for custom "reuse" strategies. But their purpose is different. It is to make components even more short-lived that they are right now. Currently, we will reuse the same component instance when its params change. The reuse strategy would allow you to get a new component instance on any params change.

Possible Solutions (before final):

We are not planning to make the createRouterState step customizable before final.

Possible Solutions (after final):

After the final is out, we can do the following:

  • make the createRouterState step customizable
  • make the instantiation of components customizable, so instead of newing them directly in RouterOutlet, the router will go through a service. You will be able to implement any caching strategy you want there.

@vakrilov
Copy link

vakrilov commented Aug 2, 2016

Thanks for the insights @vsavkin!

Looks like the should reuse feature will solve @VladimirAmiorkov case.

For the caching problem - I guess we have to wait until there is a way to implement custom create/cache strategy for NativeScript page navigation.

@EricABC
Copy link

EricABC commented Aug 3, 2016

Would really appreciate that RouterOutlet component instantiation hook (the whole thing is sounding a lot like lifecycle hooks for the routes themselves). Currently I'm using a hack 'ComponentOutlet' to handle mapping state into components (mapping data/resolvers to InputMetadata in some cases, auto-wiring up FormControls to parent groups/arrays in others) that don't know they have dynamic route state being mapped to them. Separating the concerns of state mapping from the components themselves seems like it would be another side effect of this.

@vsavkin vsavkin removed their assignment Aug 8, 2016
@toddwong
Copy link

I personally prefer a page stack (push/pop/replace) than a page cache

@jakeNiemiec
Copy link

Now that final is out, how has the situation changed?

Any information I find on this seems to be from before modules and angular 2.0.0 final.

@fredgate
Copy link

I have also problem when implementing a custom router outlet that cache components. I can have the same route activated many times with different parameters : users/foo, users/bar.
My problem is that the router automatically reuse active component if it has the same route (/users/:id). If my activate component is for url /users/foo and I click on link with url /users/foo the router does not call activate method of my custom router outlet but update the ActivatedRoute of the active component.
I will want to tell to router to not reuse components (for route /users/:id) and then it always call deactivate and activate methods of my custom router outlet. My custom router outlet redisplay the component if it is cached, else it create a new component.

@lubos-bistak
Copy link

Current version still missing similar functionality to 'CanReuse.routerCanReuse -> return false' .
Any plans for some configuration / feature to disable Component 'reuse' by navigating to same route (e.g. /path1 ->/path1)?

@DzmitryShylovich
Copy link
Contributor

#13124

@VladimirAmiorkov
Copy link

@DzmitryShylovich Now we only need an example and some docs, thanks for sharing this PR

@DzmitryShylovich
Copy link
Contributor

DzmitryShylovich commented Dec 1, 2016

Blog post is coming soon https://twitter.com/victorsavkin/status/804128642317983744

@michalstepien
Copy link

I created issue with this
manfredsteyer/angular-2-reuse-strategy-sample#1

@pilot911
Copy link

any news on @michalstepien comment ?

@ngbot ngbot bot added this to the Backlog milestone Jan 23, 2018
@MHillier98
Copy link

Not sure if I've encountered the exact same issue, but I've been having a lot of issues with components not being destroyed when navigating through pages. i.e. /form/1 to /form/2 kept subscriptions alive.

I'm a bit surprised to see this one still open after so many years 😕

@angular-robot angular-robot bot added the feature: under consideration Feature request for which voting has completed and the request is now under consideration label May 21, 2021
@atscott
Copy link
Contributor

atscott commented Jun 28, 2022

Closing as obsolete. RouterOutletContract now supports custom outlets without any hacks.

@atscott atscott closed this as completed Jun 28, 2022
@atscott atscott modified the milestones: Backlog, Fixit H1'2022 Jun 28, 2022
@angular-automatic-lock-bot
Copy link

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.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jul 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: router feature: under consideration Feature request for which voting has completed and the request is now under consideration feature Issue that requests a new feature freq2: medium state: Needs Design
Projects
None yet
Development

No branches or pull requests