Better prop comparison for shouldComponentUpdate

Partly fixes cookpete/react-player#302

src/ReactPlayer.js

@@ -1,7 +1,7 @@
 import React, { Component } from 'react'
 
 import { propTypes, defaultProps, DEPRECATED_CONFIG_PROPS } from './props'
-import { getConfig, omit, isObject } from './utils'
+import { getConfig, omit, isEqual } from './utils'
 import players from './players'
 import Player from './Player'
 import FilePlayer from './players/FilePlayer'
@@ -29,13 +29,7 @@ export default class ReactPlayer extends Component {
     clearTimeout(this.progressTimeout)
   }
   shouldComponentUpdate (nextProps) {
-    for (let key of Object.keys(this.props)) {
-      const prop = this.props[key]
-      if (!isObject(prop) && prop !== nextProps[key]) {
-        return true
-      }
-    }
-    return false
+    return !isEqual(this.props, nextProps)
   }
   getDuration = () => {
     if (!this.player) return null

src/utils.js

@@ -109,6 +109,33 @@ export function callPlayer (method, ...args) {
 }
 
 export function isObject (val) {
-  if (val === null) return false
-  return typeof val === 'function' || typeof val === 'object'
+  return val !== null && typeof val === 'object'
+}
+
+// Deep comparison of two objects but ignoring
+// functions, for use in shouldComponentUpdate
+export function isEqual (a, b) {
+  if (typeof a === 'function' && typeof b === 'function') {
+    return true
+  }
+  if (a instanceof Array && b instanceof Array) {
+    if (a.length !== b.length) {
+      return false
+    }
+    for (let i = 0; i !== a.length; i++) {
+      if (!isEqual(a[i], b[i])) {
+        return false
+      }
+    }
+    return true
+  }
+  if (isObject(a) && isObject(b)) {
+    for (let key of Object.keys(a)) {
+      if (!isEqual(a[key], b[key])) {
+        return false
+      }
+    }
+    return true
+  }
+  return a === b
 }

test/specs/utils.js

@@ -1,4 +1,4 @@
-import { parseStartTime, randomString, omit, getConfig, callPlayer } from '../../src/utils'
+import { parseStartTime, randomString, omit, getConfig, callPlayer, isEqual } from '../../src/utils'
 
 const { describe, it, expect } = window
 
@@ -176,3 +176,63 @@ describe('callPlayer', () => {
     expect(callPlayer.call(fakePlayer, 'testMethod')).to.equal(null)
   })
 })
+
+describe.only('isEqual', () => {
+  it('returns true', () => {
+    const a = {
+      b: { c: 3, d: 4 },
+      c: [1, 2, 3],
+      d: [{ a: 1 }, { b: 2 }]
+    }
+    const b = {
+      b: { c: 3, d: 4 },
+      c: [1, 2, 3],
+      d: [{ a: 1 }, { b: 2 }]
+    }
+    expect(isEqual(a, b)).to.equal(true)
+  })
+
+  it('returns false when deep property differs', () => {
+    const a = {
+      b: { c: 3, d: 4 },
+      c: [1, 2, 3],
+      d: [{ a: 1 }, { b: 2 }]
+    }
+    const b = {
+      b: { c: 3, d: 4 },
+      c: [1, 2, 3],
+      d: [{ a: 1 }, { b: 3 }]
+    }
+    expect(isEqual(a, b)).to.equal(false)
+  })
+
+  it('returns false when array size differs', () => {
+    const a = {
+      b: { c: 3, d: 4 },
+      c: [1, 2, 3],
+      d: [{ a: 1 }, { b: 2 }]
+    }
+    const b = {
+      b: { c: 3, d: 4 },
+      c: [1, 2],
+      d: [{ a: 1 }, { b: 3 }]
+    }
+    expect(isEqual(a, b)).to.equal(false)
+  })
+
+  it('ignores functions', () => {
+    const a = {
+      b: { c: 3, d: 4 },
+      c: [1, 2, 3],
+      d: [{ a: 1 }, { b: 2 }],
+      e: () => {}
+    }
+    const b = {
+      b: { c: 3, d: 4 },
+      c: [1, 2, 3],
+      d: [{ a: 1 }, { b: 2 }],
+      e: () => {}
+    }
+    expect(isEqual(a, b)).to.equal(true)
+  })
+})