diff --git a/goldens/public-api/router/router.d.ts b/goldens/public-api/router/router.d.ts index 5d911194bd..8e88ac7b7c 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 implements OnChanges { +export declare class RouterLink { fragment: string; preserveFragment: boolean; /** @deprecated */ set preserveQueryParams(value: boolean); @@ -394,7 +394,6 @@ export declare class RouterLink implements OnChanges { }; get urlTree(): UrlTree; constructor(router: Router, route: ActivatedRoute, tabIndex: string, renderer: Renderer2, el: ElementRef); - ngOnChanges(changes: SimpleChanges): void; onClick(): boolean; } @@ -430,7 +429,7 @@ export declare class RouterLinkWithHref implements OnChanges, OnDestroy { target: string; get urlTree(): UrlTree; constructor(router: Router, route: ActivatedRoute, locationStrategy: LocationStrategy); - ngOnChanges(changes: SimpleChanges): any; + ngOnChanges(changes: {}): 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 997319252a..58be39a05a 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, SimpleChanges} from '@angular/core'; -import {Subject, Subscription} from 'rxjs'; +import {Attribute, Directive, ElementRef, HostBinding, HostListener, Input, isDevMode, OnChanges, OnDestroy, Renderer2} from '@angular/core'; +import {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 implements OnChanges { +export class RouterLink { /** * Passed to {@link Router#createUrlTree Router#createUrlTree} as part of the `NavigationExtras`. * @see {@link NavigationExtras#queryParams NavigationExtras#queryParams} @@ -167,9 +167,6 @@ export class RouterLink implements OnChanges { 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) { @@ -178,13 +175,6 @@ export class RouterLink implements OnChanges { } } - /** @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}. @@ -308,9 +298,6 @@ 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) { @@ -349,9 +336,8 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy { } /** @nodoc */ - ngOnChanges(changes: SimpleChanges): any { + ngOnChanges(changes: {}): 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 7761d91dbc..209546d424 100644 --- a/packages/router/src/directives/router_link_active.ts +++ b/packages/router/src/directives/router_link_active.ts @@ -7,8 +7,7 @@ */ import {AfterContentInit, ChangeDetectorRef, ContentChildren, Directive, ElementRef, Input, OnChanges, OnDestroy, Optional, QueryList, Renderer2, SimpleChanges} from '@angular/core'; -import {from, of, Subscription} from 'rxjs'; -import {mergeAll} from 'rxjs/operators'; +import {Subscription} from 'rxjs'; import {Event, NavigationEnd} from '../events'; import {Router} from '../router'; @@ -80,13 +79,14 @@ 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 routerEventsSubscription: Subscription; - private linkInputChangesSubscription?: Subscription; + private subscription: 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.routerEventsSubscription = router.events.subscribe((s: Event) => { + this.subscription = router.events.subscribe((s: Event) => { if (s instanceof NavigationEnd) { this.update(); } @@ -104,23 +104,9 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit /** @nodoc */ ngAfterContentInit(): void { - // `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()); + this.links.changes.subscribe(_ => this.update()); + this.linksWithHrefs.changes.subscribe(_ => this.update()); + this.update(); } @Input() @@ -135,8 +121,7 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit } /** @nodoc */ ngOnDestroy(): void { - this.routerEventsSubscription.unsubscribe(); - this.linkInputChangesSubscription?.unsubscribe(); + this.subscription.unsubscribe(); } private update(): void { diff --git a/packages/router/test/regression_integration.spec.ts b/packages/router/test/regression_integration.spec.ts index 9dd5b8ba32..3dbf4eaf68 100644 --- a/packages/router/test/regression_integration.spec.ts +++ b/packages/router/test/regression_integration.spec.ts @@ -14,54 +14,6 @@ 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 {