fix(router): ensure routerLinkActive updates when associated routerLinks change (#38349)

This commit introduces a new subscription in the `routerLinkActive` directive which triggers an update
when any of its associated routerLinks have changes. `RouterLinkActive` not only needs to know when
links are added or removed, but it also needs to know about if a link it already knows about
changes in some way.

Quick note that `from...mergeAll` is used instead of just a simple
`merge` (or `scheduled...mergeAll`) to avoid introducing new rxjs
operators in order to keep bundle size down.

Fixes #18469

PR Close #38349
This commit is contained in:
Andrew Scott 2020-08-05 10:34:12 -07:00 committed by Andrew Scott
parent cfe424e875
commit e0e5c9f195
4 changed files with 93 additions and 15 deletions

View File

@ -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;
}

View File

@ -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<void>();
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<void>();
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 {

View File

@ -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<RouterLink>;
// TODO(issue/24571): remove '!'.
@ContentChildren(RouterLinkWithHref, {descendants: true})
linksWithHrefs!: QueryList<RouterLinkWithHref>;
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());
// `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 {

View File

@ -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: `
<a id="first-link" [routerLink]="[firstLink]" routerLinkActive="active">{{firstLink}}</a>
<div id="second-link" routerLinkActive="active">
<a [routerLink]="[secondLink]">{{secondLink}}</a>
</div>
`,
})
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 {