fix(unique-selection-dispatcher): remove listeners on destroy (#5164)

Author: Eddman

src/lib/button-toggle/button-toggle.ts

@@ -16,6 +16,7 @@ import {
   HostBinding,
   Input,
   OnInit,
+  OnDestroy,
   Optional,
   Output,
   QueryList,
@@ -270,7 +271,7 @@ export class MdButtonToggleGroupMultiple extends _MdButtonToggleGroupMixinBase
     'class': 'mat-button-toggle'
   }
 })
-export class MdButtonToggle implements OnInit {
+export class MdButtonToggle implements OnInit, OnDestroy {
   /** Whether or not this button toggle is checked. */
   private _checked: boolean = false;
 
@@ -286,6 +287,9 @@ export class MdButtonToggle implements OnInit {
   /** Whether or not the button toggle is a single selection. */
   private _isSingleSelector: boolean = false;
 
+  /** Unregister function for _buttonToggleDispatcher **/
+  private _removeUniqueSelectionListener: () => void = () => {};
+
   @ViewChild('input') _inputElement: ElementRef;
 
   /** The parent button toggle group (exclusive selection). Optional. */
@@ -371,11 +375,12 @@ export class MdButtonToggle implements OnInit {
     this.buttonToggleGroupMultiple = toggleGroupMultiple;
 
     if (this.buttonToggleGroup) {
-      _buttonToggleDispatcher.listen((id: string, name: string) => {
-        if (id != this.id && name == this.name) {
-          this.checked = false;
-        }
-      });
+      this._removeUniqueSelectionListener =
+        _buttonToggleDispatcher.listen((id: string, name: string) => {
+          if (id != this.id && name == this.name) {
+            this.checked = false;
+          }
+        });
 
       this._type = 'radio';
       this.name = this.buttonToggleGroup.name;
@@ -445,4 +450,9 @@ export class MdButtonToggle implements OnInit {
     event.value = this._value;
     this.change.emit(event);
   }
+
+  // Unregister buttonToggleDispatcherListener on destroy
+  ngOnDestroy(): void {
+    this._removeUniqueSelectionListener();
+  }
 }

src/lib/core/coordination/unique-selection-dispatcher.spec.ts

@@ -0,0 +1,32 @@
+import {UniqueSelectionDispatcher} from './unique-selection-dispatcher';
+
+
+describe('Unique selection dispatcher', () => {
+
+  describe('register', () => {
+    it('once unregistered the listener must not be called on notify', (done) => {
+      let dispatcher: UniqueSelectionDispatcher = new UniqueSelectionDispatcher();
+      let called = false;
+
+      // Register first listener
+      dispatcher.listen(() => {
+        called = true;
+      });
+
+      // Register a listener
+      let deregisterFn = dispatcher.listen(() => {
+        done.fail('Should not be called');
+      });
+
+      // Unregister
+      deregisterFn();
+
+      // Call registered listeners
+      dispatcher.notify('testId', 'testName');
+
+      expect(called).toBeTruthy('Registered listener must be called.');
+
+      done();
+    });
+  });
+});

src/lib/core/coordination/unique-selection-dispatcher.ts

@@ -36,9 +36,17 @@ export class UniqueSelectionDispatcher {
     }
   }
 
-  /** Listen for future changes to item selection. */
-  listen(listener: UniqueSelectionDispatcherListener) {
+  /**
+   * Listen for future changes to item selection.
+   * @return Function used to unregister listener
+   **/
+  listen(listener: UniqueSelectionDispatcherListener): () => void {
     this._listeners.push(listener);
+    return () => {
+      this._listeners = this._listeners.filter((registered: UniqueSelectionDispatcherListener) => {
+        return listener !== registered;
+      });
+    };
   }
 }
 

src/lib/expansion/accordion-item.ts

@@ -48,19 +48,24 @@ export class AccordionItem implements OnDestroy {
   }
   private _expanded: boolean;
 
+  /** Unregister function for _expansionDispatcher **/
+  private _removeUniqueSelectionListener: () => void = () => {};
+
   constructor(@Optional() public accordion: CdkAccordion,
               protected _expansionDispatcher: UniqueSelectionDispatcher) {
-     _expansionDispatcher.listen((id: string, accordionId: string) => {
-       if (this.accordion && !this.accordion.multi &&
-           this.accordion.id === accordionId && this.id !== id) {
-         this.expanded = false;
-       }
-     });
+     this._removeUniqueSelectionListener =
+       _expansionDispatcher.listen((id: string, accordionId: string) => {
+         if (this.accordion && !this.accordion.multi &&
+             this.accordion.id === accordionId && this.id !== id) {
+           this.expanded = false;
+         }
+       });
     }
 
   /** Emits an event for the accordion item being destroyed. */
   ngOnDestroy() {
     this.destroyed.emit();
+    this._removeUniqueSelectionListener();
   }
 
   /** Toggles the expanded state of the accordion item. */

src/lib/radio/radio.ts

@@ -457,6 +457,9 @@ export class MdRadioButton extends _MdRadioButtonMixinBase
   /** Reference to the current focus ripple. */
   private _focusRipple: RippleRef | null;
 
+  /** Unregister function for _radioDispatcher **/
+  private _removeUniqueSelectionListener: () => void = () => {};
+
   /** The native `<input type=radio>` element */
   @ViewChild('input') _inputElement: ElementRef;
 
@@ -472,11 +475,12 @@ export class MdRadioButton extends _MdRadioButtonMixinBase
     // TODO(jelbourn): Assert that there's no name binding AND a parent radio group.
     this.radioGroup = radioGroup;
 
-    _radioDispatcher.listen((id: string, name: string) => {
-      if (id != this.id && name == this.name) {
-        this.checked = false;
-      }
-    });
+    this._removeUniqueSelectionListener =
+      _radioDispatcher.listen((id: string, name: string) => {
+        if (id != this.id && name == this.name) {
+          this.checked = false;
+        }
+      });
   }
 
   /** Focuses the radio button. */
@@ -512,6 +516,7 @@ export class MdRadioButton extends _MdRadioButtonMixinBase
 
   ngOnDestroy() {
     this._focusOriginMonitor.stopMonitoring(this._inputElement.nativeElement);
+    this._removeUniqueSelectionListener();
   }
 
   /** Dispatch change event with current value. */