fix(router): RouterLinkActive will always remove active classes when links are not active (#54982)

atscott · dylhunn · commit 365fd504077d · 2024-03-27T10:16:23.000-07:00
Previously, `RouterLinkActive` would only add or remove the active classes when its active state changed. This means that if you accidentally add one of the active classes to the static class attribute, it won't get removed until the link becomes active and then deactives (because the class is added at creation time and never removed until the `RouterLinkActive` state changes from active to inactive). fixes #54978 PR Close #54982
diff --git a/packages/router/src/directives/router_link_active.ts b/packages/router/src/directives/router_link_active.ts
@@ -210,28 +210,30 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit
 
   private update(): void {
     if (!this.links || !this.router.navigated) return;
+
     queueMicrotask(() => {
       const hasActiveLinks = this.hasActiveLinks();
-      if (this._isActive !== hasActiveLinks) {
-        this._isActive = hasActiveLinks;
-        this.cdr.markForCheck();
-        this.classes.forEach((c) => {
-          if (hasActiveLinks) {
-            this.renderer.addClass(this.element.nativeElement, c);
-          } else {
-            this.renderer.removeClass(this.element.nativeElement, c);
-          }
-        });
-        if (hasActiveLinks && this.ariaCurrentWhenActive !== undefined) {
-          this.renderer.setAttribute(
-            this.element.nativeElement,
-            'aria-current',
-            this.ariaCurrentWhenActive.toString(),
-          );
+      this.classes.forEach((c) => {
+        if (hasActiveLinks) {
+          this.renderer.addClass(this.element.nativeElement, c);
         } else {
-          this.renderer.removeAttribute(this.element.nativeElement, 'aria-current');
+          this.renderer.removeClass(this.element.nativeElement, c);
         }
+      });
+      if (hasActiveLinks && this.ariaCurrentWhenActive !== undefined) {
+        this.renderer.setAttribute(
+          this.element.nativeElement,
+          'aria-current',
+          this.ariaCurrentWhenActive.toString(),
+        );
+      } else {
+        this.renderer.removeAttribute(this.element.nativeElement, 'aria-current');
+      }
 
+      // Only emit change if the active state changed.
+      if (this._isActive !== hasActiveLinks) {
+        this._isActive = hasActiveLinks;
+        this.cdr.markForCheck();
         // Emit on isActiveChange after classes are updated
         this.isActiveChange.emit(hasActiveLinks);
       }
diff --git a/packages/router/test/router_link_active.spec.ts b/packages/router/test/router_link_active.spec.ts
@@ -0,0 +1,28 @@
+/**
+ * @license
+ * Copyright Google LLC All Rights Reserved.
+ *
+ * Use of this source code is governed by an MIT-style license that can be
+ * found in the LICENSE file at https://angular.io/license
+ */
+import {Component} from '@angular/core';
+import {TestBed} from '@angular/core/testing';
+import {Router, RouterLink, RouterLinkActive, provideRouter} from '@angular/router';
+
+describe('RouterLinkActive', () => {
+  it('removes initial active class even if never active', async () => {
+    @Component({
+      standalone: true,
+      imports: [RouterLinkActive, RouterLink],
+      template: '<a class="active" routerLinkActive="active" routerLink="/abc123"></a>',
+    })
+    class MyCmp {}
+
+    TestBed.configureTestingModule({providers: [provideRouter([{path: '**', children: []}])]});
+    const fixture = TestBed.createComponent(MyCmp);
+    fixture.autoDetectChanges();
+    await TestBed.inject(Router).navigateByUrl('/');
+    await fixture.whenStable();
+    expect(Array.from(fixture.nativeElement.querySelector('a').classList)).toEqual([]);
+  });
+});
