Skip to content

Commit

Permalink
Better prop comparison for shouldComponentUpdate
Browse files Browse the repository at this point in the history
  • Loading branch information
seniorapple committed Jan 10, 2018
1 parent 8424ca6 commit f228a25
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 11 deletions.
10 changes: 2 additions & 8 deletions src/ReactPlayer.js
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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
Expand Down
31 changes: 29 additions & 2 deletions src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
62 changes: 61 additions & 1 deletion test/specs/utils.js
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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)
})
})

0 comments on commit f228a25

Please sign in to comment.