Ionic-framework: The suggested "guard" approach with ionViewCanEnter/ionViewCanLeave appears to lack configuration, leak objects, and could hide dangerous code.

[Barryrowe]

Ionic version: (check one with "x")
[ ] 1.x
[x] 2.x
[x] 3.x

I'm submitting a ... (check one with "x")
[x] bug report
[ ] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://forum.ionicframework.com/ or http://ionicworldwide.herokuapp.com/

Current behavior:

When implementing the suggested "guard" approach for Ionic, using the ionViewCanEnter hook on a page, there are a handful of issues that can arise with its implementation:

Lack Configuration
There is currently no configurable way to tell the navController where to go by default if an ionViewCanEnter returns/resolves false as the first route (a very possible case with DeepLinking). All you can do at the moment is inject navCtrl and manually navigate the user. This is tedious, and error prone when you have many pages that could receive a DeepLink that needs a guard. Being able to set a default redirect for a rejected ionCanViewEnter makes sense. Or maybe ionViewCanEnter needs to return an object like: { canEnter: boolean, redirectPage?: any }, so they can still override it. This is related to #11405

Leak Objects
If the ionViewCanEnter function returns/resolves false the newly instantiated component is never destroyed. (ngOnDestroy is never called).

Hide Dangerous Code
Unlike the core angular router guards, this implementation requires an instance of the component to be instantiated. This means that a developer could write some logic in the constructor for the component that they don't expect to run, that will always run regardless of the result of ionViewCanEnter

This paired with the lack of guaranteeing the components are destroyed, could lead to memory leaks, additional subscriptions, etc. Ex: it's not uncommon for Observable subscriptions to be created in a constructor, and if they aren't unsubscribed, this can prevent the component from being GC'd, as well as potentially duplicating the observable chain logic.

LazyLoading
In addition to the above items, if you're setting up lazy loaded modules with DeepLinking, the way guarding is handles means a whole module bundle has to be requested, loaded, and run before the decision is made that "Hey you shouldn't even be here"

Expected behavior:
I know there are complications with this, but the way the core angular router handles guarding seems to be the correct approach. Guarding at the navigation level and not at the page component level. This is where you have the chance to prevent the component from even being built, or loading a module that hasn't been yet loaded in lazy loading scenarios.

Steps to reproduce:

A simple example of not getting destroyed and not redirecting. Doesn't show the mulitiples as I didn't implement tabs.
http://plnkr.co/edit/Y7LNSD8xy2q6TcPPpXQf?p=preview

To create a more complete example

1. Create a project with the tabs template
2. Implement ionCanViewenter in the home page component to return false
3. Implement ngOnDestroy in the home page to console log when it is destroyed
4. Add a console.log line to the constructor for the home page to show it's being instantiated.

1. Run the app, open a console, and see that A) the page doesn't default to loading anything since the tabs page can't load it's view and B) ngOnDestroy is never called on the home page even though it is constructed.

Related code:

ionViewCanEnter(){
 //You can uncomment this to 'redirect' to the About page and see that multiple attempts to navigate to Home will result in multiple constructions and no destroys.
 //this.navCtrl.setRoot(AboutPage);
 return false;
}

ngOnDestroy(){
   console.log("sup nerds? I should be destroyed now right?");
}

Other information:
Ionic info: (run ionic info from a terminal/cmd prompt and paste output below):

Cordova CLI: 6.5.0
Ionic Framework Version: 3.0.1
Ionic CLI Version: 2.2.2
Ionic App Lib Version: 2.2.1
Ionic App Scripts Version: 1.3.0
ios-deploy version: Not installed
ios-sim version: Not installed
OS: Windows 7
Node Version: v7.9.0
Xcode version: Not installed

[jgw96]

Hello @Barryrowe , thanks for the detailed issue! We will look into this.

[mburger81]

Very good post @Barryrowe and I'm 100% in your opinion.

[olegwn]

@Barryrowe right to the point. It's our pain to implement route guards with Ionic 3 :( Works fine for small apps, but for production ready - nope

[jsanta]

App menus have the same problem.
I expected this kind of code to work:

ionViewCanEnter(): boolean {
   if(!!this.global.get('token')) { // Allow entrance if there is a token (or any other validation)
      return true;
    } 

    this.navCtrl.setRoot('LoginPage');    // LoginPage is my rootPage/default
    return false;
  }

it didn't, so I had to use ionWillEnter to make the same validations.
A white screen is presented when using ionic serve, and as the screen has a menu , I can access the menu even though I shouldn't be able to enter the page.

[Barryrowe]

@jgw96 I'm curious if this is being looked at? I know there was talk on other threads around the time I opened this that DeepLinking and routing in general was getting a heavy look, and possible restructure, and that would definitely impact this issue.

I haven't seen anything of the sorts, but I may have missed it. Just looking for any feedback the team might have.

Thank you.

[mburger81]

Have a look also to this issue #12193

[danbucholtz]

I am going to take a look at nav guards soon.

Thanks,
Dan

[macheema]

May we return observable as follow??

  ionViewCanEnter() {
     return this.sessionService.sessionEvent$.map(user => {
       this.user = user;
       if (this.user.isAuthenticated == false) {
         return false;
       }
     });
   }

[FazioNico]

Here my use of ionViewCanEnter().
It work find ;-)

https://github.com/FazioNico/mean-ionic-ngrx/blob/master/src/decorators/index.ts
https://github.com/FazioNico/mean-ionic-ngrx/blob/master/src/pages/home/home.ts

[realappie]

@danbucholtz Any updates on this dan?

[rbaumi]

@danbucholtz Any updates?

[mulish77]

@danbucholtz It will be helpful if you share us how long does it take or your opinion to solve this?

[mburger81]

Not sure if this can help, but #12193 was before in different milestones but never closed, now it is on milestones for 4.x so perhaps we will have it there and perhaps also this could be resolved on ionic4.
I think with version 4 they would do a next step with there router.
For what I know, also they are discussing using the plain angular router, which would be fantastic!
BTW on ionic4 you could use any framework with there component so you could use plain angular framework with ionic web components.
And there is also already now a project which is called ngX-Rocket where you can use ionic with plain angular4 project structure using there router.

We'll wait for ionic4 and see what they w'll do with router and lazyloading, if we are not satisfied we'll use only there web components with ngx-rocket or plain angular project.

IMO

[MaxXx1313]

It seems like this.navCtrl.setRoot() doesn't work in the same tick, but using setTimeout works fine for me:

  ionViewCanEnter(): boolean | Promise<any> {
    const isAllowed = false; // some expression here

    console.log('TestPage: ionViewCanEnter:', isAllowed);
    if (!isAllowed) {
      // can't navigate in the same tick
      setTimeout(() => {
        console.log('TestPage: redirect');
        this.navCtrl.setRoot('feedback');
      }, 0);
    }
    return isAllowed;
  }

[thegust]

The above example works fine, but the whole function needs to be replicated in every page.
I think this should be better

In your AuthProvider:

isAuthenticated(nav: NavController): boolean | Promise<any> {
    return this.storage.get("token").then(token => {
        if (token == null) {
            setTimeout(() => { nav.setRoot("SignInPage") }, 0);
            return false
        } else {
            return true
        }
    }).catch(() => {
        setTimeout(() => { nav.setRoot("SignInPage") }, 0);
        return false
    });
}

Now you can just simply call the function in your pages:

ionViewCanEnter(): boolean | Promise<any> {
    return this.auth.isAuthenticated(this.navCtrl);
}

[dalenguyen]

Hi guys, This is what I did for my firebase project. It works

  ionViewCanEnter(): Promise<any>{
    return new Promise((resolve, reject) => {
      this.auth.getUser().then((user: firebase.User) => {            
        resolve(true);        
      }).catch(() => {
        resolve(false);
      });
    })
  }

[HardikDG]

@thegust's approach is very good for individual pages. Anyone got any global solution instead of writing the 'ionViewCanEnter' on every page.

[izio38]

@HardikDG Wait for Ionic V4 with Angular Routing implementation

[taofik-nos]

One way is to use async which worked for me.

isAuthenticated() in auth provider:

 isAuthenticated() {
    return new Promise((resolve, reject) => {
      this.storage.get('token').then((token) => {
        if(token)
          resolve(true)
        else
          resolve(false)
        }).catch((e) => {
          resolve(false)
        })
    })
  }

Then using ionViewCanEnter()as follows:

async ionViewCanEnter() {
    const can = await this.authProvider.isAuthenticated();

    if(can){
      return false;
    }

    return true;
  }

[ionitron-bot[bot]]

This issue has been automatically identified as an Ionic 3 issue. We recently moved Ionic 3 to its own repository. I am moving this issue to the repository for Ionic 3. Please track this issue over there.

If I've made a mistake, and if this issue is still relevant to Ionic 4, please let the Ionic Framework team know!

Thank you for using Ionic!

[ionitron-bot[bot]]

Issue moved to: https://github.com/ionic-team/ionic-v3/issues/281