From b0c7ea8181cd5b82e42194c611856f6880b1edf8 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Wed, 18 Oct 2017 09:58:41 -0700 Subject: [PATCH] Revert "fix(router): RouterLinkActive should update its state right after checking the children (#19449)" This reverts commit c569b7524974200040c2abae6923cd4e6f59a13b. As it was synched together with 5a9ed2de272912ac74ed56bfec4bdf4551f7b38e which broke an internal test. --- .../src/directives/router_link_active.ts | 28 +++++----- packages/router/test/integration.spec.ts | 3 -- .../test/regression_integration.spec.ts | 53 +------------------ 3 files changed, 16 insertions(+), 68 deletions(-) diff --git a/packages/router/src/directives/router_link_active.ts b/packages/router/src/directives/router_link_active.ts index 5d548cabbc..07c6e09217 100644 --- a/packages/router/src/directives/router_link_active.ts +++ b/packages/router/src/directives/router_link_active.ts @@ -118,19 +118,21 @@ export class RouterLinkActive implements OnChanges, private update(): void { if (!this.links || !this.linksWithHrefs || !this.router.navigated) return; - Promise.resolve().then(() => { - const hasActiveLinks = this.hasActiveLinks(); - if (this.isActive !== hasActiveLinks) { - (this as any).isActive = hasActiveLinks; - this.classes.forEach((c) => { - if (hasActiveLinks) { - this.renderer.addClass(this.element.nativeElement, c); - } else { - this.renderer.removeClass(this.element.nativeElement, c); - } - }); - } - }); + const hasActiveLinks = this.hasActiveLinks(); + + // react only when status has changed to prevent unnecessary dom updates + if (this.isActive !== hasActiveLinks) { + this.classes.forEach((c) => { + if (hasActiveLinks) { + this.renderer.addClass(this.element.nativeElement, c); + } else { + this.renderer.removeClass(this.element.nativeElement, c); + } + }); + Promise.resolve(hasActiveLinks).then(active => (this as{ + isActive: boolean + }).isActive = active); + } } private isLinkActive(router: Router): (link: (RouterLink|RouterLinkWithHref)) => boolean { diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts index 722f0ad133..5c78c3e083 100644 --- a/packages/router/test/integration.spec.ts +++ b/packages/router/test/integration.spec.ts @@ -2655,7 +2655,6 @@ describe('Integration', () => { router.navigateByUrl('/team/22/link;exact=true'); advance(fixture); - advance(fixture); expect(location.path()).toEqual('/team/22/link;exact=true'); const nativeLink = fixture.nativeElement.querySelector('a'); @@ -2712,7 +2711,6 @@ describe('Integration', () => { router.navigateByUrl('/team/22/link;exact=true'); advance(fixture); - advance(fixture); expect(location.path()).toEqual('/team/22/link;exact=true'); const native = fixture.nativeElement.querySelector('#link-parent'); @@ -2741,7 +2739,6 @@ describe('Integration', () => { router.navigateByUrl('/team/22/link'); advance(fixture); - advance(fixture); expect(location.path()).toEqual('/team/22/link'); const native = fixture.nativeElement.querySelector('a'); diff --git a/packages/router/test/regression_integration.spec.ts b/packages/router/test/regression_integration.spec.ts index d59c2c36d2..ddfcd2af38 100644 --- a/packages/router/test/regression_integration.spec.ts +++ b/packages/router/test/regression_integration.spec.ts @@ -7,9 +7,8 @@ */ import {CommonModule} from '@angular/common'; -import {Component, ContentChild, NgModule, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core'; +import {Component, NgModule, Type} from '@angular/core'; import {ComponentFixture, TestBed, fakeAsync, tick} from '@angular/core/testing'; -import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter'; import {Router} from '@angular/router'; import {RouterTestingModule} from '@angular/router/testing'; @@ -57,56 +56,6 @@ describe('Integration', () => { instance.show = true; expect(() => advance(fixture)).not.toThrow(); })); - - it('should set isActive right after looking at its children -- #18983', fakeAsync(() => { - @Component({ - template: ` -
- isActive: {{rla.isActive}} - - - link - - - -
- ` - }) - class ComponentWithRouterLink { - @ViewChild(TemplateRef) templateRef: TemplateRef; - @ViewChild('container', {read: ViewContainerRef}) container: ViewContainerRef; - - addLink() { - this.container.createEmbeddedView(this.templateRef, {$implicit: '/simple'}); - } - - removeLink() { this.container.clear(); } - } - - @Component({template: 'simple'}) - class SimpleCmp { - } - - TestBed.configureTestingModule({ - imports: [RouterTestingModule.withRoutes([{path: 'simple', component: SimpleCmp}])], - declarations: [ComponentWithRouterLink, SimpleCmp] - }); - - const router: Router = TestBed.get(Router); - const fixture = createRoot(router, ComponentWithRouterLink); - router.navigateByUrl('/simple'); - advance(fixture); - - fixture.componentInstance.addLink(); - fixture.detectChanges(); - - fixture.componentInstance.removeLink(); - advance(fixture); - advance(fixture); - - expect(fixture.nativeElement.innerHTML).toContain('isActive: false'); - })); - }); });