Skip to content

fix(location-strategy): crash when back on named lazy loaded outlet #1618

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

Merged
merged 1 commit into from
Nov 19, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions nativescript-angular/router/ns-location-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,15 @@ export class Outlet {
// in module that lazy loads children (loadChildren) and has outlet name.
outletKeys: Array<string>;

// More than one frame available when using NSEmptyOutletComponent component
// in module that lazy loads children (loadChildren) and has outlet name.
frames: Array<Frame> = [];
// The url path to the Outlet.
// E.G: the path to Outlet3 that is nested Outlet1(home)->Outlet2(nested1)->Outlet3(nested2)
// will be 'home/nested1'
path: string;
pathByOutlets: string;
states: Array<LocationState> = [];
frame: Frame;

// Used in reuse-strategy by its children to determine if they should be detached too.
shouldDetach: boolean = true;
Expand All @@ -35,6 +37,10 @@ export class Outlet {
this.path = path;
}

containsFrame(frame: Frame): boolean {
return this.frames.indexOf(frame) > -1;
}

peekState(): LocationState {
if (this.states.length > 0) {
return this.states[this.states.length - 1];
Expand Down Expand Up @@ -244,7 +250,7 @@ export class NSLocationStrategy extends LocationStrategy {
const topmostFrame = this.frameService.getFrame();
this.currentOutlet = this.getOutletByFrame(topmostFrame);
}
this.currentOutlet.frame.goBack();
this.currentOutlet.frames[this.currentOutlet.frames.length - 1].goBack();
} else {
// Nested navigation - just pop the state
if (isLogEnabled()) {
Expand Down Expand Up @@ -423,7 +429,9 @@ export class NSLocationStrategy extends LocationStrategy {
}

updateOutletFrame(outlet: Outlet, frame: Frame) {
outlet.frame = frame;
if (!outlet.containsFrame(frame)) {
outlet.frames.push(frame);
}
this.currentOutlet = outlet;
}

Expand All @@ -436,10 +444,10 @@ export class NSLocationStrategy extends LocationStrategy {
}

// Remove outlet from the url tree.
if (currentOutlet.frame === frame && !isEqualToCurrent) {
if (currentOutlet.containsFrame(frame) && !isEqualToCurrent) {
this.callPopState(null, true, currentOutlet);
}
return currentOutlet.frame !== frame;
return !currentOutlet.containsFrame(frame);
});
}

Expand Down Expand Up @@ -528,7 +536,7 @@ export class NSLocationStrategy extends LocationStrategy {
for (let index = 0; index < this.outlets.length; index++) {
const currentOutlet = this.outlets[index];

if (currentOutlet.frame === frame) {
if (currentOutlet.containsFrame(frame)) {
outlet = currentOutlet;
break;
}
Expand Down
2 changes: 1 addition & 1 deletion nativescript-angular/router/page-router-outlet.ts
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,7 @@ export class PageRouterOutlet implements OnDestroy { // tslint:disable-line:dire
let outletKeyForRoute = this.locationStrategy.getRouteFullPath(nodeToMark);
let outlet = this.locationStrategy.findOutletByKey(outletKeyForRoute);

if (outlet && outlet.frame) {
if (outlet && outlet.frames.length) {
nodeToMark[pageRouterActivatedSymbol] = true;
if (isLogEnabled()) {
log("Activated route marked as page: " + routeToString(nodeToMark));
Expand Down
4 changes: 2 additions & 2 deletions tests/app/tests/modal-dialog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ describe("modal-dialog", () => {

let parentView = fixture.componentRef.instance.vcRef.element.nativeElement;
parentView = parentView.page && parentView.page.frame;
outlet.frame = parentView;
outlet.frames.push(parentView);
locStrategy._getOutlets().push(outlet);

locStrategy.pushState(null, "test", "/test", null);
Expand All @@ -119,7 +119,7 @@ describe("modal-dialog", () => {

let parentView = fixture.componentRef.instance.vcRef.element.nativeElement;
parentView = parentView.page && parentView.page.frame;
outlet.frame = parentView;
outlet.frames.push(parentView);
locStrategy._getOutlets().push(outlet);

locStrategy.pushState(null, "test", "/test", null);
Expand Down
2 changes: 1 addition & 1 deletion tests/app/tests/ns-location-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ function simulatePageNavigation(strategy: NSLocationStrategy, url: string, frame
strategy.pushState(null, null, url, null);

const outlet: Outlet = strategy.findOutletByOutletPath(outletName);
outlet.frame = frame;
outlet.frames.push(frame);
strategy._beginPageNavigation(frame);
}

Expand Down