fix(match-media): unregister media query event listeners on destroy (#1236)

Author: CaerusKaru

src/lib/core/match-media/match-media.ts

@@ -5,7 +5,7 @@
  * Use of this source code is governed by an MIT-style license that can be
  * found in the LICENSE file at https://angular.io/license
  */
-import {Inject, Injectable, NgZone, PLATFORM_ID} from '@angular/core';
+import {Inject, Injectable, NgZone, OnDestroy, PLATFORM_ID} from '@angular/core';
 import {DOCUMENT, isPlatformBrowser} from '@angular/common';
 import {BehaviorSubject, Observable, merge, Observer} from 'rxjs';
 import {filter} from 'rxjs/operators';
@@ -20,10 +20,11 @@ import {MediaChange} from '../media-change';
  * NOTE: both mediaQuery activations and de-activations are announced in notifications
  */
 @Injectable({providedIn: 'root'})
-export class MatchMedia {
+export class MatchMedia implements OnDestroy {
   /** Initialize source with 'all' so all non-responsive APIs trigger style updates */
   readonly source = new BehaviorSubject<MediaChange>(new MediaChange(true));
   registry = new Map<string, MediaQueryList>();
+  private readonly pendingRemoveListenerFns: Array<() => void> = [];
 
   constructor(protected _zone: NgZone,
               @Inject(PLATFORM_ID) protected _platformId: Object,
@@ -73,9 +74,8 @@ export class MatchMedia {
   observe(mqList?: string[], filterOthers = false): Observable<MediaChange> {
     if (mqList && mqList.length) {
       const matchMedia$: Observable<MediaChange> = this._observable$.pipe(
-          filter((change: MediaChange) => {
-            return !filterOthers ? true : (mqList.indexOf(change.mediaQuery) > -1);
-          })
+          filter((change: MediaChange) =>
+            !filterOthers ? true : (mqList.indexOf(change.mediaQuery) > -1))
       );
       const registration$: Observable<MediaChange> = new Observable((observer: Observer<MediaChange>) => {  // tslint:disable-line:max-line-length
         const matches: Array<MediaChange> = this.registerQuery(mqList);
@@ -113,6 +113,7 @@ export class MatchMedia {
       if (!mql) {
         mql = this.buildMQL(query);
         mql.addListener(onMQLEvent);
+        this.pendingRemoveListenerFns.push(() => mql!.removeListener(onMQLEvent));
         this.registry.set(query, mql);
       }
 
@@ -124,6 +125,13 @@ export class MatchMedia {
     return matches;
   }
 
+  ngOnDestroy(): void {
+    let fn;
+    while (fn = this.pendingRemoveListenerFns.pop()) {
+      fn();
+    }
+  }
+
   /**
    * Call window.matchMedia() to build a MediaQueryList; which
    * supports 0..n listeners for activation/deactivation
@@ -188,6 +196,14 @@ function constructMql(query: string, isBrowser: boolean): MediaQueryList {
     addListener: () => {
     },
     removeListener: () => {
+    },
+    onchange: null,
+    addEventListener() {
+    },
+    removeEventListener() {
+    },
+    dispatchEvent() {
+      return false;
     }
-  } as unknown as MediaQueryList;
+  } as MediaQueryList;
 }

src/lib/core/media-trigger/media-trigger.spec.ts

@@ -23,54 +23,52 @@ describe('media-trigger', () => {
     tick(100);  // Since MediaObserver has 50ms debounceTime
   };
 
-  describe('', () => {
-    beforeEach(() => {
-      // Configure testbed to prepare services
-      TestBed.configureTestingModule({
-        providers: [
-          MockMatchMediaProvider,
-          MediaTrigger
-        ]
-      });
+  beforeEach(() => {
+    // Configure testbed to prepare services
+    TestBed.configureTestingModule({
+      providers: [
+        MockMatchMediaProvider,
+        MediaTrigger
+      ]
     });
+  });
 
-    beforeEach(inject([MediaObserver, MediaTrigger, MatchMedia],
-        (_mediaObserver: MediaObserver, _mediaTrigger: MediaTrigger, _matchMedia: MockMatchMedia) => { // tslint:disable-line:max-line-length
-          mediaObserver = _mediaObserver;
-          mediaTrigger = _mediaTrigger;
-          matchMedia = _matchMedia;
+  beforeEach(inject([MediaObserver, MediaTrigger, MatchMedia],
+      (_mediaObserver: MediaObserver, _mediaTrigger: MediaTrigger, _matchMedia: MockMatchMedia) => { // tslint:disable-line:max-line-length
+        mediaObserver = _mediaObserver;
+        mediaTrigger = _mediaTrigger;
+        matchMedia = _matchMedia;
 
-          _matchMedia.useOverlaps = true;
-        }));
+        _matchMedia.useOverlaps = true;
+      }));
 
-    it('can trigger activations with list of breakpoint aliases', fakeAsync(() => {
-      let activations: MediaChange[] = [];
-      let subscription = mediaObserver.asObservable().subscribe(
-          (changes: MediaChange[]) => {
-            activations = [...changes];
-          });
+  it('can trigger activations with list of breakpoint aliases', fakeAsync(() => {
+    let activations: MediaChange[] = [];
+    let subscription = mediaObserver.asObservable().subscribe(
+        (changes: MediaChange[]) => {
+          activations = [...changes];
+        });
 
-      // assign default activation(s) with overlaps allowed
-      matchMedia.activate('xl');
-      const originalActivations = matchMedia.activations.length;
+    // assign default activation(s) with overlaps allowed
+    matchMedia.activate('xl');
+    const originalActivations = matchMedia.activations.length;
 
-      // Activate mediaQuery associated with 'md' alias
-      activateQuery(['sm']);
-      expect(activations.length).toEqual(1);
-      expect(activations[0].mqAlias).toEqual('sm');
+    // Activate mediaQuery associated with 'md' alias
+    activateQuery(['sm']);
+    expect(activations.length).toEqual(1);
+    expect(activations[0].mqAlias).toEqual('sm');
 
-      // Activations are sorted by descending priority
-      activateQuery(['lt-lg', 'md']);
-      expect(activations.length).toEqual(2);
-      expect(activations[0].mqAlias).toEqual('md');
-      expect(activations[1].mqAlias).toEqual('lt-lg');
+    // Activations are sorted by descending priority
+    activateQuery(['lt-lg', 'md']);
+    expect(activations.length).toEqual(2);
+    expect(activations[0].mqAlias).toEqual('md');
+    expect(activations[1].mqAlias).toEqual('lt-lg');
 
-      // Clean manual activation overrides
-      mediaTrigger.restore();
-      tick(100);
-      expect(activations.length).toEqual(originalActivations);
+    // Clean manual activation overrides
+    mediaTrigger.restore();
+    tick(100);
+    expect(activations.length).toEqual(originalActivations);
 
-      subscription.unsubscribe();
-    }));
-  });
+    subscription.unsubscribe();
+  }));
 });

src/lib/core/media-trigger/media-trigger.ts

@@ -55,11 +55,9 @@ export class MediaTrigger {
       const extractQuery = (change: MediaChange) => change.mediaQuery;
       const list = this.originalActivations.map(extractQuery);
       try {
-
         this.deactivateAll();
         this.restoreRegistryMatches();
         this.setActivations(list);
-
       } finally {
         this.originalActivations = [];
         if (this.resizeSubscription) {
@@ -151,7 +149,7 @@ export class MediaTrigger {
   private forceRegistryMatches(queries: string[], matches: boolean) {
     const registry = new Map<string, MediaQueryList>();
     queries.forEach(query => {
-      registry.set(query, {matches: matches} as MediaQueryList);
+      registry.set(query, {matches} as MediaQueryList);
     });
 
     this.matchMedia.registry = registry;