fix(router): RouterLinkActive should update its state right after che…

…cking the children Closes #18983
angular · Oct 18, 2017 · 53a807a · 53a807a
1 parent 3342a82
commit 53a807a
Show file tree

Hide file tree

Showing 3 changed files with 68 additions and 14 deletions.
diff --git a/packages/router/src/directives/router_link_active.ts b/packages/router/src/directives/router_link_active.ts
@@ -119,19 +119,19 @@ export class RouterLinkActive implements OnChanges,
 
   private update(): void {
     if (!this.links || !this.linksWithHrefs || !this.router.navigated) return;
-    const hasActiveLinks = this.hasActiveLinks();
-
-    // react only when status has changed to prevent unnecessary dom updates
-    if (this.active !== 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.active = active);
-    }
+    Promise.resolve().then(() => {
+      const hasActiveLinks = this.hasActiveLinks();
+      if (this.active !== hasActiveLinks) {
+        this.active = hasActiveLinks;
+        this.classes.forEach((c) => {
+          if (hasActiveLinks) {
+            this.renderer.addClass(this.element.nativeElement, c);
+          } else {
+            this.renderer.removeClass(this.element.nativeElement, c);
+          }
+        });
+      }
+    });
   }
 
   private isLinkActive(router: Router): (link: (RouterLink|RouterLinkWithHref)) => boolean {

diff --git a/packages/router/test/integration.spec.ts b/packages/router/test/integration.spec.ts
@@ -2572,6 +2572,7 @@ 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');
@@ -2628,6 +2629,7 @@ 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');
@@ -2656,6 +2658,7 @@ 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
@@ -7,8 +7,9 @@
  */
 
 import {CommonModule} from '@angular/common';
-import {Component, NgModule, Type} from '@angular/core';
+import {Component, ContentChild, NgModule, TemplateRef, Type, ViewChild, ViewContainerRef} 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';
 
@@ -56,6 +57,56 @@ describe('Integration', () => {
          instance.show = true;
          expect(() => advance(fixture)).not.toThrow();
        }));
+
+    it('should set isActive right after looking at its children -- #18983', fakeAsync(() => {
+         @Component({
+           template: `
+          <div #rla="routerLinkActive" routerLinkActive>
+            isActive: {{rla.isActive}}
+            
+            <ng-template let-data>
+              <a [routerLink]="data">link</a>
+            </ng-template>
+
+            <ng-container #container></ng-container>
+          </div>
+        `
+         })
+         class ComponentWithRouterLink {
+           @ViewChild(TemplateRef) templateRef: TemplateRef<any>;
+           @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');
+       }));
+
   });
 
 });