From f228a2553249c0e3f372d387a7bd6cca4247f602 Mon Sep 17 00:00:00 2001 From: seniorapple Date: Wed, 10 Jan 2018 14:11:16 +0000 Subject: [PATCH] Better prop comparison for shouldComponentUpdate Partly fixes https://github.com/CookPete/react-player/issues/302 --- src/ReactPlayer.js | 10 ++------ src/utils.js | 31 +++++++++++++++++++++-- test/specs/utils.js | 62 ++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 92 insertions(+), 11 deletions(-) diff --git a/src/ReactPlayer.js b/src/ReactPlayer.js index e2ae2be..380ccbe 100644 --- a/src/ReactPlayer.js +++ b/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 diff --git a/src/utils.js b/src/utils.js index 0352d39..781b869 100644 --- a/src/utils.js +++ b/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 } diff --git a/test/specs/utils.js b/test/specs/utils.js index f82505d..8005cbc 100644 --- a/test/specs/utils.js +++ 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) + }) +})