Skip to content

Commit 6dd3aba

Browse files
committed
refactor(multiple): use takeUntilDestroyed() for automatic cleanup
Replaces manual subscription tracking with `takeUntilDestroyed()` to ensure subscriptions are cleaned up automatically and simplify the code.
1 parent 8045b12 commit 6dd3aba

File tree

24 files changed

+197
-243
lines changed

24 files changed

+197
-243
lines changed

src/material/autocomplete/autocomplete.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
ChangeDetectorRef,
1313
Component,
1414
ContentChildren,
15+
DestroyRef,
1516
ElementRef,
1617
EventEmitter,
1718
InjectionToken,
@@ -35,7 +36,7 @@ import {
3536
} from '../core';
3637
import {_IdGenerator, ActiveDescendantKeyManager} from '@angular/cdk/a11y';
3738
import {Platform} from '@angular/cdk/platform';
38-
import {Subscription} from 'rxjs';
39+
import {takeUntilDestroyed} from '@angular/core/rxjs-interop';
3940

4041
/** Event object that is emitted when an autocomplete option is selected. */
4142
export class MatAutocompleteSelectedEvent {
@@ -116,7 +117,7 @@ export class MatAutocomplete implements AfterContentInit, OnDestroy {
116117
private _elementRef = inject<ElementRef<HTMLElement>>(ElementRef);
117118
protected _defaults = inject<MatAutocompleteDefaultOptions>(MAT_AUTOCOMPLETE_DEFAULT_OPTIONS);
118119
protected _animationsDisabled = _animationsDisabled();
119-
private _activeOptionChanges = Subscription.EMPTY;
120+
private readonly _destroyRef = inject(DestroyRef);
120121

121122
/** Manages active item in option list based on key events. */
122123
_keyManager: ActiveDescendantKeyManager<MatOption>;
@@ -266,7 +267,7 @@ export class MatAutocomplete implements AfterContentInit, OnDestroy {
266267
this._keyManager = new ActiveDescendantKeyManager<MatOption>(this.options)
267268
.withWrap()
268269
.skipPredicate(this._skipPredicate);
269-
this._activeOptionChanges = this._keyManager.change.subscribe(index => {
270+
this._keyManager.change.pipe(takeUntilDestroyed(this._destroyRef)).subscribe(index => {
270271
if (this.isOpen) {
271272
this.optionActivated.emit({source: this, option: this.options.toArray()[index] || null});
272273
}
@@ -278,7 +279,6 @@ export class MatAutocomplete implements AfterContentInit, OnDestroy {
278279

279280
ngOnDestroy() {
280281
this._keyManager?.destroy();
281-
this._activeOptionChanges.unsubscribe();
282282
}
283283

284284
/**

src/material/bottom-sheet/bottom-sheet-container.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,9 @@ import {
1616
ViewEncapsulation,
1717
inject,
1818
} from '@angular/core';
19-
import {Subscription} from 'rxjs';
2019
import {CdkPortalOutlet} from '@angular/cdk/portal';
2120
import {_animationsDisabled} from '../core';
21+
import {takeUntilDestroyed} from '@angular/core/rxjs-interop';
2222

2323
const ENTER_ANIMATION = '_mat-bottom-sheet-enter';
2424
const EXIT_ANIMATION = '_mat-bottom-sheet-exit';
@@ -53,7 +53,6 @@ const EXIT_ANIMATION = '_mat-bottom-sheet-exit';
5353
imports: [CdkPortalOutlet],
5454
})
5555
export class MatBottomSheetContainer extends CdkDialogContainer implements OnDestroy {
56-
private _breakpointSubscription: Subscription;
5756
protected _animationsDisabled = _animationsDisabled();
5857

5958
/** The state of the bottom sheet animations. */
@@ -75,8 +74,9 @@ export class MatBottomSheetContainer extends CdkDialogContainer implements OnDes
7574

7675
const breakpointObserver = inject(BreakpointObserver);
7776

78-
this._breakpointSubscription = breakpointObserver
77+
breakpointObserver
7978
.observe([Breakpoints.Medium, Breakpoints.Large, Breakpoints.XLarge])
79+
.pipe(takeUntilDestroyed())
8080
.subscribe(() => {
8181
const classList = (this._elementRef.nativeElement as HTMLElement).classList;
8282

@@ -121,7 +121,6 @@ export class MatBottomSheetContainer extends CdkDialogContainer implements OnDes
121121

122122
override ngOnDestroy() {
123123
super.ngOnDestroy();
124-
this._breakpointSubscription.unsubscribe();
125124
this._destroyed = true;
126125
}
127126

src/material/chips/chip-grid.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,11 @@ import {
3434
import {_ErrorStateTracker, ErrorStateMatcher} from '../core';
3535
import {MatFormFieldControl} from '../form-field';
3636
import {merge, Observable, Subject} from 'rxjs';
37-
import {takeUntil} from 'rxjs/operators';
3837
import {MatChipEvent} from './chip';
3938
import {MatChipRow} from './chip-row';
4039
import {MatChipSet} from './chip-set';
4140
import {MatChipTextControl} from './chip-text-control';
41+
import {takeUntilDestroyed} from '@angular/core/rxjs-interop';
4242

4343
/** Change event object that is emitted when the chip grid value has changed. */
4444
export class MatChipGridChange {
@@ -277,13 +277,13 @@ export class MatChipGrid
277277
}
278278

279279
ngAfterContentInit() {
280-
this.chipBlurChanges.pipe(takeUntil(this._destroyed)).subscribe(() => {
280+
this.chipBlurChanges.pipe(takeUntilDestroyed(this._destroyRef)).subscribe(() => {
281281
this._blur();
282282
this.stateChanges.next();
283283
});
284284

285285
merge(this.chipFocusChanges, this._chips.changes)
286-
.pipe(takeUntil(this._destroyed))
286+
.pipe(takeUntilDestroyed(this._destroyRef))
287287
.subscribe(() => this.stateChanges.next());
288288
}
289289

src/material/chips/chip-listbox.ts

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -16,20 +16,20 @@ import {
1616
forwardRef,
1717
inject,
1818
Input,
19-
OnDestroy,
2019
Output,
2120
QueryList,
2221
ViewEncapsulation,
2322
} from '@angular/core';
2423
import {ControlValueAccessor, NG_VALUE_ACCESSOR} from '@angular/forms';
2524
import {Observable} from 'rxjs';
26-
import {startWith, takeUntil} from 'rxjs/operators';
25+
import {startWith} from 'rxjs/operators';
2726
import {TAB} from '@angular/cdk/keycodes';
2827
import {MatChip, MatChipEvent} from './chip';
2928
import {MatChipOption, MatChipSelectionChange} from './chip-option';
3029
import {MatChipSet} from './chip-set';
3130
import {MatChipAction} from './chip-action';
3231
import {MAT_CHIPS_DEFAULT_OPTIONS} from './tokens';
32+
import {takeUntilDestroyed} from '@angular/core/rxjs-interop';
3333

3434
/** Change event object that is emitted when the chip listbox value has changed. */
3535
export class MatChipListboxChange {
@@ -82,10 +82,7 @@ export const MAT_CHIP_LISTBOX_CONTROL_VALUE_ACCESSOR: any = {
8282
encapsulation: ViewEncapsulation.None,
8383
changeDetection: ChangeDetectionStrategy.OnPush,
8484
})
85-
export class MatChipListbox
86-
extends MatChipSet
87-
implements AfterContentInit, OnDestroy, ControlValueAccessor
88-
{
85+
export class MatChipListbox extends MatChipSet implements AfterContentInit, ControlValueAccessor {
8986
/**
9087
* Function when touched. Set as part of ControlValueAccessor implementation.
9188
* @docs-private
@@ -199,18 +196,20 @@ export class MatChipListbox
199196
override _chips: QueryList<MatChipOption> = undefined!;
200197

201198
ngAfterContentInit() {
202-
this._chips.changes.pipe(startWith(null), takeUntil(this._destroyed)).subscribe(() => {
203-
if (this.value !== undefined) {
204-
Promise.resolve().then(() => {
205-
this._setSelectionByValue(this.value, false);
206-
});
207-
}
208-
// Update listbox selectable/multiple properties on chips
209-
this._syncListboxProperties();
210-
});
199+
this._chips.changes
200+
.pipe(startWith(null), takeUntilDestroyed(this._destroyRef))
201+
.subscribe(() => {
202+
if (this.value !== undefined) {
203+
Promise.resolve().then(() => {
204+
this._setSelectionByValue(this.value, false);
205+
});
206+
}
207+
// Update listbox selectable/multiple properties on chips
208+
this._syncListboxProperties();
209+
});
211210

212-
this.chipBlurChanges.pipe(takeUntil(this._destroyed)).subscribe(() => this._blur());
213-
this.chipSelectionChanges.pipe(takeUntil(this._destroyed)).subscribe(event => {
211+
this.chipBlurChanges.pipe(takeUntilDestroyed(this._destroyRef)).subscribe(() => this._blur());
212+
this.chipSelectionChanges.pipe(takeUntilDestroyed(this._destroyRef)).subscribe(event => {
214213
if (!this.multiple) {
215214
this._chips.forEach(chip => {
216215
if (chip !== event.source) {

src/material/chips/chip-set.ts

Lines changed: 42 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,13 @@ import {
2222
booleanAttribute,
2323
numberAttribute,
2424
inject,
25+
DestroyRef,
2526
} from '@angular/core';
26-
import {Observable, Subject, merge} from 'rxjs';
27-
import {startWith, switchMap, takeUntil} from 'rxjs/operators';
27+
import {Observable, merge} from 'rxjs';
28+
import {startWith, switchMap} from 'rxjs/operators';
2829
import {MatChip, MatChipEvent} from './chip';
2930
import {MatChipAction, MatChipContent} from './chip-action';
31+
import {takeUntilDestroyed} from '@angular/core/rxjs-interop';
3032

3133
/**
3234
* Basic container component for the MatChip component.
@@ -53,16 +55,14 @@ export class MatChipSet implements AfterViewInit, OnDestroy {
5355
protected _elementRef = inject<ElementRef<HTMLElement>>(ElementRef);
5456
protected _changeDetectorRef = inject(ChangeDetectorRef);
5557
private _dir = inject(Directionality, {optional: true});
58+
protected readonly _destroyRef = inject(DestroyRef);
5659

5760
/** Index of the last destroyed chip that had focus. */
5861
private _lastDestroyedFocusedChipIndex: number | null = null;
5962

6063
/** Used to manage focus within the chip list. */
6164
protected _keyManager: FocusKeyManager<MatChipAction>;
6265

63-
/** Subject that emits when the component has been destroyed. */
64-
protected _destroyed = new Subject<void>();
65-
6666
/** Role to use if it hasn't been overwritten by the user. */
6767
protected _defaultRole = 'presentation';
6868

@@ -146,8 +146,6 @@ export class MatChipSet implements AfterViewInit, OnDestroy {
146146
ngOnDestroy() {
147147
this._keyManager?.destroy();
148148
this._chipActions.destroy();
149-
this._destroyed.next();
150-
this._destroyed.complete();
151149
}
152150

153151
/** Checks whether any of the chips is focused. */
@@ -249,7 +247,7 @@ export class MatChipSet implements AfterViewInit, OnDestroy {
249247

250248
// Keep the manager active index in sync so that navigation picks
251249
// up from the current chip if the user clicks into the list directly.
252-
this.chipFocusChanges.pipe(takeUntil(this._destroyed)).subscribe(({chip}) => {
250+
this.chipFocusChanges.pipe(takeUntilDestroyed(this._destroyRef)).subscribe(({chip}) => {
253251
const action = chip._getSourceAction(document.activeElement as Element);
254252

255253
if (action) {
@@ -258,7 +256,7 @@ export class MatChipSet implements AfterViewInit, OnDestroy {
258256
});
259257

260258
this._dir?.change
261-
.pipe(takeUntil(this._destroyed))
259+
.pipe(takeUntilDestroyed(this._destroyRef))
262260
.subscribe(direction => this._keyManager.withHorizontalOrientation(direction));
263261
}
264262

@@ -273,42 +271,46 @@ export class MatChipSet implements AfterViewInit, OnDestroy {
273271

274272
/** Listens to changes in the chip set and syncs up the state of the individual chips. */
275273
private _trackChipSetChanges() {
276-
this._chips.changes.pipe(startWith(null), takeUntil(this._destroyed)).subscribe(() => {
277-
if (this.disabled) {
278-
// Since this happens after the content has been
279-
// checked, we need to defer it to the next tick.
280-
Promise.resolve().then(() => this._syncChipsState());
281-
}
274+
this._chips.changes
275+
.pipe(startWith(null), takeUntilDestroyed(this._destroyRef))
276+
.subscribe(() => {
277+
if (this.disabled) {
278+
// Since this happens after the content has been
279+
// checked, we need to defer it to the next tick.
280+
Promise.resolve().then(() => this._syncChipsState());
281+
}
282282

283-
this._redirectDestroyedChipFocus();
284-
});
283+
this._redirectDestroyedChipFocus();
284+
});
285285
}
286286

287287
/** Starts tracking the destroyed chips in order to capture the focused one. */
288288
private _trackDestroyedFocusedChip() {
289-
this.chipDestroyedChanges.pipe(takeUntil(this._destroyed)).subscribe((event: MatChipEvent) => {
290-
// If the focused chip is destroyed, save its index so that we can move focus to the next
291-
// chip. We only save the index here, rather than move the focus immediately, because we want
292-
// to wait until the chip is removed from the chip list before focusing the next one. This
293-
// allows us to keep focus on the same index if the chip gets swapped out.
294-
const chipArray = this._chips.toArray();
295-
const chipIndex = chipArray.indexOf(event.chip);
296-
const hasFocus = event.chip._hasFocus();
297-
const wasLastFocused =
298-
event.chip._hadFocusOnRemove &&
299-
this._keyManager.activeItem &&
300-
event.chip._getActions().includes(this._keyManager.activeItem);
301-
302-
// Note that depending on the timing, the chip might've already lost focus by the
303-
// time we check this. We need the `wasLastFocused` as a fallback to detect such cases.
304-
// In `wasLastFocused` we also need to ensure that the chip actually had focus when it was
305-
// deleted so that we don't steal away the user's focus after they've moved on from the chip.
306-
const shouldMoveFocus = hasFocus || wasLastFocused;
307-
308-
if (this._isValidIndex(chipIndex) && shouldMoveFocus) {
309-
this._lastDestroyedFocusedChipIndex = chipIndex;
310-
}
311-
});
289+
this.chipDestroyedChanges
290+
.pipe(takeUntilDestroyed(this._destroyRef))
291+
.subscribe((event: MatChipEvent) => {
292+
// If the focused chip is destroyed, save its index so that we can move focus to the next
293+
// chip. We only save the index here, rather than move the focus immediately, because we want
294+
// to wait until the chip is removed from the chip list before focusing the next one. This
295+
// allows us to keep focus on the same index if the chip gets swapped out.
296+
const chipArray = this._chips.toArray();
297+
const chipIndex = chipArray.indexOf(event.chip);
298+
const hasFocus = event.chip._hasFocus();
299+
const wasLastFocused =
300+
event.chip._hadFocusOnRemove &&
301+
this._keyManager.activeItem &&
302+
event.chip._getActions().includes(this._keyManager.activeItem);
303+
304+
// Note that depending on the timing, the chip might've already lost focus by the
305+
// time we check this. We need the `wasLastFocused` as a fallback to detect such cases.
306+
// In `wasLastFocused` we also need to ensure that the chip actually had focus when it was
307+
// deleted so that we don't steal away the user's focus after they've moved on from the chip.
308+
const shouldMoveFocus = hasFocus || wasLastFocused;
309+
310+
if (this._isValidIndex(chipIndex) && shouldMoveFocus) {
311+
this._lastDestroyedFocusedChipIndex = chipIndex;
312+
}
313+
});
312314
}
313315

314316
/**

src/material/datepicker/calendar.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import {
2626
inject,
2727
} from '@angular/core';
2828
import {DateAdapter, MAT_DATE_FORMATS, MatDateFormats} from '../core';
29-
import {Subject, Subscription} from 'rxjs';
29+
import {Subject} from 'rxjs';
3030
import {MatCalendarUserEvent, MatCalendarCellClassFunction} from './calendar-body';
3131
import {createMissingDateImplError} from './datepicker-errors';
3232
import {MatDatepickerIntl} from './datepicker-intl';
@@ -44,6 +44,7 @@ import {_IdGenerator, CdkMonitorFocus} from '@angular/cdk/a11y';
4444
import {_CdkPrivateStyleLoader, _VisuallyHiddenLoader} from '@angular/cdk/private';
4545
import {_getFocusedElementPierceShadowDom} from '@angular/cdk/platform';
4646
import {MatTooltip} from '../tooltip';
47+
import {takeUntilDestroyed} from '@angular/core/rxjs-interop';
4748

4849
/**
4950
* Possible views for the calendar.
@@ -270,8 +271,6 @@ export class MatCalendar<D> implements AfterContentInit, AfterViewChecked, OnDes
270271
/** A portal containing the header component type for this calendar. */
271272
_calendarHeaderPortal: Portal<any>;
272273

273-
private _intlChanges: Subscription;
274-
275274
/**
276275
* Used for scheduling that focus should be moved to the active cell on the next tick.
277276
* We need to schedule it, rather than do it immediately, because we have to wait
@@ -433,10 +432,12 @@ export class MatCalendar<D> implements AfterContentInit, AfterViewChecked, OnDes
433432
}
434433
}
435434

436-
this._intlChanges = inject(MatDatepickerIntl).changes.subscribe(() => {
437-
this._changeDetectorRef.markForCheck();
438-
this.stateChanges.next();
439-
});
435+
inject(MatDatepickerIntl)
436+
.changes.pipe(takeUntilDestroyed())
437+
.subscribe(() => {
438+
this._changeDetectorRef.markForCheck();
439+
this.stateChanges.next();
440+
});
440441
}
441442

442443
ngAfterContentInit() {
@@ -455,7 +456,6 @@ export class MatCalendar<D> implements AfterContentInit, AfterViewChecked, OnDes
455456
}
456457

457458
ngOnDestroy() {
458-
this._intlChanges.unsubscribe();
459459
this.stateChanges.complete();
460460
}
461461

0 commit comments

Comments
 (0)