Skip to content

Commit

Permalink
feat(router): improve typings for RouterLink boolean inputs (#47101)
Browse files Browse the repository at this point in the history 
Add wider typings to setter of preserveFragment, skipLocationChange and replaceUrl inputs of routerLink directives and coerce them to boolean

PR Close #47101
  • Loading branch information
@EmmanuelRoux
EmmanuelRoux authored and Pawel Kozlowski committed Aug 12, 2022
1 parent 93289f9 commit 422323c
Show file tree
Hide file tree
Showing 3 changed files with 170 additions and 63 deletions.
28 changes: 20 additions & 8 deletions goldens/public-api/router/index.md
View file
Expand Up @@ -677,20 +677,26 @@ export class RouterLink implements OnChanges {
ngOnChanges(changes: SimpleChanges): void;
// (undocumented)
onClick(): boolean;
preserveFragment: boolean;
set preserveFragment(preserveFragment: boolean | string | null | undefined);
// (undocumented)
get preserveFragment(): boolean;
queryParams?: Params | null;
queryParamsHandling?: QueryParamsHandling | null;
relativeTo?: ActivatedRoute | null;
replaceUrl: boolean;
set replaceUrl(replaceUrl: boolean | string | null | undefined);
// (undocumented)
get replaceUrl(): boolean;
set routerLink(commands: any[] | string | null | undefined);
skipLocationChange: boolean;
set skipLocationChange(skipLocationChange: boolean | string | null | undefined);
// (undocumented)
get skipLocationChange(): boolean;
state?: {
[k: string]: any;
};
// (undocumented)
get urlTree(): UrlTree | null;
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<RouterLink, ":not(a):not(area)[routerLink]", never, { "queryParams": "queryParams"; "fragment": "fragment"; "queryParamsHandling": "queryParamsHandling"; "preserveFragment": "preserveFragment"; "skipLocationChange": "skipLocationChange"; "replaceUrl": "replaceUrl"; "state": "state"; "relativeTo": "relativeTo"; "routerLink": "routerLink"; }, {}, never, never, true>;
static ɵdir: i0.ɵɵDirectiveDeclaration<RouterLink, ":not(a):not(area)[routerLink]", never, { "queryParams": "queryParams"; "fragment": "fragment"; "queryParamsHandling": "queryParamsHandling"; "state": "state"; "relativeTo": "relativeTo"; "preserveFragment": "preserveFragment"; "skipLocationChange": "skipLocationChange"; "replaceUrl": "replaceUrl"; "routerLink": "routerLink"; }, {}, never, never, true>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<RouterLink, [null, null, { attribute: "tabindex"; }, null, null]>;
}
Expand Down Expand Up @@ -735,13 +741,19 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy {
ngOnDestroy(): any;
// (undocumented)
onClick(button: number, ctrlKey: boolean, shiftKey: boolean, altKey: boolean, metaKey: boolean): boolean;
preserveFragment: boolean;
set preserveFragment(preserveFragment: boolean | string | null | undefined);
// (undocumented)
get preserveFragment(): boolean;
queryParams?: Params | null;
queryParamsHandling?: QueryParamsHandling | null;
relativeTo?: ActivatedRoute | null;
replaceUrl: boolean;
set replaceUrl(replaceUrl: boolean | string | null | undefined);
// (undocumented)
get replaceUrl(): boolean;
set routerLink(commands: any[] | string | null | undefined);
skipLocationChange: boolean;
set skipLocationChange(skipLocationChange: boolean | string | null | undefined);
// (undocumented)
get skipLocationChange(): boolean;
state?: {
[k: string]: any;
};
Expand All @@ -750,7 +762,7 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy {
// (undocumented)
get urlTree(): UrlTree | null;
// (undocumented)
static ɵdir: i0.ɵɵDirectiveDeclaration<RouterLinkWithHref, "a[routerLink],area[routerLink]", never, { "target": "target"; "queryParams": "queryParams"; "fragment": "fragment"; "queryParamsHandling": "queryParamsHandling"; "preserveFragment": "preserveFragment"; "skipLocationChange": "skipLocationChange"; "replaceUrl": "replaceUrl"; "state": "state"; "relativeTo": "relativeTo"; "routerLink": "routerLink"; }, {}, never, never, true>;
static ɵdir: i0.ɵɵDirectiveDeclaration<RouterLinkWithHref, "a[routerLink],area[routerLink]", never, { "target": "target"; "queryParams": "queryParams"; "fragment": "fragment"; "queryParamsHandling": "queryParamsHandling"; "state": "state"; "relativeTo": "relativeTo"; "preserveFragment": "preserveFragment"; "skipLocationChange": "skipLocationChange"; "replaceUrl": "replaceUrl"; "routerLink": "routerLink"; }, {}, never, never, true>;
// (undocumented)
static ɵfac: i0.ɵɵFactoryDeclaration<RouterLinkWithHref, never>;
}
Expand Down
158 changes: 104 additions & 54 deletions packages/router/src/directives/router_link.ts
View file
Expand Up @@ -120,6 +120,10 @@ import {UrlTree} from '../url_tree';
standalone: true,
})
export class RouterLink implements OnChanges {
private _preserveFragment = false;
private _skipLocationChange = false;
private _replaceUrl = false;

/**
* Passed to {@link Router#createUrlTree Router#createUrlTree} as part of the
* `UrlCreationOptions`.
Expand All @@ -141,30 +145,6 @@ export class RouterLink implements OnChanges {
* @see {@link Router#createUrlTree Router#createUrlTree}
*/
@Input() queryParamsHandling?: QueryParamsHandling|null;
/**
* Passed to {@link Router#createUrlTree Router#createUrlTree} as part of the
* `UrlCreationOptions`.
* @see {@link UrlCreationOptions#preserveFragment UrlCreationOptions#preserveFragment}
* @see {@link Router#createUrlTree Router#createUrlTree}
*/
// TODO(issue/24571): remove '!'.
@Input() preserveFragment!: boolean;
/**
* Passed to {@link Router#navigateByUrl Router#navigateByUrl} as part of the
* `NavigationBehaviorOptions`.
* @see {@link NavigationBehaviorOptions#skipLocationChange NavigationBehaviorOptions#skipLocationChange}
* @see {@link Router#navigateByUrl Router#navigateByUrl}
*/
// TODO(issue/24571): remove '!'.
@Input() skipLocationChange!: boolean;
/**
* Passed to {@link Router#navigateByUrl Router#navigateByUrl} as part of the
* `NavigationBehaviorOptions`.
* @see {@link NavigationBehaviorOptions#replaceUrl NavigationBehaviorOptions#replaceUrl}
* @see {@link Router#navigateByUrl Router#navigateByUrl}
*/
// TODO(issue/24571): remove '!'.
@Input() replaceUrl!: boolean;
/**
* Passed to {@link Router#navigateByUrl Router#navigateByUrl} as part of the
* `NavigationBehaviorOptions`.
Expand Down Expand Up @@ -195,6 +175,51 @@ export class RouterLink implements OnChanges {
this.setTabIndexIfNotOnNativeEl('0');
}

/**
* Passed to {@link Router#createUrlTree Router#createUrlTree} as part of the
* `UrlCreationOptions`.
* @see {@link UrlCreationOptions#preserveFragment UrlCreationOptions#preserveFragment}
* @see {@link Router#createUrlTree Router#createUrlTree}
*/
@Input()
set preserveFragment(preserveFragment: boolean|string|null|undefined) {
this._preserveFragment = coerceToBoolean(preserveFragment);
}

get preserveFragment(): boolean {
return this._preserveFragment;
}

/**
* Passed to {@link Router#navigateByUrl Router#navigateByUrl} as part of the
* `NavigationBehaviorOptions`.
* @see {@link NavigationBehaviorOptions#skipLocationChange NavigationBehaviorOptions#skipLocationChange}
* @see {@link Router#navigateByUrl Router#navigateByUrl}
*/
@Input()
set skipLocationChange(skipLocationChange: boolean|string|null|undefined) {
this._skipLocationChange = coerceToBoolean(skipLocationChange);
}

get skipLocationChange(): boolean {
return this._skipLocationChange;
}

/**
* Passed to {@link Router#navigateByUrl Router#navigateByUrl} as part of the
* `NavigationBehaviorOptions`.
* @see {@link NavigationBehaviorOptions#replaceUrl NavigationBehaviorOptions#replaceUrl}
* @see {@link Router#navigateByUrl Router#navigateByUrl}
*/
@Input()
set replaceUrl(replaceUrl: boolean|string|null|undefined) {
this._replaceUrl = coerceToBoolean(replaceUrl);
}

get replaceUrl(): boolean {
return this._replaceUrl;
}

/**
* Modifies the tab index if there was not a tabindex attribute on the element during
* instantiation.
Expand Down Expand Up @@ -245,8 +270,8 @@ export class RouterLink implements OnChanges {
}

const extras = {
skipLocationChange: coerceToBoolean(this.skipLocationChange),
replaceUrl: coerceToBoolean(this.replaceUrl),
skipLocationChange: this.skipLocationChange,
replaceUrl: this.replaceUrl,
state: this.state,
};
this.router.navigateByUrl(this.urlTree, extras);
Expand All @@ -264,7 +289,7 @@ export class RouterLink implements OnChanges {
queryParams: this.queryParams,
fragment: this.fragment,
queryParamsHandling: this.queryParamsHandling,
preserveFragment: coerceToBoolean(this.preserveFragment),
preserveFragment: this.preserveFragment,
});
}
}
Expand All @@ -282,6 +307,10 @@ export class RouterLink implements OnChanges {
*/
@Directive({selector: 'a[routerLink],area[routerLink]', standalone: true})
export class RouterLinkWithHref implements OnChanges, OnDestroy {
private _preserveFragment = false;
private _skipLocationChange = false;
private _replaceUrl = false;

// TODO(issue/24571): remove '!'.
@HostBinding('attr.target') @Input() target!: string;
/**
Expand All @@ -305,30 +334,6 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy {
* @see {@link Router#createUrlTree Router#createUrlTree}
*/
@Input() queryParamsHandling?: QueryParamsHandling|null;
/**
* Passed to {@link Router#createUrlTree Router#createUrlTree} as part of the
* `UrlCreationOptions`.
* @see {@link UrlCreationOptions#preserveFragment UrlCreationOptions#preserveFragment}
* @see {@link Router#createUrlTree Router#createUrlTree}
*/
// TODO(issue/24571): remove '!'.
@Input() preserveFragment!: boolean;
/**
* Passed to {@link Router#navigateByUrl Router#navigateByUrl} as part of the
* `NavigationBehaviorOptions`.
* @see {@link NavigationBehaviorOptions#skipLocationChange NavigationBehaviorOptions#skipLocationChange}
* @see {@link Router#navigateByUrl Router#navigateByUrl}
*/
// TODO(issue/24571): remove '!'.
@Input() skipLocationChange!: boolean;
/**
* Passed to {@link Router#navigateByUrl Router#navigateByUrl} as part of the
* `NavigationBehaviorOptions`.
* @see {@link NavigationBehaviorOptions#replaceUrl NavigationBehaviorOptions#replaceUrl}
* @see {@link Router#navigateByUrl Router#navigateByUrl}
*/
// TODO(issue/24571): remove '!'.
@Input() replaceUrl!: boolean;
/**
* Passed to {@link Router#navigateByUrl Router#navigateByUrl} as part of the
* `NavigationBehaviorOptions`.
Expand Down Expand Up @@ -368,6 +373,51 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy {
});
}

/**
* Passed to {@link Router#createUrlTree Router#createUrlTree} as part of the
* `UrlCreationOptions`.
* @see {@link UrlCreationOptions#preserveFragment UrlCreationOptions#preserveFragment}
* @see {@link Router#createUrlTree Router#createUrlTree}
*/
@Input()
set preserveFragment(preserveFragment: boolean|string|null|undefined) {
this._preserveFragment = coerceToBoolean(preserveFragment);
}

get preserveFragment(): boolean {
return this._preserveFragment;
}

/**
* Passed to {@link Router#navigateByUrl Router#navigateByUrl} as part of the
* `NavigationBehaviorOptions`.
* @see {@link NavigationBehaviorOptions#skipLocationChange NavigationBehaviorOptions#skipLocationChange}
* @see {@link Router#navigateByUrl Router#navigateByUrl}
*/
@Input()
set skipLocationChange(skipLocationChange: boolean|string|null|undefined) {
this._skipLocationChange = coerceToBoolean(skipLocationChange);
}

get skipLocationChange(): boolean {
return this._skipLocationChange;
}

/**
* Passed to {@link Router#navigateByUrl Router#navigateByUrl} as part of the
* `NavigationBehaviorOptions`.
* @see {@link NavigationBehaviorOptions#replaceUrl NavigationBehaviorOptions#replaceUrl}
* @see {@link Router#navigateByUrl Router#navigateByUrl}
*/
@Input()
set replaceUrl(replaceUrl: boolean|string|null|undefined) {
this._replaceUrl = coerceToBoolean(replaceUrl);
}

get replaceUrl(): boolean {
return this._replaceUrl;
}

/**
* Commands to pass to {@link Router#createUrlTree Router#createUrlTree}.
* - **array**: commands to pass to {@link Router#createUrlTree Router#createUrlTree}.
Expand Down Expand Up @@ -409,8 +459,8 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy {
}

const extras = {
skipLocationChange: coerceToBoolean(this.skipLocationChange),
replaceUrl: coerceToBoolean(this.replaceUrl),
skipLocationChange: this.skipLocationChange,
replaceUrl: this.replaceUrl,
state: this.state
};
this.router.navigateByUrl(this.urlTree, extras);
Expand All @@ -434,7 +484,7 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy {
queryParams: this.queryParams,
fragment: this.fragment,
queryParamsHandling: this.queryParamsHandling,
preserveFragment: coerceToBoolean(this.preserveFragment),
preserveFragment: this.preserveFragment,
});
}
}
47 changes: 46 additions & 1 deletion packages/router/test/router_link_spec.ts
View file
Expand Up @@ -9,7 +9,7 @@
import {Component} from '@angular/core';
import {ComponentFixture, TestBed} from '@angular/core/testing';
import {By} from '@angular/platform-browser';
import {Router} from '@angular/router';
import {Router, RouterLink, RouterLinkWithHref} from '@angular/router';
import {RouterTestingModule} from '@angular/router/testing';

describe('RouterLink', () => {
Expand Down Expand Up @@ -69,6 +69,28 @@ describe('RouterLink', () => {
link.click();
expect(router.navigateByUrl).not.toHaveBeenCalled();
});

it('should coerce boolean input values', () => {
const dir = fixture.debugElement.query(By.directive(RouterLink)).injector.get(RouterLink);

for (const truthy of [true, '', 'true', 'anything']) {
dir.preserveFragment = truthy;
dir.skipLocationChange = truthy;
dir.replaceUrl = truthy;
expect(dir.preserveFragment).toBeTrue();
expect(dir.skipLocationChange).toBeTrue();
expect(dir.replaceUrl).toBeTrue();
}

for (const falsy of [false, null, undefined, 'false']) {
dir.preserveFragment = falsy;
dir.skipLocationChange = falsy;
dir.replaceUrl = falsy;
expect(dir.preserveFragment).toBeFalse();
expect(dir.skipLocationChange).toBeFalse();
expect(dir.replaceUrl).toBeFalse();
}
});
});

describe('RouterLinkWithHref', () => {
Expand Down Expand Up @@ -100,5 +122,28 @@ describe('RouterLink', () => {
fixture.detectChanges();
expect(link.outerHTML).not.toContain('href');
});

it('should coerce boolean input values', () => {
const dir = fixture.debugElement.query(By.directive(RouterLinkWithHref))
.injector.get(RouterLinkWithHref);

for (const truthy of [true, '', 'true', 'anything']) {
dir.preserveFragment = truthy;
dir.skipLocationChange = truthy;
dir.replaceUrl = truthy;
expect(dir.preserveFragment).toBeTrue();
expect(dir.skipLocationChange).toBeTrue();
expect(dir.replaceUrl).toBeTrue();
}

for (const falsy of [false, null, undefined, 'false']) {
dir.preserveFragment = falsy;
dir.skipLocationChange = falsy;
dir.replaceUrl = falsy;
expect(dir.preserveFragment).toBeFalse();
expect(dir.skipLocationChange).toBeFalse();
expect(dir.replaceUrl).toBeFalse();
}
});
});
});

0 comments on commit 422323c

Please sign in to comment.