From 82417b3ca59aa2e5f41ee12db4cb18970b5b4f47 Mon Sep 17 00:00:00 2001 From: Victor Berchet Date: Wed, 12 Apr 2017 16:31:02 -0700 Subject: [PATCH] fix(router): prevent `RouterLinkActive` from causing an infinite CD loop fixes #15825 --- .../src/directives/router_link_active.ts | 3 +- packages/router/test/integration.spec.ts | 6 +- .../test/regression_integration.spec.ts | 74 +++++++++++++++++++ 3 files changed, 78 insertions(+), 5 deletions(-) create mode 100644 packages/router/test/regression_integration.spec.ts diff --git a/packages/router/src/directives/router_link_active.ts b/packages/router/src/directives/router_link_active.ts index be6288a296..27b07cf900 100644 --- a/packages/router/src/directives/router_link_active.ts +++ b/packages/router/src/directives/router_link_active.ts @@ -123,10 +123,9 @@ export class RouterLinkActive implements OnChanges, // 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(); + Promise.resolve(hasActiveLinks).then(active => this.active = active); } } diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts index 68612befd8..1a98a5dab2 100644 --- a/packages/router/test/integration.spec.ts +++ b/packages/router/test/integration.spec.ts @@ -11,11 +11,10 @@ import {Component, Injectable, NgModule, NgModuleFactoryLoader, NgModuleRef} fro import {ComponentFixture, TestBed, fakeAsync, inject, tick} from '@angular/core/testing'; import {By} from '@angular/platform-browser/src/dom/debug/by'; import {expect} from '@angular/platform-browser/testing/src/matchers'; +import {ActivatedRoute, ActivatedRouteSnapshot, CanActivate, CanDeactivate, DetachedRouteHandle, Event, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, PRIMARY_OUTLET, ParamMap, Params, PreloadAllModules, PreloadingStrategy, Resolve, RouteConfigLoadEnd, RouteConfigLoadStart, RouteReuseStrategy, Router, RouterModule, RouterPreloader, RouterStateSnapshot, RoutesRecognized, RunGuardsAndResolvers, UrlHandlingStrategy, UrlSegmentGroup, UrlTree} from '@angular/router'; import {Observable} from 'rxjs/Observable'; import {map} from 'rxjs/operator/map'; -import {ActivatedRoute, ActivatedRouteSnapshot, CanActivate, CanDeactivate, DetachedRouteHandle, Event, NavigationCancel, NavigationEnd, NavigationError, NavigationStart, PRIMARY_OUTLET, ParamMap, Params, PreloadAllModules, PreloadingStrategy, Resolve, RouteConfigLoadEnd, RouteConfigLoadStart, RouteReuseStrategy, Router, RouterModule, RouterStateSnapshot, RoutesRecognized, RunGuardsAndResolvers, UrlHandlingStrategy, UrlSegmentGroup, UrlTree} from '../index'; -import {RouterPreloader} from '../src/router_preloader'; import {forEach} from '../src/utils/collection'; import {RouterTestingModule, SpyNgModuleFactoryLoader} from '../testing'; @@ -2470,13 +2469,14 @@ describe('Integration', () => { const fixture = TestBed.createComponent(ComponentWithRouterLink); router.navigateByUrl('/team'); expect(() => advance(fixture)).not.toThrow(); + advance(fixture); const paragraph = fixture.nativeElement.querySelector('p'); expect(paragraph.textContent).toEqual('true'); router.navigateByUrl('/otherteam'); advance(fixture); - + advance(fixture); expect(paragraph.textContent).toEqual('false'); })); diff --git a/packages/router/test/regression_integration.spec.ts b/packages/router/test/regression_integration.spec.ts new file mode 100644 index 0000000000..ddfcd2af38 --- /dev/null +++ b/packages/router/test/regression_integration.spec.ts @@ -0,0 +1,74 @@ +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +import {CommonModule} from '@angular/common'; +import {Component, NgModule, Type} from '@angular/core'; +import {ComponentFixture, TestBed, fakeAsync, tick} from '@angular/core/testing'; +import {Router} from '@angular/router'; +import {RouterTestingModule} from '@angular/router/testing'; + +describe('Integration', () => { + + describe('routerLinkActive', () => { + it('should not cause infinite loops in the change detection - #15825', fakeAsync(() => { + @Component({selector: 'simple', template: 'simple'}) + class SimpleCmp { + } + + @Component({ + selector: 'some-root', + template: ` +
+ +
+ + + + ` + }) + class MyCmp { + show: boolean = false; + } + + @NgModule({ + imports: [CommonModule, RouterTestingModule], + declarations: [MyCmp, SimpleCmp], + entryComponents: [SimpleCmp], + }) + class MyModule { + } + + TestBed.configureTestingModule({imports: [MyModule]}); + + const router: Router = TestBed.get(Router); + const fixture = createRoot(router, MyCmp); + router.resetConfig([{path: 'simple', component: SimpleCmp}]); + + router.navigateByUrl('/simple'); + advance(fixture); + + const instance = fixture.componentInstance; + instance.show = true; + expect(() => advance(fixture)).not.toThrow(); + })); + }); + +}); + +function advance(fixture: ComponentFixture): void { + tick(); + fixture.detectChanges(); +} + +function createRoot(router: Router, type: Type): ComponentFixture { + const f = TestBed.createComponent(type); + advance(f); + router.initialNavigation(); + advance(f); + return f; +}