Skip to content

Commit acf692c

Browse files
Revert "fix(router): ensure routerLinkActive updates when associated routerLinks change (angular#38349)"
This reverts commit e7f5e84.
1 parent 44f0ee5 commit acf692c

File tree

4 files changed

+15
-93
lines changed

4 files changed

+15
-93
lines changed

goldens/public-api/router/router.d.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ export declare class RouterEvent {
378378
url: string);
379379
}
380380

381-
export declare class RouterLink implements OnChanges {
381+
export declare class RouterLink {
382382
fragment: string;
383383
preserveFragment: boolean;
384384
/** @deprecated */ set preserveQueryParams(value: boolean);
@@ -394,7 +394,6 @@ export declare class RouterLink implements OnChanges {
394394
};
395395
get urlTree(): UrlTree;
396396
constructor(router: Router, route: ActivatedRoute, tabIndex: string, renderer: Renderer2, el: ElementRef);
397-
ngOnChanges(changes: SimpleChanges): void;
398397
onClick(): boolean;
399398
}
400399

@@ -430,7 +429,7 @@ export declare class RouterLinkWithHref implements OnChanges, OnDestroy {
430429
target: string;
431430
get urlTree(): UrlTree;
432431
constructor(router: Router, route: ActivatedRoute, locationStrategy: LocationStrategy);
433-
ngOnChanges(changes: SimpleChanges): any;
432+
ngOnChanges(changes: {}): any;
434433
ngOnDestroy(): any;
435434
onClick(button: number, ctrlKey: boolean, metaKey: boolean, shiftKey: boolean): boolean;
436435
}

packages/router/src/directives/router_link.ts

Lines changed: 4 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,8 @@
77
*/
88

99
import {LocationStrategy} from '@angular/common';
10-
import {Attribute, Directive, ElementRef, HostBinding, HostListener, Input, isDevMode, OnChanges, OnDestroy, Renderer2, SimpleChanges} from '@angular/core';
11-
import {Subject, Subscription} from 'rxjs';
10+
import {Attribute, Directive, ElementRef, HostBinding, HostListener, Input, isDevMode, OnChanges, OnDestroy, Renderer2} from '@angular/core';
11+
import {Subscription} from 'rxjs';
1212

1313
import {QueryParamsHandling} from '../config';
1414
import {Event, NavigationEnd} from '../events';
@@ -115,7 +115,7 @@ import {UrlTree} from '../url_tree';
115115
* @publicApi
116116
*/
117117
@Directive({selector: ':not(a):not(area)[routerLink]'})
118-
export class RouterLink implements OnChanges {
118+
export class RouterLink {
119119
/**
120120
* Passed to {@link Router#createUrlTree Router#createUrlTree} as part of the `NavigationExtras`.
121121
* @see {@link NavigationExtras#queryParams NavigationExtras#queryParams}
@@ -167,9 +167,6 @@ export class RouterLink implements OnChanges {
167167
private commands: any[] = [];
168168
private preserve!: boolean;
169169

170-
/** @internal */
171-
onChanges = new Subject<void>();
172-
173170
constructor(
174171
private router: Router, private route: ActivatedRoute,
175172
@Attribute('tabindex') tabIndex: string, renderer: Renderer2, el: ElementRef) {
@@ -178,13 +175,6 @@ export class RouterLink implements OnChanges {
178175
}
179176
}
180177

181-
/** @nodoc */
182-
ngOnChanges(changes: SimpleChanges) {
183-
// This is subscribed to by `RouterLinkActive` so that it knows to update when there are changes
184-
// to the RouterLinks it's tracking.
185-
this.onChanges.next();
186-
}
187-
188178
/**
189179
* Commands to pass to {@link Router#createUrlTree Router#createUrlTree}.
190180
* - **array**: commands to pass to {@link Router#createUrlTree Router#createUrlTree}.
@@ -308,9 +298,6 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy {
308298
// TODO(issue/24571): remove '!'.
309299
@HostBinding() href!: string;
310300

311-
/** @internal */
312-
onChanges = new Subject<void>();
313-
314301
constructor(
315302
private router: Router, private route: ActivatedRoute,
316303
private locationStrategy: LocationStrategy) {
@@ -349,9 +336,8 @@ export class RouterLinkWithHref implements OnChanges, OnDestroy {
349336
}
350337

351338
/** @nodoc */
352-
ngOnChanges(changes: SimpleChanges): any {
339+
ngOnChanges(changes: {}): any {
353340
this.updateTargetUrlAndHref();
354-
this.onChanges.next();
355341
}
356342
/** @nodoc */
357343
ngOnDestroy(): any {

packages/router/src/directives/router_link_active.ts

Lines changed: 9 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,7 @@
77
*/
88

99
import {AfterContentInit, ChangeDetectorRef, ContentChildren, Directive, ElementRef, Input, OnChanges, OnDestroy, Optional, QueryList, Renderer2, SimpleChanges} from '@angular/core';
10-
import {from, of, Subscription} from 'rxjs';
11-
import {mergeAll} from 'rxjs/operators';
10+
import {Subscription} from 'rxjs';
1211

1312
import {Event, NavigationEnd} from '../events';
1413
import {Router} from '../router';
@@ -80,13 +79,14 @@ import {RouterLink, RouterLinkWithHref} from './router_link';
8079
exportAs: 'routerLinkActive',
8180
})
8281
export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit {
82+
// TODO(issue/24571): remove '!'.
8383
@ContentChildren(RouterLink, {descendants: true}) links!: QueryList<RouterLink>;
84+
// TODO(issue/24571): remove '!'.
8485
@ContentChildren(RouterLinkWithHref, {descendants: true})
8586
linksWithHrefs!: QueryList<RouterLinkWithHref>;
8687

8788
private classes: string[] = [];
88-
private routerEventsSubscription: Subscription;
89-
private linkInputChangesSubscription?: Subscription;
89+
private subscription: Subscription;
9090
public readonly isActive: boolean = false;
9191

9292
@Input() routerLinkActiveOptions: {exact: boolean} = {exact: false};
@@ -95,7 +95,7 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit
9595
private router: Router, private element: ElementRef, private renderer: Renderer2,
9696
private readonly cdr: ChangeDetectorRef, @Optional() private link?: RouterLink,
9797
@Optional() private linkWithHref?: RouterLinkWithHref) {
98-
this.routerEventsSubscription = router.events.subscribe((s: Event) => {
98+
this.subscription = router.events.subscribe((s: Event) => {
9999
if (s instanceof NavigationEnd) {
100100
this.update();
101101
}
@@ -104,23 +104,9 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit
104104

105105
/** @nodoc */
106106
ngAfterContentInit(): void {
107-
// `of(null)` is used to force subscribe body to execute once immediately (like `startWith`).
108-
from([this.links.changes, this.linksWithHrefs.changes, of(null)])
109-
.pipe(mergeAll())
110-
.subscribe(_ => {
111-
this.update();
112-
this.subscribeToEachLinkOnChanges();
113-
});
114-
}
115-
116-
private subscribeToEachLinkOnChanges() {
117-
this.linkInputChangesSubscription?.unsubscribe();
118-
const allLinkChanges =
119-
[...this.links.toArray(), ...this.linksWithHrefs.toArray(), this.link, this.linkWithHref]
120-
.filter((link): link is RouterLink|RouterLinkWithHref => !!link)
121-
.map(link => link.onChanges);
122-
this.linkInputChangesSubscription =
123-
from(allLinkChanges).pipe(mergeAll()).subscribe(() => this.update());
107+
this.links.changes.subscribe(_ => this.update());
108+
this.linksWithHrefs.changes.subscribe(_ => this.update());
109+
this.update();
124110
}
125111

126112
@Input()
@@ -135,8 +121,7 @@ export class RouterLinkActive implements OnChanges, OnDestroy, AfterContentInit
135121
}
136122
/** @nodoc */
137123
ngOnDestroy(): void {
138-
this.routerEventsSubscription.unsubscribe();
139-
this.linkInputChangesSubscription?.unsubscribe();
124+
this.subscription.unsubscribe();
140125
}
141126

142127
private update(): void {

packages/router/test/regression_integration.spec.ts

Lines changed: 0 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -14,54 +14,6 @@ import {RouterTestingModule} from '@angular/router/testing';
1414

1515
describe('Integration', () => {
1616
describe('routerLinkActive', () => {
17-
it('should update when the associated routerLinks change - #18469', fakeAsync(() => {
18-
@Component({
19-
template: `
20-
<a id="first-link" [routerLink]="[firstLink]" routerLinkActive="active">{{firstLink}}</a>
21-
<div id="second-link" routerLinkActive="active">
22-
<a [routerLink]="[secondLink]">{{secondLink}}</a>
23-
</div>
24-
`,
25-
})
26-
class LinkComponent {
27-
firstLink = 'link-a';
28-
secondLink = 'link-b';
29-
30-
changeLinks(): void {
31-
const temp = this.secondLink;
32-
this.secondLink = this.firstLink;
33-
this.firstLink = temp;
34-
}
35-
}
36-
37-
@Component({template: 'simple'})
38-
class SimpleCmp {
39-
}
40-
41-
TestBed.configureTestingModule({
42-
imports: [RouterTestingModule.withRoutes(
43-
[{path: 'link-a', component: SimpleCmp}, {path: 'link-b', component: SimpleCmp}])],
44-
declarations: [LinkComponent, SimpleCmp]
45-
});
46-
47-
const router: Router = TestBed.inject(Router);
48-
const fixture = createRoot(router, LinkComponent);
49-
const firstLink = fixture.debugElement.query(p => p.nativeElement.id === 'first-link');
50-
const secondLink = fixture.debugElement.query(p => p.nativeElement.id === 'second-link');
51-
router.navigateByUrl('/link-a');
52-
advance(fixture);
53-
54-
expect(firstLink.nativeElement.classList).toContain('active');
55-
expect(secondLink.nativeElement.classList).not.toContain('active');
56-
57-
fixture.componentInstance.changeLinks();
58-
fixture.detectChanges();
59-
advance(fixture);
60-
61-
expect(firstLink.nativeElement.classList).not.toContain('active');
62-
expect(secondLink.nativeElement.classList).toContain('active');
63-
}));
64-
6517
it('should not cause infinite loops in the change detection - #15825', fakeAsync(() => {
6618
@Component({selector: 'simple', template: 'simple'})
6719
class SimpleCmp {

0 commit comments

Comments
 (0)