From e8ea741039122af2e8c7bd2bb91f94e4b89dc0c7 Mon Sep 17 00:00:00 2001 From: Dzmitry Shylovich Date: Wed, 7 Dec 2016 03:22:38 +0300 Subject: [PATCH] fix(router): routerLinkActive should not throw when not initialized (#13273) Fixes #13270 PR Close #13273 --- .../src/directives/router_link_active.ts | 37 +++++++++++-------- .../@angular/router/test/integration.spec.ts | 10 +++-- tools/public_api_guard/router/index.d.ts | 2 +- 3 files changed, 29 insertions(+), 20 deletions(-) diff --git a/modules/@angular/router/src/directives/router_link_active.ts b/modules/@angular/router/src/directives/router_link_active.ts index d9a6fcbf08..9384ba424b 100644 --- a/modules/@angular/router/src/directives/router_link_active.ts +++ b/modules/@angular/router/src/directives/router_link_active.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AfterContentInit, ChangeDetectorRef, ContentChildren, Directive, ElementRef, Input, OnChanges, OnDestroy, QueryList, Renderer} from '@angular/core'; +import {AfterContentInit, ChangeDetectorRef, ContentChildren, Directive, ElementRef, Input, OnChanges, OnDestroy, QueryList, Renderer, SimpleChanges} from '@angular/core'; import {Subscription} from 'rxjs/Subscription'; import {NavigationEnd, Router} from '../router'; @@ -14,6 +14,7 @@ import {NavigationEnd, Router} from '../router'; import {RouterLink, RouterLinkWithHref} from './router_link'; + /** * @whatItDoes Lets you add a CSS class to an element when the link's route becomes active. * @@ -82,17 +83,20 @@ import {RouterLink, RouterLinkWithHref} from './router_link'; exportAs: 'routerLinkActive', }) export class RouterLinkActive implements OnChanges, - OnDestroy, AfterContentInit { + OnDestroy, AfterContentInit { @ContentChildren(RouterLink, {descendants: true}) links: QueryList; @ContentChildren(RouterLinkWithHref, {descendants: true}) linksWithHrefs: QueryList; private classes: string[] = []; private subscription: Subscription; + private active: boolean = false; @Input() routerLinkActiveOptions: {exact: boolean} = {exact: false}; - constructor(private router: Router, private element: ElementRef, private renderer: Renderer) { + constructor( + private router: Router, private element: ElementRef, private renderer: Renderer, + private cdr: ChangeDetectorRef) { this.subscription = router.events.subscribe(s => { if (s instanceof NavigationEnd) { this.update(); @@ -100,7 +104,7 @@ export class RouterLinkActive implements OnChanges, }); } - get isActive(): boolean { return this.hasActiveLink(); } + get isActive(): boolean { return this.active; } ngAfterContentInit(): void { this.links.changes.subscribe(_ => this.update()); @@ -110,30 +114,33 @@ export class RouterLinkActive implements OnChanges, @Input() set routerLinkActive(data: string[]|string) { - this.classes = (Array.isArray(data) ? data : data.split(' ')).filter(c => !!c); + const classes = Array.isArray(data) ? data : data.split(' '); + this.classes = classes.filter(c => !!c); } - ngOnChanges(changes: {}): void { this.update(); } + ngOnChanges(changes: SimpleChanges): void { this.update(); } ngOnDestroy(): void { this.subscription.unsubscribe(); } private update(): void { if (!this.links || !this.linksWithHrefs || !this.router.navigated) return; + const hasActiveLinks = this.hasActiveLinks(); - const isActive = this.hasActiveLink(); - this.classes.forEach(c => { - if (c) { - this.renderer.setElementClass(this.element.nativeElement, c, isActive); - } - }); + // react only when status has changed to prevent unnecessary dom updates + if (this.active !== hasActiveLinks) { + this.active = hasActiveLinks; + this.classes.forEach( + c => this.renderer.setElementClass(this.element.nativeElement, c, hasActiveLinks)); + this.cdr.detectChanges(); + } } private isLinkActive(router: Router): (link: (RouterLink|RouterLinkWithHref)) => boolean { return (link: RouterLink | RouterLinkWithHref) => - router.isActive(link.urlTree, this.routerLinkActiveOptions.exact); + router.isActive(link.urlTree, this.routerLinkActiveOptions.exact); } - private hasActiveLink(): boolean { + private hasActiveLinks(): boolean { return this.links.some(this.isLinkActive(this.router)) || - this.linksWithHrefs.some(this.isLinkActive(this.router)); + this.linksWithHrefs.some(this.isLinkActive(this.router)); } } diff --git a/modules/@angular/router/test/integration.spec.ts b/modules/@angular/router/test/integration.spec.ts index cf781887f9..7a6b209064 100644 --- a/modules/@angular/router/test/integration.spec.ts +++ b/modules/@angular/router/test/integration.spec.ts @@ -2096,6 +2096,8 @@ describe('Integration', () => { @Component({ template: `

{{rla.isActive}}

+ + ` }) class ComponentWithRouterLink { @@ -2115,15 +2117,15 @@ describe('Integration', () => { } ]); - const f = TestBed.createComponent(ComponentWithRouterLink); + const fixture = TestBed.createComponent(ComponentWithRouterLink); router.navigateByUrl('/team'); - advance(f); + expect(() => advance(fixture)).not.toThrow(); - const paragraph = f.nativeElement.querySelector('p'); + const paragraph = fixture.nativeElement.querySelector('p'); expect(paragraph.textContent).toEqual('true'); router.navigateByUrl('/otherteam'); - advance(f); + advance(fixture); expect(paragraph.textContent).toEqual('false'); })); diff --git a/tools/public_api_guard/router/index.d.ts b/tools/public_api_guard/router/index.d.ts index 9c4da71bb8..704ceb2c62 100644 --- a/tools/public_api_guard/router/index.d.ts +++ b/tools/public_api_guard/router/index.d.ts @@ -264,7 +264,7 @@ export declare class RouterLinkActive implements OnChanges, OnDestroy, AfterCont }; constructor(router: Router, element: ElementRef, renderer: Renderer, cdr: ChangeDetectorRef); ngAfterContentInit(): void; - ngOnChanges(changes: {}): void; + ngOnChanges(changes: SimpleChanges): void; ngOnDestroy(): void; }