Skip to content

Commit 26b0635

Browse files
crisbetotinayuangao
authored andcommitted
fix(collections): clean up UniqueSelectionDispatcher listeners on destroy (#9673)
* Clears out all of the registered listeners in the `UniqueSelectionDispatcher` when the module is unloaded to avoid potential memory leaks due to references inside the listeners. * Cleans up the unit test setup for the dispatcher to align it with the rest of the project.
1 parent efb03c9 commit 26b0635

File tree

2 files changed

+30
-21
lines changed

2 files changed

+30
-21
lines changed

src/cdk/collections/unique-selection-dispatcher.spec.ts

Lines changed: 24 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -2,31 +2,36 @@ import {UniqueSelectionDispatcher} from './unique-selection-dispatcher';
22

33

44
describe('Unique selection dispatcher', () => {
5+
let dispatcher: UniqueSelectionDispatcher;
56

6-
describe('register', () => {
7-
it('once unregistered the listener must not be called on notify', (done) => {
8-
let dispatcher: UniqueSelectionDispatcher = new UniqueSelectionDispatcher();
9-
let called = false;
7+
beforeEach(() => dispatcher = new UniqueSelectionDispatcher());
108

11-
// Register first listener
12-
dispatcher.listen(() => {
13-
called = true;
14-
});
9+
it('should notify registered listeners', () => {
10+
const spy = jasmine.createSpy('listen handler');
1511

16-
// Register a listener
17-
let deregisterFn = dispatcher.listen(() => {
18-
done.fail('Should not be called');
19-
});
12+
dispatcher.listen(spy);
13+
dispatcher.notify('id', 'name');
2014

21-
// Unregister
22-
deregisterFn();
15+
expect(spy).toHaveBeenCalledWith('id', 'name');
16+
});
17+
18+
it('should not notify unregistered listeners', () => {
19+
const spy = jasmine.createSpy('listen handler');
20+
const unregister = dispatcher.listen(spy);
21+
22+
unregister();
23+
dispatcher.notify('id', 'name');
24+
25+
expect(spy).not.toHaveBeenCalled();
26+
});
2327

24-
// Call registered listeners
25-
dispatcher.notify('testId', 'testName');
28+
it('should remove all listeners when destroyed', () => {
29+
const spy = jasmine.createSpy('listen handler');
30+
dispatcher.listen(spy);
2631

27-
expect(called).toBeTruthy('Registered listener must be called.');
32+
dispatcher.ngOnDestroy();
33+
dispatcher.notify('id', 'name');
2834

29-
done();
30-
});
35+
expect(spy).not.toHaveBeenCalled();
3136
});
3237
});

src/cdk/collections/unique-selection-dispatcher.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {Injectable, Optional, SkipSelf} from '@angular/core';
9+
import {Injectable, Optional, SkipSelf, OnDestroy} from '@angular/core';
1010

1111

1212
// Users of the Dispatcher never need to see this type, but TypeScript requires it to be exported.
@@ -22,7 +22,7 @@ export type UniqueSelectionDispatcherListener = (id: string, name: string) => vo
2222
* less error-prone if they are simply passed through when the events occur.
2323
*/
2424
@Injectable()
25-
export class UniqueSelectionDispatcher {
25+
export class UniqueSelectionDispatcher implements OnDestroy {
2626
private _listeners: UniqueSelectionDispatcherListener[] = [];
2727

2828
/**
@@ -48,6 +48,10 @@ export class UniqueSelectionDispatcher {
4848
});
4949
};
5050
}
51+
52+
ngOnDestroy() {
53+
this._listeners = [];
54+
}
5155
}
5256

5357
/** @docs-private */

0 commit comments

Comments
 (0)