fix(router): prevent RouterLinkActive from causing an infinite CD loop

fixes #15825
angular · Apr 21, 2017 · 4479c42 · 4479c42
1 parent f92b6b6
commit 4479c42
Show file tree

Hide file tree

Showing 3 changed files with 78 additions and 5 deletions.
diff --git 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
@@ -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
@@ -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: `
+        <div *ngIf="show">
+          <ng-container *ngTemplateOutlet="tpl"></ng-container>
+        </div>
+        <router-outlet></router-outlet>
+        <ng-template #tpl>
+          <a routerLink="/simple" routerLinkActive="active"></a>
+        </ng-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<T>(fixture: ComponentFixture<T>): void {
+  tick();
+  fixture.detectChanges();
+}
+
+function createRoot<T>(router: Router, type: Type<T>): ComponentFixture<T> {
+  const f = TestBed.createComponent(type);
+  advance(f);
+  router.initialNavigation();
+  advance(f);
+  return f;
+}