Revert "fix(router): ensure routerLinkActive updates when associated routerLinks change (angular#38511)"

This reverts commit 594eff1.

Author: subratpalhar92

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

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<RouterLink>();
-
   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(this);
-  }
-
   /**
    * 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<RouterLinkWithHref>();
-
   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(this);
   }
   /** @nodoc */
   ngOnDestroy(): any {

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<RouterLink>;
+  // TODO(issue/24571): remove '!'.
   @ContentChildren(RouterLinkWithHref, {descendants: true})
   linksWithHrefs!: QueryList<RouterLinkWithHref>;
 
   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,26 +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(link => {
-      if (this.isActive !== this.isLinkActive(this.router)(link)) {
-        this.update();
-      }
-    });
+    this.links.changes.subscribe(_ => this.update());
+    this.linksWithHrefs.changes.subscribe(_ => this.update());
+    this.update();
   }
 
   @Input()
@@ -138,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 {

packages/router/test/integration.spec.ts

@@ -4344,7 +4344,7 @@ describe('Integration', () => {
        })));
   });
 
-  describe('routerLinkActive', () => {
+  describe('routerActiveLink', () => {
     it('should set the class when the link is active (a tag)',
        fakeAsync(inject([Router, Location], (router: Router, location: Location) => {
          const fixture = createRoot(router, RootCmp);
@@ -4494,29 +4494,6 @@ describe('Integration', () => {
          advance(fixture);
          expect(paragraph.textContent).toEqual('false');
        }));
-
-    it('should not trigger change detection when active state has not changed', fakeAsync(() => {
-         @Component({
-           template: `<div id="link" routerLinkActive="active" [routerLink]="link"></div>`,
-         })
-         class LinkComponent {
-           link = 'notactive';
-         }
-
-         @Component({template: ''})
-         class SimpleComponent {
-         }
-
-         TestBed.configureTestingModule({
-           imports: [RouterTestingModule.withRoutes([{path: '', component: SimpleComponent}])],
-           declarations: [LinkComponent, SimpleComponent]
-         });
-
-         const fixture = createRoot(TestBed.inject(Router), LinkComponent);
-         fixture.componentInstance.link = 'stillnotactive';
-         fixture.detectChanges(false /** checkNoChanges */);
-         expect(TestBed.inject(NgZone).hasPendingMicrotasks).toBe(false);
-       }));
   });
 
   describe('lazy loading', () => {

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: `
-          <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 {