diff --git a/goldens/public-api/router/router.d.ts b/goldens/public-api/router/router.d.ts index 8e88ac7b7c..5d911194bd 100644 --- a/goldens/public-api/router/router.d.ts +++ b/goldens/public-api/router/router.d.ts @@ -378,7 +378,7 @@ export declare class RouterEvent { url: string); } -export declare class RouterLink { +export declare class RouterLink implements OnChanges { fragment: string; preserveFragment: boolean; /** @deprecated */ set preserveQueryParams(value: boolean); @@ -394,6 +394,7 @@ export declare class RouterLink { }; get urlTree(): UrlTree; constructor(router: Router, route: ActivatedRoute, tabIndex: string, renderer: Renderer2, el: ElementRef); + ngOnChanges(changes: SimpleChanges): void; onClick(): boolean; } @@ -429,7 +430,7 @@ export declare class RouterLinkWithHref implements OnChanges, OnDestroy { target: string; get urlTree(): UrlTree; constructor(router: Router, route: ActivatedRoute, locationStrategy: LocationStrategy); - ngOnChanges(changes: {}): any; + ngOnChanges(changes: SimpleChanges): any; ngOnDestroy(): any; onClick(button: number, ctrlKey: boolean, metaKey: boolean, shiftKey: boolean): boolean; } diff --git a/packages/router/src/directives/router_link.ts b/packages/router/src/directives/router_link.ts index 58be39a05a..997319252a 100644 --- a/packages/router/src/directives/router_link.ts +++ b/packages/router/src/directives/router_link.ts @@ -7,8 +7,8 @@ */ import {LocationStrategy} from '@angular/common'; -import {Attribute, Directive, ElementRef, HostBinding, HostListener, Input, isDevMode, OnChanges, OnDestroy, Renderer2} from '@angular/core'; -import {Subscription} from 'rxjs'; +import {Attribute, Directive, ElementRef, HostBinding, HostListener, Input, isDevMode, OnChanges, OnDestroy, Renderer2, SimpleChanges} from '@angular/core'; +import {Subject, Subscription} from 'rxjs'; import {QueryParamsHandling} from '../config'; import {Event, NavigationEnd} from '../events'; @@ -115,7 +115,7 @@ import {UrlTree} from '../url_tree'; * @publicApi */ @Directive({selector: ':not(a):not(area)[routerLink]'}) -export class RouterLink { +export class RouterLink implements OnChanges { /** * Passed to {@link Router#createUrlTree Router#createUrlTree} as part of the `NavigationExtras`. * @see {@link NavigationExtras#queryParams NavigationExtras#queryParams} @@ -167,6 +167,9 @@ export class RouterLink { private commands: any[] = []; private preserve!: boolean; + /** @internal */ + onChanges = new Subject(); + constructor( private router: Router, private route: ActivatedRoute, @Attribute('tabindex') tabIndex: string, renderer: Renderer2, el: ElementRef) { @@ -175,6 +178,13 @@ export class RouterLink { } } + /** @nodoc */ + ngOnChanges(changes: SimpleChanges) { + // This is subscribed to by `RouterLinkActive` so that it knows to update when there are changes + // to the RouterLinks it's tracking. + this.onChanges.next(); + } + /** * Commands to pass to {@link Router#createUrlTree Router#createUrlTree}. * - **array**: commands to pass to {@link Router#createUrlTree Router#createUrlTree}. @@ -298,6 +308,9 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy { // TODO(issue/24571): remove '!'. @HostBinding() href!: string; + /** @internal */ + onChanges = new Subject(); + constructor( private router: Router, private route: ActivatedRoute, private locationStrategy: LocationStrategy) { @@ -336,8 +349,9 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy { } /** @nodoc */ - ngOnChanges(changes: {}): any { + ngOnChanges(changes: SimpleChanges): any { this.updateTargetUrlAndHref(); + this.onChanges.next(); } /** @nodoc */ ngOnDestroy(): any { diff --git a/packages/router/src/directives/router_link_active.ts b/packages/router/src/directives/router_link_active.ts index 209546d424..7761d91dbc 100644 --- a/packages/router/src/directives/router_link_active.ts +++ b/packages/router/src/directives/router_link_active.ts @@ -7,7 +7,8 @@ */ import {AfterContentInit, ChangeDetectorRef, ContentChildren, Directive, ElementRef, Input, OnChanges, OnDestroy, Optional, QueryList, Renderer2, SimpleChanges} from '@angular/core'; -import {Subscription} from 'rxjs'; +import {from, of, Subscription} from 'rxjs'; +import {mergeAll} from 'rxjs/operators'; import {Event, NavigationEnd} from '../events'; import {Router} from '../router'; @@ -79,14 +80,13 @@ import {RouterLink, RouterLinkWithHref} from './router_link'; exportAs: 'routerLinkActive', }) export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit { - // TODO(issue/24571): remove '!'. @ContentChildren(RouterLink, {descendants: true}) links!: QueryList; - // TODO(issue/24571): remove '!'. @ContentChildren(RouterLinkWithHref, {descendants: true}) linksWithHrefs!: QueryList; private classes: string[] = []; - private subscription: Subscription; + private routerEventsSubscription: Subscription; + private linkInputChangesSubscription?: Subscription; public readonly isActive: boolean = false; @Input() routerLinkActiveOptions: {exact: boolean} = {exact: false}; @@ -95,7 +95,7 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit private router: Router, private element: ElementRef, private renderer: Renderer2, private readonly cdr: ChangeDetectorRef, @Optional() private link?: RouterLink, @Optional() private linkWithHref?: RouterLinkWithHref) { - this.subscription = router.events.subscribe((s: Event) => { + this.routerEventsSubscription = router.events.subscribe((s: Event) => { if (s instanceof NavigationEnd) { this.update(); } @@ -104,9 +104,23 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit /** @nodoc */ ngAfterContentInit(): void { - this.links.changes.subscribe(_ => this.update()); - this.linksWithHrefs.changes.subscribe(_ => this.update()); - this.update(); + // `of(null)` is used to force subscribe body to execute once immediately (like `startWith`). + from([this.links.changes, this.linksWithHrefs.changes, of(null)]) + .pipe(mergeAll()) + .subscribe(_ => { + this.update(); + this.subscribeToEachLinkOnChanges(); + }); + } + + private subscribeToEachLinkOnChanges() { + this.linkInputChangesSubscription?.unsubscribe(); + const allLinkChanges = + [...this.links.toArray(), ...this.linksWithHrefs.toArray(), this.link, this.linkWithHref] + .filter((link): link is RouterLink|RouterLinkWithHref => !!link) + .map(link => link.onChanges); + this.linkInputChangesSubscription = + from(allLinkChanges).pipe(mergeAll()).subscribe(() => this.update()); } @Input() @@ -121,7 +135,8 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit } /** @nodoc */ ngOnDestroy(): void { - this.subscription.unsubscribe(); + this.routerEventsSubscription.unsubscribe(); + this.linkInputChangesSubscription?.unsubscribe(); } private update(): void { diff --git a/packages/router/test/regression_integration.spec.ts b/packages/router/test/regression_integration.spec.ts index 3dbf4eaf68..9dd5b8ba32 100644 --- a/packages/router/test/regression_integration.spec.ts +++ b/packages/router/test/regression_integration.spec.ts @@ -14,6 +14,54 @@ import {RouterTestingModule} from '@angular/router/testing'; describe('Integration', () => { describe('routerLinkActive', () => { + it('should update when the associated routerLinks change - #18469', fakeAsync(() => { + @Component({ + template: ` + {{firstLink}} + + `, + }) + class LinkComponent { + firstLink = 'link-a'; + secondLink = 'link-b'; + + changeLinks(): void { + const temp = this.secondLink; + this.secondLink = this.firstLink; + this.firstLink = temp; + } + } + + @Component({template: 'simple'}) + class SimpleCmp { + } + + TestBed.configureTestingModule({ + imports: [RouterTestingModule.withRoutes( + [{path: 'link-a', component: SimpleCmp}, {path: 'link-b', component: SimpleCmp}])], + declarations: [LinkComponent, SimpleCmp] + }); + + const router: Router = TestBed.inject(Router); + const fixture = createRoot(router, LinkComponent); + const firstLink = fixture.debugElement.query(p => p.nativeElement.id === 'first-link'); + const secondLink = fixture.debugElement.query(p => p.nativeElement.id === 'second-link'); + router.navigateByUrl('/link-a'); + advance(fixture); + + expect(firstLink.nativeElement.classList).toContain('active'); + expect(secondLink.nativeElement.classList).not.toContain('active'); + + fixture.componentInstance.changeLinks(); + fixture.detectChanges(); + advance(fixture); + + expect(firstLink.nativeElement.classList).not.toContain('active'); + expect(secondLink.nativeElement.classList).toContain('active'); + })); + it('should not cause infinite loops in the change detection - #15825', fakeAsync(() => { @Component({selector: 'simple', template: 'simple'}) class SimpleCmp {