Skip to content

fix(router): query params not being saved #2062

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 13 commits into from
Jun 5, 2020
11 changes: 10 additions & 1 deletion e2e/router/app/first/first.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,16 @@ import { RouterExtensions } from "nativescript-angular/router";
import { Page } from "tns-core-modules/ui/page";

import { CounterService } from "../counter.service";
import { ActivatedRoute } from "@angular/router";
import { Subscription } from "rxjs";

@Component({
selector: "first",
template: `
<StackLayout>
<Label text="FirstComponent" class="header"></Label>

<Button text="GO TO SECOND" automationText="GO TO SECOND" [nsRouterLink]="['/second','1']"></Button>
<Button text="GO TO SECOND" automationText="GO TO SECOND" [nsRouterLink]="['/second','1']" [queryParams]="{prop: 'xxx'}"></Button>
<Button text="GO TO C-LESS SECOND" automationText="GO TO C-LESS SECOND" [nsRouterLink]="['/c-less', 'deep', '100', 'detail', '200']"></Button>

<Button text="GO TO LAZY HOME" automationText="GO TO LAZY HOME" [nsRouterLink]="['/lazy','home']"></Button>
Expand All @@ -26,9 +28,11 @@ import { CounterService } from "../counter.service";
export class FirstComponent implements OnInit, OnDestroy, DoCheck {
public message: string = "";
public doCheckCount: number = 0;
sub: Subscription;

constructor(
private routerExt: RouterExtensions,
private route: ActivatedRoute,
public service: CounterService,
page: Page) {

Expand All @@ -37,9 +41,14 @@ export class FirstComponent implements OnInit, OnDestroy, DoCheck {

ngOnInit() {
console.log("FirstComponent - ngOnInit()");
this.sub = this.route.queryParams.subscribe((params) =>{
console.log("FIRST PARAMS:");
console.log(params);
});
}

ngOnDestroy() {
this.sub.unsubscribe();
console.log("FirstComponent - ngOnDestroy()");
}

Expand Down
11 changes: 10 additions & 1 deletion e2e/router/app/second/second.component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { ActivatedRoute } from "@angular/router";

import { RouterExtensions } from "nativescript-angular/router";
import { Page } from "tns-core-modules/ui/page";
import { Observable } from "rxjs";
import { Observable, Subscription } from "rxjs";
import { map } from "rxjs/operators";
import { CounterService } from "../counter.service";

Expand Down Expand Up @@ -36,6 +36,7 @@ import { CounterService } from "../counter.service";
export class SecondComponent implements OnInit, OnDestroy {
public depth$: Observable<string>;
public nextDepth$: Observable<number>;
sub: Subscription;

constructor(
private routerExt: RouterExtensions,
Expand All @@ -48,10 +49,18 @@ export class SecondComponent implements OnInit, OnDestroy {
}

ngOnInit() {
this.sub = this.route.queryParams.subscribe((params) => {
console.log("");
console.log("");
console.log(params);
console.log("");
console.log("");
});
console.log("SecondComponent - ngOnInit()");
}

ngOnDestroy() {
this.sub.unsubscribe();
console.log("SecondComponent - ngOnDestroy()");
}

Expand Down
31 changes: 18 additions & 13 deletions nativescript-angular/router/ns-location-strategy.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Injectable } from "@angular/core";
import { LocationStrategy } from "@angular/common";
import { DefaultUrlSerializer, UrlSegmentGroup, UrlTree, ActivatedRouteSnapshot } from "@angular/router";
import { DefaultUrlSerializer, UrlSegmentGroup, UrlTree, ActivatedRouteSnapshot, Params } from "@angular/router";
import { routerLog, routerError, isLogEnabled } from "../trace";
import { NavigationTransition, Frame } from "tns-core-modules/ui/frame";
import { isPresent } from "../lang-facade";
Expand Down Expand Up @@ -85,6 +85,7 @@ const defaultNavOptions: NavigationOptions = {
};

export interface LocationState {
queryParams: Params;
segmentGroup: UrlSegmentGroup;
isRootSegmentGroup: boolean;
isPageNavigation: boolean;
Expand Down Expand Up @@ -131,6 +132,7 @@ export class NSLocationStrategy extends LocationStrategy {
}

const urlSerializer = new DefaultUrlSerializer();
tree.queryParams = state.queryParams;
const url = urlSerializer.serialize(tree);
if (isLogEnabled()) {
routerLog("NSLocationStrategy.path(): " + url);
Expand Down Expand Up @@ -165,10 +167,11 @@ export class NSLocationStrategy extends LocationStrategy {
const outletKey = this.getOutletKey(this.getSegmentGroupFullPath(segmentGroup), "primary");
const outlet = this.findOutlet(outletKey);

if (outlet && this.updateStates(outlet, segmentGroup)) {
if (outlet && this.updateStates(outlet, segmentGroup, this.currentUrlTree.queryParams)) {
this.currentOutlet = outlet; // If states updated
} else if (!outlet) {
const rootOutlet = this.createOutlet("primary", null, segmentGroup, null);
// tslint:disable-next-line:max-line-length
const rootOutlet = this.createOutlet("primary", null, segmentGroup, null, null, this.currentUrlTree.queryParams);
this.currentOutlet = rootOutlet;
}

Expand Down Expand Up @@ -197,15 +200,15 @@ export class NSLocationStrategy extends LocationStrategy {
const containsLastState = outlet && outlet.containsTopState(currentSegmentGroup.toString());
if (!outlet) {
// tslint:disable-next-line:max-line-length
outlet = this.createOutlet(outletKey, outletPath, currentSegmentGroup, parentOutlet, this._modalNavigationDepth);
outlet = this.createOutlet(outletKey, outletPath, currentSegmentGroup, parentOutlet, this._modalNavigationDepth, this.currentUrlTree.queryParams);
this.currentOutlet = outlet;
} else if (this._modalNavigationDepth > 0 && outlet.showingModal && !containsLastState) {
// Navigation inside modal view.
this.upsertModalOutlet(outlet, currentSegmentGroup);
this.upsertModalOutlet(outlet, currentSegmentGroup, this.currentUrlTree.queryParams);
} else {
outlet.parent = parentOutlet;

if (this.updateStates(outlet, currentSegmentGroup)) {
if (this.updateStates(outlet, currentSegmentGroup, this.currentUrlTree.queryParams)) {
this.currentOutlet = outlet; // If states updated
}
}
Expand Down Expand Up @@ -604,15 +607,16 @@ export class NSLocationStrategy extends LocationStrategy {
return outlet;
}

private updateStates(outlet: Outlet, currentSegmentGroup: UrlSegmentGroup): boolean {
private updateStates(outlet: Outlet, currentSegmentGroup: UrlSegmentGroup, queryParams: Params): boolean {
const isNewPage = outlet.states.length === 0;
const lastState = outlet.states[outlet.states.length - 1];
const equalStateUrls = outlet.containsTopState(currentSegmentGroup.toString());

const locationState: LocationState = {
segmentGroup: currentSegmentGroup,
isRootSegmentGroup: false,
isPageNavigation: isNewPage
isPageNavigation: isNewPage,
queryParams: {...queryParams}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be best to use some protection here in case queryParams comes in null or undefined?

queryParams: { ...(queryParams || {}) }

};

if (!lastState || !equalStateUrls) {
Expand Down Expand Up @@ -645,14 +649,15 @@ export class NSLocationStrategy extends LocationStrategy {
}

// tslint:disable-next-line:max-line-length
private createOutlet(outletKey: string, path: string, segmentGroup: any, parent: Outlet, modalNavigation?: number): Outlet {
private createOutlet(outletKey: string, path: string, segmentGroup: any, parent: Outlet, modalNavigation?: number, queryParams: Params = {}): Outlet {
const pathByOutlets = this.getPathByOutlets(segmentGroup);
const newOutlet = new Outlet(outletKey, path, pathByOutlets, modalNavigation);

const locationState: LocationState = {
segmentGroup: segmentGroup,
isRootSegmentGroup: false,
isPageNavigation: true // It is a new OutletNode.
isPageNavigation: true, // It is a new OutletNode.
queryParams: {...queryParams}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here:

queryParams: { ...(queryParams || {}) }

};

newOutlet.states = [locationState];
Expand Down Expand Up @@ -719,7 +724,7 @@ export class NSLocationStrategy extends LocationStrategy {
}
}

private upsertModalOutlet(parentOutlet: Outlet, segmentedGroup: UrlSegmentGroup) {
private upsertModalOutlet(parentOutlet: Outlet, segmentedGroup: UrlSegmentGroup, queryParams: Params) {
let currentModalOutlet = this.findOutletByModal(this._modalNavigationDepth);

// We want to treat every p-r-o as a standalone Outlet.
Expand All @@ -734,9 +739,9 @@ export class NSLocationStrategy extends LocationStrategy {
const outletPath = parentOutlet.peekState().segmentGroup.toString();
const outletKey = this.getOutletKey(outletPath, outletName);
// tslint:disable-next-line:max-line-length
currentModalOutlet = this.createOutlet(outletKey, outletPath, segmentedGroup, parentOutlet, this._modalNavigationDepth);
currentModalOutlet = this.createOutlet(outletKey, outletPath, segmentedGroup, parentOutlet, this._modalNavigationDepth, queryParams);
this.currentOutlet = currentModalOutlet;
} else if (this.updateStates(currentModalOutlet, segmentedGroup)) {
} else if (this.updateStates(currentModalOutlet, segmentedGroup, queryParams)) {
this.currentOutlet = currentModalOutlet; // If states updated
}
}
Expand Down
41 changes: 41 additions & 0 deletions tests/app/tests/ns-location-strategy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ function createState(
segmentGroup: isRoot ? stateUrlTree.root : rootOutlets[outletName],
isPageNavigation: isPageNav,
isRootSegmentGroup: isRoot,
queryParams: stateUrlTree.queryParams
};
}

Expand Down Expand Up @@ -269,6 +270,46 @@ describe("NSLocationStrategy", () => {
assert.equal(popCount, 0); // no onPopState when replacing
});

it("back() preserves query params", () => {
const { strategy } = initStrategy("/?param=1");
let popCount = 0;
strategy.onPopState(() => {
popCount++;
});

strategy.pushState(null, "test", "/test", null);
assert.equal(strategy.path(), "/test");
assert.equal(popCount, 0);

strategy.pushState(null, "test2", "/test2?param2=2", null);
assert.equal(strategy.path(), "/test2?param2=2");
assert.equal(popCount, 0);

strategy.pushState(null, "test3", "/test3?param3=3", null);
assert.equal(strategy.path(), "/test3?param3=3");
assert.equal(popCount, 0);

strategy.pushState(null, "test4", "/test4", null);
assert.equal(strategy.path(), "/test4");
assert.equal(popCount, 0);

strategy.back();
assert.equal(strategy.path(), "/test3?param3=3");
assert.equal(popCount, 1);

strategy.back();
assert.equal(strategy.path(), "/test2?param2=2");
assert.equal(popCount, 2);

strategy.back();
assert.equal(strategy.path(), "/test");
assert.equal(popCount, 3);

strategy.back();
assert.equal(strategy.path(), "/?param=1");
assert.equal(popCount, 4);
});

it("pushState() with page navigation", () => {
const { strategy } = initStrategy("/");
const outletName = "primary";
Expand Down