From 80d0067048e4568cb20ddf898122984d19e35707 Mon Sep 17 00:00:00 2001 From: Manduro Date: Fri, 23 Feb 2018 09:48:19 +0100 Subject: [PATCH] fix(router): `RouterLinkActive` should run CD when setting `isActive` (#21411) When using the routerLinkActive directive inside a component that is using ChangeDetectionStrategy.OnPush and lazy loaded module routes the routerLinkActive directive does not update after clicking a link to a lazy loaded route that has not already been loaded. Also the OnPush nav component does not set routerLinkActive correctly when the default route loads, the non-OnPush nav component works fine. regression caused by #15943 closes #19934 PR Close #21411 --- goldens/public-api/router/router.d.ts | 2 +- .../src/directives/router_link_active.ts | 5 +-- .../test/regression_integration.spec.ts | 31 ++++++++++++++++++- 3 files changed, 34 insertions(+), 4 deletions(-) diff --git a/goldens/public-api/router/router.d.ts b/goldens/public-api/router/router.d.ts index 4add286dfd..f83df4d485 100644 --- a/goldens/public-api/router/router.d.ts +++ b/goldens/public-api/router/router.d.ts @@ -398,7 +398,7 @@ export declare class RouterLinkActive implements OnChanges, OnDestroy, AfterCont routerLinkActiveOptions: { exact: boolean; }; - constructor(router: Router, element: ElementRef, renderer: Renderer2, link?: RouterLink | undefined, linkWithHref?: RouterLinkWithHref | undefined); + constructor(router: Router, element: ElementRef, renderer: Renderer2, cdr: ChangeDetectorRef, link?: RouterLink | undefined, linkWithHref?: RouterLinkWithHref | undefined); ngAfterContentInit(): void; ngOnChanges(changes: SimpleChanges): void; ngOnDestroy(): void; diff --git a/packages/router/src/directives/router_link_active.ts b/packages/router/src/directives/router_link_active.ts index 16272ef428..2e5a898665 100644 --- a/packages/router/src/directives/router_link_active.ts +++ b/packages/router/src/directives/router_link_active.ts @@ -6,7 +6,7 @@ * found in the LICENSE file at https://angular.io/license */ -import {AfterContentInit, ContentChildren, Directive, ElementRef, Input, OnChanges, OnDestroy, Optional, QueryList, Renderer2, SimpleChanges} from '@angular/core'; +import {AfterContentInit, ChangeDetectorRef, ContentChildren, Directive, ElementRef, Input, OnChanges, OnDestroy, Optional, QueryList, Renderer2, SimpleChanges} from '@angular/core'; import {Subscription} from 'rxjs'; import {Event, NavigationEnd} from '../events'; @@ -91,7 +91,7 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit constructor( private router: Router, private element: ElementRef, private renderer: Renderer2, - @Optional() private link?: RouterLink, + private readonly cdr: ChangeDetectorRef, @Optional() private link?: RouterLink, @Optional() private linkWithHref?: RouterLinkWithHref) { this.subscription = router.events.subscribe((s: Event) => { if (s instanceof NavigationEnd) { @@ -126,6 +126,7 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit const hasActiveLinks = this.hasActiveLinks(); if (this.isActive !== hasActiveLinks) { (this as any).isActive = hasActiveLinks; + this.cdr.markForCheck(); this.classes.forEach((c) => { if (hasActiveLinks) { this.renderer.addClass(this.element.nativeElement, c); diff --git a/packages/router/test/regression_integration.spec.ts b/packages/router/test/regression_integration.spec.ts index ef69900ffa..3dbf4eaf68 100644 --- a/packages/router/test/regression_integration.spec.ts +++ b/packages/router/test/regression_integration.spec.ts @@ -7,7 +7,7 @@ */ import {CommonModule} from '@angular/common'; -import {Component, ContentChild, NgModule, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core'; +import {ChangeDetectionStrategy, Component, ContentChild, NgModule, TemplateRef, Type, ViewChild, ViewContainerRef} from '@angular/core'; import {ComponentFixture, fakeAsync, TestBed, tick} from '@angular/core/testing'; import {Router} from '@angular/router'; import {RouterTestingModule} from '@angular/router/testing'; @@ -109,6 +109,35 @@ describe('Integration', () => { expect(fixture.nativeElement.innerHTML).toContain('isActive: false'); })); + + it('should set isActive with OnPush change detection - #19934', fakeAsync(() => { + @Component({ + template: ` +
+ isActive: {{rla.isActive}} +
+ `, + changeDetection: ChangeDetectionStrategy.OnPush + }) + class OnPushComponent { + } + + @Component({template: 'simple'}) + class SimpleCmp { + } + + TestBed.configureTestingModule({ + imports: [RouterTestingModule.withRoutes([{path: 'simple', component: SimpleCmp}])], + declarations: [OnPushComponent, SimpleCmp] + }); + + const router: Router = TestBed.get(Router); + const fixture = createRoot(router, OnPushComponent); + router.navigateByUrl('/simple'); + advance(fixture); + + expect(fixture.nativeElement.innerHTML).toContain('isActive: true'); + })); }); });