#79 Unbind events on unmounting (#83)

Author: mderrick

src/video/constants.js

@@ -1,33 +1,33 @@
 export const EVENTS = [
-    'onAbort',
-    'onCanPlay',
-    'onCanPlayThrough',
-    'onDurationChange',
-    'onEmptied',
-    'onEncrypted',
-    'onEnded',
-    'onError',
-    'onLoadedData',
-    'onLoadedMetadata',
-    'onLoadStart',
-    'onPause',
-    'onPlay',
-    'onPlaying',
-    'onProgress',
-    'onRateChange',
-    'onSeeked',
-    'onSeeking',
-    'onStalled',
-    'onSuspend',
-    'onTimeUpdate',
-    'onVolumeChange',
-    'onWaiting'
+    'abort',
+    'canPlay',
+    'canPlayThrough',
+    'durationChange',
+    'emptied',
+    'encrypted',
+    'ended',
+    'error',
+    'loadedData',
+    'loadedMetadata',
+    'loadStart',
+    'pause',
+    'play',
+    'playing',
+    'progress',
+    'rateChange',
+    'seeked',
+    'seeking',
+    'stalled',
+    'suspend',
+    'timeUpdate',
+    'volumeChange',
+    'waiting'
 ];
 
 export const TRACKEVENTS = [
-    'onChange',
-    'onAddTrack',
-    'onRemoveTrack'
+    'change',
+    'addTrack',
+    'removeTrack'
 ];
 
 export const METHODS = [

src/video/video.js

@@ -54,11 +54,14 @@ export default (
 
         bindEventsToUpdateState() {
             EVENTS.forEach(event => {
-                this.videoEl[event.toLowerCase()] = this.updateState;
+                this.videoEl.addEventListener(event.toLowerCase(), this.updateState);
             });
 
             TRACKEVENTS.forEach(event => {
-                this.videoEl.textTracks[event.toLowerCase()] = this.updateState;
+                // TODO: JSDom does not have this method on
+                // `textTracks`. Investigate so we can test this without this check.
+                this.videoEl.textTracks.addEventListener
+                && this.videoEl.textTracks.addEventListener(event.toLowerCase(), this.updateState);
             });
 
             // If <source> elements are used instead of a src attribute then
@@ -73,6 +76,29 @@ export default (
             }
         }
 
+        unbindEvents() {
+            EVENTS.forEach(event => {
+                this.videoEl.removeEventListener(event.toLowerCase(), this.updateState);
+            });
+
+            TRACKEVENTS.forEach(event => {
+                // TODO: JSDom does not have this method on
+                // `textTracks`. Investigate so we can test this without this check.
+                this.videoEl.textTracks.removeEventListener
+                && this.videoEl.textTracks.removeEventListener(event.toLowerCase(), this.updateState);
+            });
+
+            const sources = this.videoEl.getElementsByTagName('source');
+            if (sources.length) {
+                const lastSource = sources[sources.length - 1];
+                lastSource.removeEventListener('error', this.updateState);
+            }
+        }
+
+        componentWillUnmount() {
+            this.unbindEvents();
+        }
+
         // Stop `this.el` from being null briefly on every render,
         // see: https://github.com/mderrick/react-html5video/pull/65
         setRef(el) {

src/video/video.test.js

@@ -1,6 +1,7 @@
 import React from 'react';
 import { mount, shallow } from 'enzyme';
 import video from './video';
+import { EVENTS } from './constants';
 
 const TestControl = ({ duration }) => {
     return (
@@ -85,6 +86,32 @@ describe('video', () => {
                 });
             });
         });
+
+        it('should remove all event listeners from the video element when unmounted', () => {
+            const removeEventListenerSpy = jest.fn();
+            component = mount(
+                <Component autoPlay />
+            );
+            const updateState = component.instance().updateState;
+            component.find('video').node.removeEventListener = removeEventListenerSpy;
+            expect(removeEventListenerSpy).not.toHaveBeenCalled();
+            component.unmount();
+            EVENTS.forEach((event) => {
+                expect(removeEventListenerSpy).toHaveBeenCalledWith(event.toLowerCase(), updateState);
+            });
+        });
+
+        it('should remove "error" event listener from the source element when unmounted', () => {
+            const removeEventListenerSpy = jest.fn();
+            component = mount(
+                <Component autoPlay />
+            );
+            const updateState = component.instance().updateState;
+            component.find('source').node.removeEventListener = removeEventListenerSpy;
+            expect(removeEventListenerSpy).not.toHaveBeenCalled();
+            component.unmount();
+            expect(removeEventListenerSpy).toHaveBeenCalledWith('error', updateState);
+        });
     });
 
     describe('mapping to props', () => {