Skip to content
This repository was archived by the owner on Mar 27, 2023. It is now read-only.

Commit f51e999

Browse files
arturovthippee-lee
authored andcommitted
fix(dropdown): remove memory leaks inside the DropdownFocusHandler
This PR removes memory leaks from the `DropdownFocusHandler` by unsubscribing from observables and removing observables, created through the `wrapObservable` function, since the `onSubscribe` function captures `this`. Signed-off-by: Artur Androsovych <[email protected]>
1 parent 29d8c2e commit f51e999

File tree

1 file changed

+16
-6
lines changed

1 file changed

+16
-6
lines changed

packages/angular/projects/clr-angular/src/popover/dropdown/providers/dropdown-focus-handler.service.ts

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
/*
2-
* Copyright (c) 2016-2020 VMware, Inc. All Rights Reserved.
2+
* Copyright (c) 2016-2021 VMware, Inc. All Rights Reserved.
33
* This software is released under MIT license.
44
* The full license information can be found in LICENSE in the root directory of this project.
55
*/
66

77
import { isPlatformBrowser } from '@angular/common';
8-
import { Inject, Injectable, Optional, PLATFORM_ID, Renderer2, SkipSelf } from '@angular/core';
8+
import { Inject, Injectable, OnDestroy, Optional, PLATFORM_ID, Renderer2, SkipSelf } from '@angular/core';
99
import { Observable, of, ReplaySubject } from 'rxjs';
1010
import { map, take } from 'rxjs/operators';
1111
import { ClrPopoverToggleService } from '../../../utils/popover/providers/popover-toggle.service';
@@ -18,7 +18,7 @@ import { Linkers } from '../../../utils/focus/focusable-item/linkers';
1818
import { wrapObservable } from '../../../utils/focus/wrap-observable';
1919

2020
@Injectable()
21-
export class DropdownFocusHandler implements FocusableItem {
21+
export class DropdownFocusHandler implements OnDestroy, FocusableItem {
2222
constructor(
2323
@Inject(UNIQUE_ID) public id: string,
2424
private renderer: Renderer2,
@@ -42,7 +42,7 @@ export class DropdownFocusHandler implements FocusableItem {
4242
* If the dropdown was opened by clicking on the trigger, we automatically move to the first item
4343
*/
4444
moveToFirstItemWhenOpen() {
45-
this.toggleService.openChange.subscribe(open => {
45+
const subscription = this.toggleService.openChange.subscribe(open => {
4646
if (open && this.toggleService.originalEvent) {
4747
// Even if we properly waited for ngAfterViewInit, the container still wouldn't be attached to the DOM.
4848
// So setTimeout is the only way to wait for the container to be ready to move focus to first item.
@@ -56,6 +56,8 @@ export class DropdownFocusHandler implements FocusableItem {
5656
});
5757
}
5858
});
59+
60+
this._unlistenFuncs.push(() => subscription.unsubscribe());
5961
}
6062

6163
private focusBackOnTrigger = false;
@@ -64,7 +66,7 @@ export class DropdownFocusHandler implements FocusableItem {
6466
* Focus on the menu when it opens, and focus back on the root trigger when the whole dropdown becomes closed
6567
*/
6668
handleRootFocus() {
67-
this.toggleService.openChange.subscribe(open => {
69+
const subscription = this.toggleService.openChange.subscribe(open => {
6870
if (!open) {
6971
// We reset the state of the focus service both on initialization and when closing.
7072
this.focusService.reset(this);
@@ -75,6 +77,8 @@ export class DropdownFocusHandler implements FocusableItem {
7577
}
7678
this.focusBackOnTrigger = open;
7779
});
80+
81+
this._unlistenFuncs.push(() => subscription.unsubscribe());
7882
}
7983

8084
private _trigger: HTMLElement;
@@ -205,8 +209,14 @@ export class DropdownFocusHandler implements FocusableItem {
205209
}
206210

207211
ngOnDestroy() {
208-
this._unlistenFuncs.forEach((unlisten: Function) => unlisten());
212+
while (this._unlistenFuncs.length) {
213+
this._unlistenFuncs.pop()();
214+
}
209215
this.focusService.detachListeners();
216+
// Caretaker note: we're explicitly setting these observables to `undefined` since they're
217+
// created via `wrapObservable`. We provide the `onSubscribe` function, which captures `this`.
218+
// This leads to a circular reference and prevents the `DropdownFocusHandler` from being GC'd.
219+
this.right = this.down = this.up = undefined;
210220
}
211221
}
212222

0 commit comments

Comments
 (0)