This repository has been archived by the owner on May 4, 2022. It is now read-only.
The suggested "guard" approach with ionViewCanEnter/ionViewCanLeave appears to lack configuration, leak objects, and could hide dangerous code. #281
Labels
Original issue by @Barryrowe on 2017-05-01T13:39:38Z
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 rejectedionCanViewEnter
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 #11405Leak Objects
If the
ionViewCanEnter
function returns/resolvesfalse
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
Create a project with the tabs template
Implement
ionCanViewenter
in the home page component to return falseImplement
ngOnDestroy
in the home page to console log when it is destroyedAdd a console.log line to the constructor for the home page to show it's being instantiated.
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:
Other information:
Ionic info: (run
ionic info
from a terminal/cmd prompt and paste output below):The text was updated successfully, but these errors were encountered: