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.03k stars 13.51k forks source link

Missing CanActivate/Resolve Functionality in Ionic2 #6148

Closed ataraxus closed 8 years ago

ataraxus commented 8 years ago

Short description of the problem:

Need to resolve data before @Page gets rendered.

What behavior are you expecting?

IMHO: Ionics View callbacks onPage* to respect/handle returned promises/observables.

Detailed description of the problem:

Hello,

I'm opening this issue after trying to find a solution/help in the forum, SO and slack. Till now there are good hints for how to work around this issue, but no definitive solution.

As for now the decorator @CanActivate of Angular2 gets not called within Ionic2. Which opens the question what is the correct way to get data async before a Page gets rendered? This issue was posted on the forums, but didn't get any "solution".

Apparently Ionic2 does something with the @CanActivate decorator, but its not documented and i can't figure out what it does exactly.

Nevertheless this guy points out one should use Ionic2 View States instead. His example looks like this:

  onPageWillEnter() { 
      return this._service.getComments().then(data => this.comments = data);
  }

Which looks like he is expecting Ionic to consider the returned promise, but a quick glance at Ionics sources reveals (at least I think so) that the returned value is ignored. Hence there is no guarantee that the promise gets resolved before the page gets rendered. During a discussion on #ionic-v2 I created an example with onPage* and how it does not perform as needed/expected.

Many people suggested to resolve the data before navigating to the page, which burdens the knowledge which data is needed for the page on the callee. This breaks the nice encapsulation of the Angular2 Component in my opinion.

I'm using

Cordova CLI: 6.1.1 Gulp version: CLI version 3.9.1 Gulp local: Local version 3.9.1 Ionic Framework Version: 2.0.0-beta.4 Ionic CLI Version: 2.0.0-beta.24 Ionic App Lib Version: 2.0.0-beta.14 OS: Windows 7 SP1 Node Version: v4.4.2

derekdon commented 8 years ago

Any update on this? Not having resolve is the main reason why I haven't migrated to ionic2 yet. I can work around the issue, but would rather wait if it's going to land soon.

stefan-1st commented 8 years ago

I would love to see this in the ionic code as well!

ataraxus commented 8 years ago

I checked recently with

Ionic Framework Version: 2.0.0-beta.7
Ionic CLI Version: 2.0.0-beta.25
Ionic App Lib Version: 2.0.0-beta.15
OS: Windows 7 SP1
Node Version: v4.4.4

no progress so far :(

melkir commented 8 years ago

I agree, it would be cool to have this feature. I'm trying to implement an authenticate system with resticted tab, for the moment I'm listening for onPageWillEnter to redirect if the user is not logged but it's not really efficient since the page is showed 0,1 sec. If someone know how can I redirect the user before the page is loaded like prevent default...

Hufschmidt commented 8 years ago

This has been driving me nuts the last few days, so I've gone and implemented this myself.

Since the NavController uses transitions directly instead of the relying on Angular2's (relatively new) beta-router, I simply delayed the transition to wait until both the promise returned by the 'entering-view component' using 'ionCanActivate(enteringView: ViewController, leavingView: ViewController)' and the 'leaving-view component' using 'ionCanDeactivate(enteringView: ViewController, leavingView: ViewController)' both have been resolved or cancel the transition if any Promise fails.

See my modifications, might create a Pull-Request... Currently debating wether to pass the views or the controllers (view.instance) to the ionCan... calls.

Now I can finally implement authentication without relying on workarounds with NavParams or the LoadingController. :)

Hufschmidt commented 8 years ago

I need some feedback from the ionic crew: Are there any (immediate) plans to update/rewrite the current nav-controller?

I'm asking, because I'd like to improve the current implementation to reject the navigation promise (eg. returned by nav.setRoot()) upon rejection of an ionCan... promise. For this I'd like to change the current nav-controller to actually use promise-chaining instead of using the deferred-promise anti-pattern, so I don't have to pass done (and failed) callbacks around...

ataraxus commented 8 years ago

Hey @Hufschmidt and @melkir have a look on my SO Question which got some sound workarounds. Maybe its also working for you @melkir: https://stackoverflow.com/questions/36569179/get-data-async-before-a-page-gets-rendered

Upvotes are appreciated :palm_tree:

jgw96 commented 8 years ago

@Hufschmidt We are currently doing alot of work around navigation. While it is not a rewrite of NavController, as NavController works perfectly for what it is meant to do, we are adding some very cool functionality that should solve this issue. Thanks!

Hufschmidt commented 8 years ago

@jgw96 Thanks for the feedback. With this info I'll probably not put any more work-hours into improving my current changes to the NavController and rather wait on the awesome things to come, keep up the good work. :smiley:

@ataraxus Yes, those where the workarounds I was talking about, thanks for posting the link though, good idea. :smiley: Sadly both solutions have their own unique flaws:

sebaferreras solutiuon: While I really like this one, especially for the additional UI feedback thanks to the loading-controller the problem remains that this component is fully loaded and gets all of its (angular and ionic) life-cycle hooks executed regardless of the additional async parameters state. Furthermore, if any (required) async parameter rejects, the component itself would be responsible for restoring the previous navigation/application state. (Which could be as simple as nav.pop() but could also get arbitrarily complex)

Thomas Withaars solution: While this solutions does not suffer from the above flaws, now each component responsible for navigation has to know about the requirements of every component it can navigate to, circumventing angulars DI for those parameters. On the plus side, it does kind of work like the angular1 router, in the sense that resolved parameters get injected into the component (via the nav-params), this is something my changes to the NavController is lacking for example. :sweat_smile:

Comparatively I'd say the later solution (while not the accepted one) is the lesser of both "evils". (You could even use the loading-controller here while the async parameters get resolved before beeing passed as nav-params... :smirk:)

akshaycs-zz commented 8 years ago

Is there an update on this now that we have RC0? Thanks!

manucorporat commented 8 years ago

RC0 supports ionViewCanLeave and ionViewCanEnter.

luchillo17 commented 8 years ago

@manucorporat It says so in the changelog, however the NavController docs says nothing about how are they used, do they expect true or false? do they expect a promise to resolve or reject?

luchillo17 commented 8 years ago

I've been playing with it and returning false is the way to stop navigation, however i don't see how to trigger a redirect other than using setTimeout to put the redirect last in the js event loop:

import { Component } from '@angular/core';
import { NavController } from 'ionic-angular';

import { Page2 } from '../page2/page2.ts';

@Component({
  selector: 'page-page1',
  templateUrl: 'page1.html'
})
export class Page1 {
  constructor(public navCtrl: NavController) { }

  ionViewCanEnter() {
    console.log('Can enter.');
    setTimeout(() => {
      this.navCtrl.setRoot(Page2)
    }, 0);
    return false;
  }

}
luchillo17 commented 8 years ago

Ok not so much, that only works at the start, if i just return false in a page that's supposed to be navigated after the root component is been set, it breaks.

danishin commented 8 years ago

Is using ionViewCanEnter really a valid solution for angular's resolve? As far as I can tell, It is equivalent of angular's CanActivate, not resolve.

mohammadshamma commented 7 years ago

This is just awful. I can't conceive how the authentication use case was not fully fleshed out when designing the navigation controller.

luchillo17 commented 7 years ago

Is still no way to control authentication?

ataraxus commented 7 years ago

@Luchillo, @mohammadshamma have a look at the navcontroller ionViewCanEnter it does exactly what you are looking for. http://ionicframework.com/docs/v2/nightly/api/navigation/NavController

ahoebeke commented 7 years ago

Indeed, thanks @ataraxus for the link. Seems the documentation contains very clear examples and explanations now. when doing a "push()" on the nav controller, a ".catch()" is needed to intercept the true/false values emitted by the ionViewCanLeave/Enter (use canLeave in same component, or canEnter in the destination page or component).

Haven't had time to actually test this yet, but it's clearly written so hopefully reproducing this will be straightforward and simple. 👍

krysalead commented 7 years ago

Hi,

I jump in the discussion as I am also facing the same issue. I am trying to secure pages of my app but if we compare the ionViewCanEnter and canActivate they don't have the same signature which quite annoying if we want to achieve the same goal.

mohammadshamma commented 7 years ago

@ataraxus My criticism is for the fact that there is no cross cutting way of doing the check for every page. My understanding is that I might have to define the ionViewCanEnter for every page that expects authentication gating.

ataraxus commented 7 years ago

@mohammadshamma I think this, as it is always a trade of... either you whitelist or you blacklist. If you dont want to duplicate your auth code, just create a class SecurePage with your ionViewCanEnter and subclass all pages need to be secured. I know its not as common in JS to subclass, I need to get used to it too ;)

omaxlive commented 6 years ago

Any updates?

ionitron-bot[bot] commented 6 years ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.