From 23ee29b6a2ae396f6ec13f71c9552e3e0a983c3c Mon Sep 17 00:00:00 2001 From: vsavkin Date: Wed, 20 Jul 2016 14:30:04 -0700 Subject: [PATCH] fix(router): navigation should not preserve query params and fragment by default BREAKING CHANGE Previously both imperative (router.navigate) and declarative (routerLink) navigations would preserve the current query params and fragment. This behavior turned out to be confusing. This commit changes it. Now, neither is preserved by default. To preserve them, you need to do the following: router.naviage("newUrl", {preserveQueryParams: true, preserveFragment: true}) --- .../@angular/router/src/create_url_tree.ts | 8 ++-- .../router/src/directives/router_link.ts | 38 +++++++++++++---- modules/@angular/router/src/router.ts | 11 +++-- modules/@angular/router/src/url_tree.ts | 4 +- .../router/test/create_url_tree.spec.ts | 12 ------ modules/@angular/router/test/router.spec.ts | 42 +++++++++++++++---- 6 files changed, 79 insertions(+), 36 deletions(-) diff --git a/modules/@angular/router/src/create_url_tree.ts b/modules/@angular/router/src/create_url_tree.ts index 6d9cb24b90..d2794d07e5 100644 --- a/modules/@angular/router/src/create_url_tree.ts +++ b/modules/@angular/router/src/create_url_tree.ts @@ -42,13 +42,11 @@ function validateCommands(n: NormalizedNavigationCommands): void { function tree( oldSegment: UrlSegment, newSegment: UrlSegment, urlTree: UrlTree, queryParams: Params, fragment: string): UrlTree { - const q = queryParams ? stringify(queryParams) : urlTree.queryParams; - const f = fragment ? fragment : urlTree.fragment; - if (urlTree.root === oldSegment) { - return new UrlTree(newSegment, q, f); + return new UrlTree(newSegment, stringify(queryParams), fragment); } else { - return new UrlTree(replaceSegment(urlTree.root, oldSegment, newSegment), q, f); + return new UrlTree( + replaceSegment(urlTree.root, oldSegment, newSegment), stringify(queryParams), fragment); } } diff --git a/modules/@angular/router/src/directives/router_link.ts b/modules/@angular/router/src/directives/router_link.ts index 904e44f0ca..ad26a4f17d 100644 --- a/modules/@angular/router/src/directives/router_link.ts +++ b/modules/@angular/router/src/directives/router_link.ts @@ -51,9 +51,15 @@ import {UrlTree} from '../url_tree'; * link to user component * ``` - * * RouterLink will use these to generate this link: `/user/bob#education?debug=true`. * + * You can also tell the directive to preserve the current query params and fragment: + * + * ``` + * link to user + component + * ``` + * * @stable */ @Directive({selector: ':not(a)[routerLink]'}) @@ -61,6 +67,8 @@ export class RouterLink { private commands: any[] = []; @Input() queryParams: {[k: string]: any}; @Input() fragment: string; + @Input() preserveQueryParams: boolean; + @Input() preserveFragment: boolean; constructor( private router: Router, private route: ActivatedRoute, @@ -85,9 +93,13 @@ export class RouterLink { } get urlTree(): UrlTree { - return this.router.createUrlTree( - this.commands, - {relativeTo: this.route, queryParams: this.queryParams, fragment: this.fragment}); + return this.router.createUrlTree(this.commands, { + relativeTo: this.route, + queryParams: this.queryParams, + fragment: this.fragment, + preserveQueryParams: toBool(this.preserveQueryParams), + preserveFragment: toBool(this.preserveFragment) + }); } } @@ -101,6 +113,9 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy { private commands: any[] = []; @Input() queryParams: {[k: string]: any}; @Input() fragment: string; + @Input() routerLinkOptions: {preserveQueryParams: boolean, preserveFragment: boolean}; + @Input() preserveQueryParams: boolean; + @Input() preserveFragment: boolean; private subscription: Subscription; // the url displayed on the anchor element. @@ -148,12 +163,21 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy { } private updateTargetUrlAndHref(): void { - this.urlTree = this.router.createUrlTree( - this.commands, - {relativeTo: this.route, queryParams: this.queryParams, fragment: this.fragment}); + this.urlTree = this.router.createUrlTree(this.commands, { + relativeTo: this.route, + queryParams: this.queryParams, + fragment: this.fragment, + preserveQueryParams: toBool(this.preserveQueryParams), + preserveFragment: toBool(this.preserveFragment) + }); if (this.urlTree) { this.href = this.locationStrategy.prepareExternalUrl(this.router.serializeUrl(this.urlTree)); } } } + +function toBool(s?: any): boolean { + if (s === '') return true; + return !!s; +} \ No newline at end of file diff --git a/modules/@angular/router/src/router.ts b/modules/@angular/router/src/router.ts index 913ad39a1a..d00f0ddf40 100644 --- a/modules/@angular/router/src/router.ts +++ b/modules/@angular/router/src/router.ts @@ -45,6 +45,8 @@ export interface NavigationExtras { relativeTo?: ActivatedRoute; queryParams?: Params; fragment?: string; + preserveQueryParams?: boolean; + preserveFragment?: boolean; } /** @@ -227,10 +229,13 @@ export class Router { * router.createUrlTree(['../../team/44/user/22'], {relativeTo: route}); * ``` */ - createUrlTree(commands: any[], {relativeTo, queryParams, fragment}: NavigationExtras = {}): - UrlTree { + createUrlTree( + commands: any[], {relativeTo, queryParams, fragment, preserveQueryParams, + preserveFragment}: NavigationExtras = {}): UrlTree { const a = relativeTo ? relativeTo : this.routerState.root; - return createUrlTree(a, this.currentUrlTree, commands, queryParams, fragment); + const q = preserveQueryParams ? this.currentUrlTree.queryParams : queryParams; + const f = preserveFragment ? this.currentUrlTree.fragment : fragment; + return createUrlTree(a, this.currentUrlTree, commands, q, f); } /** diff --git a/modules/@angular/router/src/url_tree.ts b/modules/@angular/router/src/url_tree.ts index 69759449c8..c7db4b6d71 100644 --- a/modules/@angular/router/src/url_tree.ts +++ b/modules/@angular/router/src/url_tree.ts @@ -183,7 +183,9 @@ export class DefaultUrlSerializer implements UrlSerializer { serialize(tree: UrlTree): string { const segment = `/${serializeSegment(tree.root, true)}`; const query = serializeQueryParams(tree.queryParams); - const fragment = tree.fragment !== null ? `#${encodeURIComponent(tree.fragment)}` : ''; + const fragment = tree.fragment !== null && tree.fragment !== undefined ? + `#${encodeURIComponent(tree.fragment)}` : + ''; return `${segment}${query}${fragment}`; } } diff --git a/modules/@angular/router/test/create_url_tree.spec.ts b/modules/@angular/router/test/create_url_tree.spec.ts index 9189e72c42..821d440af4 100644 --- a/modules/@angular/router/test/create_url_tree.spec.ts +++ b/modules/@angular/router/test/create_url_tree.spec.ts @@ -182,23 +182,11 @@ describe('createUrlTree', () => { expect(t.queryParams).toEqual({a: '1'}); }); - it('should reuse old query params when given undefined', () => { - const p = serializer.parse('/?a=1'); - const t = createRoot(p, [], undefined); - expect(t.queryParams).toEqual({a: '1'}); - }); - it('should set fragment', () => { const p = serializer.parse('/'); const t = createRoot(p, [], {}, 'fragment'); expect(t.fragment).toEqual('fragment'); }); - - it('should reused old fragment when given undefined', () => { - const p = serializer.parse('/#fragment'); - const t = createRoot(p, [], undefined, undefined); - expect(t.fragment).toEqual('fragment'); - }); }); diff --git a/modules/@angular/router/test/router.spec.ts b/modules/@angular/router/test/router.spec.ts index fd0c4ea628..9c667d41a7 100644 --- a/modules/@angular/router/test/router.spec.ts +++ b/modules/@angular/router/test/router.spec.ts @@ -594,10 +594,9 @@ describe('Integration', () => { expect(fixture.debugElement.nativeElement).toHaveText('team 33 [ simple, right: ]'); }))); - it('should update hrefs when query params change', + it('should not preserve query params and fragment by default', fakeAsync( inject([Router, TestComponentBuilder], (router: Router, tcb: TestComponentBuilder) => { - @Component({ selector: 'someRoot', template: `Link`, @@ -612,15 +611,42 @@ describe('Integration', () => { const native = fixture.debugElement.nativeElement.querySelector('a'); - router.navigateByUrl('/home?q=123'); + router.navigateByUrl('/home?q=123#fragment'); advance(fixture); - expect(native.getAttribute('href')).toEqual('/home?q=123'); - - router.navigateByUrl('/home?q=456'); - advance(fixture); - expect(native.getAttribute('href')).toEqual('/home?q=456'); + expect(native.getAttribute('href')).toEqual('/home'); }))); + it('should update hrefs when query params or fragment change', + fakeAsync(inject([Router, TestComponentBuilder], (router: Router, tcb: TestComponentBuilder) => { + + @Component({ + selector: 'someRoot', + template: + `Link`, + directives: ROUTER_DIRECTIVES + }) + class RootCmpWithLink { + } + + const fixture = createRoot(tcb, router, RootCmpWithLink); + + router.resetConfig([{path: 'home', component: SimpleCmp}]); + + const native = fixture.debugElement.nativeElement.querySelector('a'); + + router.navigateByUrl('/home?q=123'); + advance(fixture); + expect(native.getAttribute('href')).toEqual('/home?q=123'); + + router.navigateByUrl('/home?q=456'); + advance(fixture); + expect(native.getAttribute('href')).toEqual('/home?q=456'); + + router.navigateByUrl('/home?q=456#1'); + advance(fixture); + expect(native.getAttribute('href')).toEqual('/home?q=456#1'); + }))); + it('should support using links on non-a tags', fakeAsync( inject([Router, TestComponentBuilder], (router: Router, tcb: TestComponentBuilder) => {