-
-
Notifications
You must be signed in to change notification settings - Fork 241
fix(router): avoiding throw for app stability improvements #1344
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
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,6 +102,8 @@ function routeToString(activatedRoute: ActivatedRoute | ActivatedRouteSnapshot): | |
return activatedRoute.pathFromRoot.join("->"); | ||
} | ||
|
||
const routeTransitionWarning = "This could be due to route transition timing and could be ignored."; | ||
|
||
@Directive({ selector: "page-router-outlet" }) // tslint:disable-line:directive-selector | ||
export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:directive-class-suffix | ||
private activated: ComponentRef<any> | null = null; | ||
|
@@ -126,14 +128,16 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire | |
|
||
get component(): Object { | ||
if (!this.activated) { | ||
throw new Error("Outlet is not activated"); | ||
log("Outlet is not activated"); | ||
return; | ||
} | ||
|
||
return this.activated.instance; | ||
} | ||
get activatedRoute(): ActivatedRoute { | ||
if (!this.activated) { | ||
throw new Error("Outlet is not activated"); | ||
log("Outlet is not activated"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @vakrilov worth noting that obviously this logic mirrors what Angular framework does for |
||
return; | ||
} | ||
|
||
return this._activatedRoute; | ||
|
@@ -173,8 +177,9 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire | |
|
||
deactivate(): void { | ||
if (!this.locationStrategy._isPageNavigatingBack()) { | ||
throw new Error("Currently not in page back navigation" + | ||
" - component should be detached instead of deactivated."); | ||
log("Currently not in page back navigation" + | ||
" - component should be detached instead of deactivated." + routeTransitionWarning); | ||
return; | ||
} | ||
|
||
log("PageRouterOutlet.deactivate() while going back - should destroy"); | ||
|
@@ -197,7 +202,8 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire | |
*/ | ||
detach(): ComponentRef<any> { | ||
if (!this.isActivated) { | ||
throw new Error("Outlet is not activated"); | ||
log("Outlet is not activated"); | ||
return; | ||
} | ||
|
||
log("PageRouterOutlet.detach() - " + routeToString(this._activatedRoute)); | ||
|
@@ -232,7 +238,9 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire | |
resolver: ComponentFactoryResolver | null): void { | ||
|
||
if (this.locationStrategy._isPageNavigatingBack()) { | ||
throw new Error("Currently in page back navigation - component should be reattached instead of activated."); | ||
log("Currently in page back navigation - component should be reattached instead of activated. " + | ||
routeTransitionWarning); | ||
this.locationStrategy._finishBackPageNavigation(); | ||
} | ||
|
||
log("PageRouterOutlet.activateWith() - " + routeToString(activatedRoute)); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@vakrilov shoot so sorry this should be deleted as I don’t believe it ended up being used - not in front of computer but could fix up tomorrow or if you just want to remove this line just wanted to flag it before it went in.