Skip to content

Commit

Permalink
feat: Object Type-Detection and Replacing object.assign (#3757)
Browse files Browse the repository at this point in the history
  • Loading branch information
misteroneill authored and gkatsev committed Dec 2, 2016
1 parent 54ff1f9 commit 8f16de2
Show file tree
Hide file tree
Showing 16 changed files with 121 additions and 29 deletions.
1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
"babel-runtime": "^6.9.2",
"global": "4.3.0",
"lodash-compat": "3.10.2",
"object.assign": "^4.0.4",
"safe-json-parse": "4.0.0",
"tsml": "1.0.1",
"videojs-font": "2.0.0",
Expand Down
2 changes: 1 addition & 1 deletion src/js/button.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import ClickableComponent from './clickable-component.js';
import Component from './component';
import log from './utils/log.js';
import assign from 'object.assign';
import {assign} from './utils/obj';

/**
* Base class for all buttons.
Expand Down
2 changes: 1 addition & 1 deletion src/js/clickable-component.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import * as Events from './utils/events.js';
import * as Fn from './utils/fn.js';
import log from './utils/log.js';
import document from 'global/document';
import assign from 'object.assign';
import {assign} from './utils/obj';

/**
* Clickable Component which is clickable or keyboard actionable,
Expand Down
3 changes: 2 additions & 1 deletion src/js/extend.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import log from './utils/log';
import {isObject} from './utils/obj';

/*
* @file extend.js
Expand Down Expand Up @@ -51,7 +52,7 @@ const extendFn = function(superClass, subClassMethods = {}) {

let methods = {};

if (typeof subClassMethods === 'object') {
if (isObject(subClassMethods)) {
if (typeof subClassMethods.init === 'function') {
log.warn('Constructor logic via init() is deprecated; please use constructor() instead.');
subClassMethods.constructor = subClassMethods.init;
Expand Down
4 changes: 2 additions & 2 deletions src/js/media-error.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
/**
* @file media-error.js
*/
import assign from 'object.assign';
import {assign, isObject} from './utils/obj';

/**
* A Custom `MediaError` class which mimics the standard HTML5 `MediaError` class.
Expand Down Expand Up @@ -33,7 +33,7 @@ function MediaError(value) {
} else if (typeof value === 'string') {
// default code is zero, so this is a custom error
this.message = value;
} else if (typeof value === 'object') {
} else if (isObject(value)) {

// We assign the `code` property manually because native `MediaError` objects
// do not expose it as an own/enumerable property of the object.
Expand Down
2 changes: 1 addition & 1 deletion src/js/menu/menu-item.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
import ClickableComponent from '../clickable-component.js';
import Component from '../component.js';
import assign from 'object.assign';
import {assign} from '../utils/obj';

/**
* The component for a menu item. `<li>`
Expand Down
2 changes: 1 addition & 1 deletion src/js/player.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ import * as stylesheet from './utils/stylesheet.js';
import FullscreenApi from './fullscreen-api.js';
import MediaError from './media-error.js';
import safeParseTuple from 'safe-json-parse/tuple';
import assign from 'object.assign';
import {assign} from './utils/obj';
import mergeOptions from './utils/merge-options.js';
import textTrackConverter from './tracks/text-track-list-converter.js';
import ModalDialog from './modal-dialog';
Expand Down
2 changes: 1 addition & 1 deletion src/js/slider/slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
*/
import Component from '../component.js';
import * as Dom from '../utils/dom.js';
import assign from 'object.assign';
import {assign} from '../utils/obj';

/**
* The base functionality for a slider. Can be vertical or horizontal.
Expand Down
2 changes: 1 addition & 1 deletion src/js/tech/flash.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { createTimeRange } from '../utils/time-ranges.js';
import FlashRtmpDecorator from './flash-rtmp';
import Component from '../component';
import window from 'global/window';
import assign from 'object.assign';
import {assign} from '../utils/obj';

const navigator = window.navigator;

Expand Down
2 changes: 1 addition & 1 deletion src/js/tech/html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import tsml from 'tsml';
import * as browser from '../utils/browser.js';
import document from 'global/document';
import window from 'global/window';
import assign from 'object.assign';
import {assign} from '../utils/obj';
import mergeOptions from '../utils/merge-options.js';
import toTitleCase from '../utils/to-title-case.js';

Expand Down
5 changes: 3 additions & 2 deletions src/js/utils/dom.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import window from 'global/window';
import * as Guid from './guid.js';
import log from './log.js';
import tsml from 'tsml';
import {isObject} from './obj';

/**
* Detect if a value is a string with any non-whitespace characters.
Expand Down Expand Up @@ -65,7 +66,7 @@ function classRegExp(className) {
* - False otherwise
*/
export function isEl(value) {
return !!value && typeof value === 'object' && value.nodeType === 1;
return isObject(value) && value.nodeType === 1;
}

/**
Expand Down Expand Up @@ -674,7 +675,7 @@ export function getPointerPosition(el, event) {
* - False otherwise
*/
export function isTextNode(value) {
return !!value && typeof value === 'object' && value.nodeType === 3;
return isObject(value) && value.nodeType === 3;
}

/**
Expand Down
3 changes: 2 additions & 1 deletion src/js/utils/log.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/
import window from 'global/window';
import {IE_VERSION} from './browser';
import {isObject} from './obj';

let log;

Expand Down Expand Up @@ -51,7 +52,7 @@ export const logByType = (type, args, stringify = !!IE_VERSION && IE_VERSION < 1
// objects and arrays for those less-capable browsers.
if (stringify) {
args = args.map(a => {
if (a && typeof a === 'object' || Array.isArray(a)) {
if (isObject(a) || Array.isArray(a)) {
try {
return JSON.stringify(a);
} catch (x) {
Expand Down
15 changes: 1 addition & 14 deletions src/js/utils/merge-options.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,20 +3,7 @@
* @module merge-options
*/
import merge from 'lodash-compat/object/merge';

/**
* Check if an object direct descentant of Object and
* not of Array or RegExp.
*
* @param {Mixed} obj
* The object to check
*/
function isPlain(obj) {
return !!obj &&
typeof obj === 'object' &&
obj.toString() === '[object Object]' &&
obj.constructor === Object;
}
import {isPlain} from './obj';

/**
* Merge customizer. video.js simply overwrites non-simple objects
Expand Down
53 changes: 53 additions & 0 deletions src/js/utils/obj.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
* @return {Mixed}
* The new accumulated value.
*/
const toString = Object.prototype.toString;

/**
* Array-like iteration for objects.
Expand Down Expand Up @@ -63,3 +64,55 @@ export function reduce(object, fn, initial = 0) {
return Object.keys(object).reduce(
(accum, key) => fn(accum, object[key], key), initial);
}

/**
* Object.assign-style object shallow merge/extend.
*
* @param {Object} target
* @param {Object} ...sources
* @return {Object}
*/
export function assign(target, ...sources) {
if (Object.assign) {
return Object.assign(target, ...sources);
}

sources.forEach(source => {
if (!source) {
return;
}

each(source, (value, key) => {
target[key] = value;
});
});

return target;
}

/**
* Returns whether a value is an object of any kind - including DOM nodes,
* arrays, regular expressions, etc. Not functions, though.
*
* This avoids the gotcha where using `typeof` on a `null` value
* results in `'object'`.
*
* @param {Object} value
* @return {Boolean}
*/
export function isObject(value) {
return !!value && typeof value === 'object';
}

/**
* Returns whether an object appears to be a "plain" object - that is, a
* direct instance of `Object`.
*
* @param {Object} value
* @return {Boolean}
*/
export function isPlain(value) {
return isObject(value) &&
toString.call(value) === '[object Object]' &&
value.constructor === Object;
}
3 changes: 2 additions & 1 deletion src/js/video.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import log from './utils/log.js';
import * as Dom from './utils/dom.js';
import * as browser from './utils/browser.js';
import * as Url from './utils/url.js';
import {isObject} from './utils/obj';
import computedStyle from './utils/computed-style.js';
import extendFn from './extend.js';
import merge from 'lodash-compat/object/merge';
Expand Down Expand Up @@ -117,7 +118,7 @@ function videojs(id, options, ready) {
videojs.hooks('beforesetup').forEach(function(hookFunction) {
const opts = hookFunction(tag, mergeOptions(options));

if (!opts || typeof opts !== 'object' || Array.isArray(opts)) {
if (!isObject(opts) || Array.isArray(opts)) {
videojs.log.error('please return an object in beforesetup hooks');
return;
}
Expand Down
49 changes: 49 additions & 0 deletions test/unit/utils/obj.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,23 @@ import * as Obj from '../../../src/js/utils/obj';

QUnit.module('utils/obj');

class Foo {
constructor() {}
toString() {
return 'I am a Foo!';
}
}

const passFail = (assert, fn, descriptor, passes, failures) => {
Object.keys(passes).forEach(key => {
assert.ok(fn(passes[key]), `${key} IS ${descriptor}`);
});

Object.keys(failures).forEach(key => {
assert.notOk(fn(failures[key]), `${key} IS NOT ${descriptor}`);
});
};

QUnit.test('each', function(assert) {
const spy = sinon.spy();

Expand Down Expand Up @@ -56,3 +73,35 @@ QUnit.test('reduce', function(assert) {
assert.strictEqual(third.c, -3);
assert.strictEqual(third.d, -4);
});

QUnit.test('isObject', function(assert) {
passFail(assert, Obj.isObject, 'an object', {
'plain object': {},
'constructed object': new Foo(),
'array': [],
'regex': new RegExp('.'),
'date': new Date()
}, {
null: null,
function() {},
boolean: true,
number: 1,
string: 'xyz'
});
});

QUnit.test('isPlain', function(assert) {
passFail(assert, Obj.isPlain, 'a plain object', {
'plain object': {}
}, {
'constructed object': new Foo(),
'null': null,
'array': [],
'function'() {},
'regex': new RegExp('.'),
'date': new Date(),
'boolean': true,
'number': 1,
'string': 'xyz'
});
});

0 comments on commit 8f16de2

Please sign in to comment.