From f641c0f6d30eebaf9f7a004b1fb4563c6c59c143 Mon Sep 17 00:00:00 2001 From: Pat O'Neill Date: Fri, 13 May 2022 12:59:49 -0400 Subject: [PATCH] remove merge-options in favor of obj.merge --- src/js/component.js | 10 ++-- src/js/live-tracker.js | 4 +- src/js/player.js | 18 +++---- src/js/resize-manager.js | 4 +- src/js/tech/html5.js | 5 +- src/js/tech/loader.js | 4 +- src/js/tech/setup-sourceset.js | 6 +-- src/js/tech/tech.js | 5 +- src/js/tracks/audio-track.js | 2 +- src/js/tracks/text-track.js | 2 +- src/js/tracks/video-track.js | 2 +- src/js/utils/merge-options.js | 7 --- src/js/video.js | 1 - test/api/api.js | 2 - test/unit/autoplay.test.js | 2 +- test/unit/play.test.js | 2 +- test/unit/utils/merge-options.test.js | 36 ------------- test/unit/utils/obj.test.js | 78 +++++++++++++++++++-------- 18 files changed, 86 insertions(+), 104 deletions(-) delete mode 100644 src/js/utils/merge-options.js delete mode 100644 test/unit/utils/merge-options.test.js diff --git a/src/js/component.js b/src/js/component.js index 9557f2ca8..e0fa7ed52 100644 --- a/src/js/component.js +++ b/src/js/component.js @@ -10,7 +10,7 @@ import * as Dom from './utils/dom.js'; import * as Fn from './utils/fn.js'; import * as Guid from './utils/guid.js'; import {toTitleCase, toLowerCase} from './utils/str.js'; -import mergeOptions from './utils/merge-options.js'; +import {merge} from './utils/obj.js'; import computedStyle from './utils/computed-style'; import Map from './utils/map.js'; import Set from './utils/set.js'; @@ -69,10 +69,10 @@ class Component { this.parentComponent_ = null; // Make a copy of prototype.options_ to protect against overriding defaults - this.options_ = mergeOptions({}, this.options_); + this.options_ = merge({}, this.options_); // Updated options with supplied options - options = this.options_ = mergeOptions(this.options_, options); + options = this.options_ = merge(this.options_, options); // Get ID from options or options element if one is supplied this.id_ = options.id || (options.el && options.el.id); @@ -215,7 +215,7 @@ class Component { /** * Deep merge of options objects with new options. * > Note: When both `obj` and `options` contain properties whose values are objects. - * The two properties get merged using {@link module:mergeOptions} + * The two properties get merged using {@link module:obj.merge} * * @param {Object} obj * The object that contains new options. @@ -228,7 +228,7 @@ class Component { return this.options_; } - this.options_ = mergeOptions(this.options_, obj); + this.options_ = merge(this.options_, obj); return this.options_; } diff --git a/src/js/live-tracker.js b/src/js/live-tracker.js index ee1ed52f4..f3551cb64 100644 --- a/src/js/live-tracker.js +++ b/src/js/live-tracker.js @@ -1,5 +1,5 @@ import Component from './component.js'; -import mergeOptions from './utils/merge-options.js'; +import {merge} from './utils/obj.js'; import document from 'global/document'; import * as browser from './utils/browser.js'; import window from 'global/window'; @@ -40,7 +40,7 @@ class LiveTracker extends Component { */ constructor(player, options) { // LiveTracker does not need an element - const options_ = mergeOptions(defaults, options, {createEl: false}); + const options_ = merge(defaults, options, {createEl: false}); super(player, options_); diff --git a/src/js/player.js b/src/js/player.js index 963a98d5d..b2074d86a 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -23,8 +23,6 @@ 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 './utils/obj'; -import mergeOptions from './utils/merge-options.js'; import {silencePromise, isPromise} from './utils/promise'; import textTrackConverter from './tracks/text-track-list-converter.js'; import ModalDialog from './modal-dialog'; @@ -34,7 +32,7 @@ import {ALL as TRACK_TYPES} from './tracks/track-types'; import filterSource from './utils/filter-source'; import {getMimetype, findMimetype} from './utils/mimetypes'; import {hooks} from './utils/hooks'; -import {isObject} from './utils/obj'; +import {assign, merge, isObject} from './utils/obj'; import keycode from 'keycode'; // The following imports are used only to ensure that the corresponding modules @@ -504,7 +502,7 @@ class Player extends Component { // as well so they don't need to reach back into the player for options later. // We also need to do another copy of this.options_ so we don't end up with // an infinite loop. - const playerOptionsCopy = mergeOptions(this.options_); + const playerOptionsCopy = merge(this.options_); // Load plugins if (options.plugins) { @@ -1535,7 +1533,7 @@ class Player extends Component { } // update `currentSource` cache always - this.cache_.source = mergeOptions({}, srcObj, {src, type}); + this.cache_.source = merge({}, srcObj, {src, type}); const matchingSources = this.cache_.sources.filter((s) => s.src && s.src === src); const sourceElSources = []; @@ -4648,7 +4646,7 @@ class Player extends Component { * An array of of supported languages */ languages() { - return mergeOptions(Player.prototype.options_.languages, this.languages_); + return merge(Player.prototype.options_.languages, this.languages_); } /** @@ -4659,7 +4657,7 @@ class Player extends Component { * Object representing the current of track info */ toJSON() { - const options = mergeOptions(this.options_); + const options = merge(this.options_); const tracks = options.tracks; options.tracks = []; @@ -4668,7 +4666,7 @@ class Player extends Component { let track = tracks[i]; // deep merge tracks and null out player so no circular references - track = mergeOptions(track); + track = merge(track); track.player = undefined; options.tracks[i] = track; } @@ -4942,7 +4940,7 @@ class Player extends Component { this.reset(); // Clone the media object so it cannot be mutated from outside. - this.cache_.media = mergeOptions(media); + this.cache_.media = merge(media); const {artwork, poster, src, textTracks} = this.cache_.media; @@ -5001,7 +4999,7 @@ class Player extends Component { return media; } - return mergeOptions(this.cache_.media); + return merge(this.cache_.media); } /** diff --git a/src/js/resize-manager.js b/src/js/resize-manager.js index 0086e4a6d..41bbef627 100644 --- a/src/js/resize-manager.js +++ b/src/js/resize-manager.js @@ -4,7 +4,7 @@ import window from 'global/window'; import { debounce } from './utils/fn.js'; import * as Events from './utils/events.js'; -import mergeOptions from './utils/merge-options.js'; +import {merge} from './utils/obj.js'; import Component from './component.js'; /** @@ -47,7 +47,7 @@ class ResizeManager extends Component { } // Only create an element when ResizeObserver isn't available - const options_ = mergeOptions({ + const options_ = merge({ createEl: !RESIZE_OBSERVER_AVAILABLE, reportTouchActivity: false }, options); diff --git a/src/js/tech/html5.js b/src/js/tech/html5.js index 6ca4acc3f..fd7f060af 100644 --- a/src/js/tech/html5.js +++ b/src/js/tech/html5.js @@ -8,8 +8,7 @@ import log from '../utils/log.js'; import * as browser from '../utils/browser.js'; import document from 'global/document'; import window from 'global/window'; -import {assign} from '../utils/obj'; -import mergeOptions from '../utils/merge-options.js'; +import {assign, merge} from '../utils/obj'; import {toTitleCase} from '../utils/str.js'; import {NORMAL as TRACK_TYPES, REMOTE} from '../tracks/track-types'; import setupSourceset from './setup-sourceset'; @@ -385,7 +384,7 @@ class Html5 extends Tech { // determine if native controls should be used const tagAttributes = this.options_.tag && Dom.getAttributes(this.options_.tag); - const attributes = mergeOptions({}, tagAttributes); + const attributes = merge({}, tagAttributes); if (!browser.TOUCH_ENABLED || this.options_.nativeControlsForTouch !== true) { delete attributes.controls; diff --git a/src/js/tech/loader.js b/src/js/tech/loader.js index bf6f64ade..b7beb64d1 100644 --- a/src/js/tech/loader.js +++ b/src/js/tech/loader.js @@ -4,7 +4,7 @@ import Component from '../component.js'; import Tech from './tech.js'; import {toTitleCase} from '../utils/str.js'; -import mergeOptions from '../utils/merge-options.js'; +import {merge} from '../utils/obj.js'; /** * The `MediaLoader` is the `Component` that decides which playback technology to load @@ -28,7 +28,7 @@ class MediaLoader extends Component { */ constructor(player, options, ready) { // MediaLoader has no element - const options_ = mergeOptions({createEl: false}, options); + const options_ = merge({createEl: false}, options); super(player, options_, ready); diff --git a/src/js/tech/setup-sourceset.js b/src/js/tech/setup-sourceset.js index 889b7f27c..752759046 100644 --- a/src/js/tech/setup-sourceset.js +++ b/src/js/tech/setup-sourceset.js @@ -1,6 +1,6 @@ import window from 'global/window'; import document from 'global/document'; -import mergeOptions from '../utils/merge-options'; +import {merge} from '../utils/obj'; import {getAbsoluteURL} from '../utils/url'; /** @@ -183,7 +183,7 @@ const firstSourceWatch = function(tech) { el[k] = appendWrapper(old[k]); }); - Object.defineProperty(el, 'innerHTML', mergeOptions(innerDescriptor, { + Object.defineProperty(el, 'innerHTML', merge(innerDescriptor, { set: appendWrapper(innerDescriptor.set) })); @@ -252,7 +252,7 @@ const setupSourceset = function(tech) { const oldSetAttribute = el.setAttribute; const oldLoad = el.load; - Object.defineProperty(el, 'src', mergeOptions(srcDescriptor, { + Object.defineProperty(el, 'src', merge(srcDescriptor, { set: (v) => { const retval = srcDescriptor.set.call(el, v); diff --git a/src/js/tech/tech.js b/src/js/tech/tech.js index 00dd0624f..c0e546bcc 100644 --- a/src/js/tech/tech.js +++ b/src/js/tech/tech.js @@ -3,7 +3,6 @@ */ import Component from '../component'; -import mergeOptions from '../utils/merge-options.js'; import * as Fn from '../utils/fn.js'; import log from '../utils/log.js'; import { createTimeRange } from '../utils/time-ranges.js'; @@ -11,7 +10,7 @@ import { bufferedPercent } from '../utils/buffer.js'; import MediaError from '../media-error.js'; import window from 'global/window'; import document from 'global/document'; -import {isPlain} from '../utils/obj'; +import {isPlain, merge} from '../utils/obj'; import * as TRACK_TYPES from '../tracks/track-types'; import {toTitleCase, toLowerCase} from '../utils/str.js'; import vtt from 'videojs-vtt.js'; @@ -743,7 +742,7 @@ class Tech extends Component { * The track element that gets created. */ createRemoteTextTrack(options) { - const track = mergeOptions(options, { + const track = merge(options, { tech: this }); diff --git a/src/js/tracks/audio-track.js b/src/js/tracks/audio-track.js index 6091835cc..3eb9c7d5c 100644 --- a/src/js/tracks/audio-track.js +++ b/src/js/tracks/audio-track.js @@ -1,6 +1,6 @@ import {AudioTrackKind} from './track-enums'; import Track from './track'; -import merge from '../utils/merge-options'; +import {merge} from '../utils/obj'; /** * A representation of a single `AudioTrack`. If it is part of an {@link AudioTrackList} diff --git a/src/js/tracks/text-track.js b/src/js/tracks/text-track.js index 7cb02c899..702c80394 100644 --- a/src/js/tracks/text-track.js +++ b/src/js/tracks/text-track.js @@ -9,7 +9,7 @@ import window from 'global/window'; import Track from './track.js'; import { isCrossOrigin } from '../utils/url.js'; import XHR from '@videojs/xhr'; -import merge from '../utils/merge-options'; +import {merge} from '../utils/obj'; /** * Takes a webvtt file contents and parses it into cues diff --git a/src/js/tracks/video-track.js b/src/js/tracks/video-track.js index 88c4e5be4..0e156f3c3 100644 --- a/src/js/tracks/video-track.js +++ b/src/js/tracks/video-track.js @@ -1,6 +1,6 @@ import {VideoTrackKind} from './track-enums'; import Track from './track'; -import merge from '../utils/merge-options'; +import {merge} from '../utils/obj'; /** * A representation of a single `VideoTrack`. diff --git a/src/js/utils/merge-options.js b/src/js/utils/merge-options.js deleted file mode 100644 index 01d5b5048..000000000 --- a/src/js/utils/merge-options.js +++ /dev/null @@ -1,7 +0,0 @@ -/** - * @file merge-options.js - * @module merge-options - */ -import {merge} from './obj'; - -export default merge; diff --git a/src/js/video.js b/src/js/video.js index 90c3af910..06a2cd446 100644 --- a/src/js/video.js +++ b/src/js/video.js @@ -109,7 +109,6 @@ const normalizeId = (id) => id.indexOf('#') === 0 ? id.slice(1) : id; * @borrows module:format-time.formatTime as formatTime * @borrows module:format-time.resetFormatTime as resetFormatTime * @borrows module:format-time.setFormatTime as setFormatTime - * @borrows module:merge-options.mergeOptions as mergeOptions * @borrows module:middleware.use as use * @borrows Player.players as players * @borrows Plugin.registerPlugin as registerPlugin diff --git a/test/api/api.js b/test/api/api.js index 167cd9dea..59ab6ba90 100644 --- a/test/api/api.js +++ b/test/api/api.js @@ -208,8 +208,6 @@ QUnit.test('should export useful components to the public', function(assert) { assert.ok(videojs.getComponent('ChaptersButton'), 'ChaptersButton should be public'); assert.ok(videojs.getComponent('ChaptersTrackMenuItem'), 'ChaptersTrackMenuItem should be public'); - - assert.ok(videojs.mergeOptions, 'mergeOptions should be public'); }); QUnit.test('should be able to initialize player twice on the same tag using string reference', function(assert) { diff --git a/test/unit/autoplay.test.js b/test/unit/autoplay.test.js index 4f9a569b1..6cdede1b6 100644 --- a/test/unit/autoplay.test.js +++ b/test/unit/autoplay.test.js @@ -63,7 +63,7 @@ QUnit.module('autoplay', { videoTag.setAttribute(a, attributes[a]); }); - this.player = videojs(videoTag.id, videojs.mergeOptions({techOrder: ['techFaker']}, options)); + this.player = videojs(videoTag.id, videojs.obj.merge({techOrder: ['techFaker']}, options)); const oldMuted = this.player.muted; this.player.play = () => { diff --git a/test/unit/play.test.js b/test/unit/play.test.js index afede86ca..cfce26d2f 100644 --- a/test/unit/play.test.js +++ b/test/unit/play.test.js @@ -51,7 +51,7 @@ const mainModule = function(playReturnValue, middlewareTermination, subhooks) { }; this.checkState = (assertName, options = {}) => { - const expectedState = videojs.mergeOptions({ + const expectedState = videojs.obj.merge({ playCalls: 0, techLoaded: false, techReady: false, diff --git a/test/unit/utils/merge-options.test.js b/test/unit/utils/merge-options.test.js deleted file mode 100644 index aebffb593..000000000 --- a/test/unit/utils/merge-options.test.js +++ /dev/null @@ -1,36 +0,0 @@ -/* eslint-env qunit */ -import mergeOptions from '../../../src/js/utils/merge-options.js'; - -QUnit.module('merge-options'); - -QUnit.test('should merge options objects', function(assert) { - const ob1 = { - a: true, - b: { b1: true, b2: true, b3: true }, - c: true - }; - - const ob2 = { - // override value - a: false, - // merge sub-option values - b: { b1: true, b2: false, b4: true }, - // add new option - d: true - }; - - const ob3 = mergeOptions(ob1, ob2); - - assert.deepEqual(ob3, { - a: false, - b: { b1: true, b2: false, b3: true, b4: true }, - c: true, - d: true - }, 'options objects merged correctly'); -}); - -QUnit.test('should ignore non-objects', function(assert) { - const obj = { a: 1 }; - - assert.deepEqual(mergeOptions(obj, true), obj, 'ignored non-object input'); -}); diff --git a/test/unit/utils/obj.test.js b/test/unit/utils/obj.test.js index 69a5917b3..0138b1b0f 100644 --- a/test/unit/utils/obj.test.js +++ b/test/unit/utils/obj.test.js @@ -1,6 +1,6 @@ /* eslint-env qunit */ import sinon from 'sinon'; -import * as Obj from '../../../src/js/utils/obj'; +import * as obj from '../../../src/js/utils/obj'; QUnit.module('utils/obj'); @@ -24,7 +24,7 @@ const passFail = (assert, fn, descriptor, passes, failures) => { QUnit.test('each', function(assert) { const spy = sinon.spy(); - Obj.each({ + obj.each({ a: 1, b: 'foo', c: null @@ -35,12 +35,12 @@ QUnit.test('each', function(assert) { assert.ok(spy.calledWith('foo', 'b')); assert.ok(spy.calledWith(null, 'c')); - Obj.each({}, spy); + obj.each({}, spy); assert.strictEqual(spy.callCount, 3, 'an empty object was not iterated over'); }); QUnit.test('reduce', function(assert) { - const first = Obj.reduce({ + const first = obj.reduce({ a: 1, b: 2, c: 3, @@ -49,7 +49,7 @@ QUnit.test('reduce', function(assert) { assert.strictEqual(first, 10); - const second = Obj.reduce({ + const second = obj.reduce({ a: 1, b: 2, c: 3, @@ -58,7 +58,7 @@ QUnit.test('reduce', function(assert) { assert.strictEqual(second, 20); - const third = Obj.reduce({ + const third = obj.reduce({ a: 1, b: 2, c: 3, @@ -75,7 +75,7 @@ QUnit.test('reduce', function(assert) { }); QUnit.test('isObject', function(assert) { - passFail(assert, Obj.isObject, 'an object', { + passFail(assert, obj.isObject, 'an object', { 'plain object': {}, 'constructed object': new Foo(), 'array': [], @@ -91,7 +91,7 @@ QUnit.test('isObject', function(assert) { }); QUnit.test('isPlain', function(assert) { - passFail(assert, Obj.isPlain, 'a plain object', { + passFail(assert, obj.isPlain, 'a plain object', { 'plain object': {} }, { 'constructed object': new Foo(), @@ -134,21 +134,21 @@ QUnit.module('utils/obj.assign', function() { QUnit.test('override object', function(assert) { const foo = {foo: 'yellow'}; - assert.deepEqual(Obj.assign(foo, {foo: 'blue'}), {foo: 'blue'}, 'Obj.assign should return overriden result'); + assert.deepEqual(obj.assign(foo, {foo: 'blue'}), {foo: 'blue'}, 'obj.assign should return overriden result'); assert.deepEqual(foo, {foo: 'blue'}, 'foo should be modified directly'); }); QUnit.test('new object', function(assert) { const foo = {foo: 'yellow'}; - assert.deepEqual(Obj.assign({}, foo, {foo: 'blue'}), {foo: 'blue'}, 'Obj.assign should return result'); + assert.deepEqual(obj.assign({}, foo, {foo: 'blue'}), {foo: 'blue'}, 'obj.assign should return result'); assert.deepEqual(foo, {foo: 'yellow'}, 'foo should not be modified'); }); QUnit.test('empty override', function(assert) { const foo = {foo: 'yellow'}; - assert.deepEqual(Obj.assign(foo, {}), {foo: 'yellow'}, 'Obj.assign should return result'); + assert.deepEqual(obj.assign(foo, {}), {foo: 'yellow'}, 'obj.assign should return result'); assert.deepEqual(foo, {foo: 'yellow'}, 'foo should not be modified'); }); @@ -157,7 +157,7 @@ QUnit.module('utils/obj.assign', function() { const bar = {foo: 'bar'}; const baz = {foo: 'baz'}; - assert.deepEqual(Obj.assign(foo, bar, baz), baz, 'Obj.assign should return result'); + assert.deepEqual(obj.assign(foo, bar, baz), baz, 'obj.assign should return result'); assert.deepEqual(foo, baz, 'foo should be overridden'); }); @@ -165,7 +165,7 @@ QUnit.module('utils/obj.assign', function() { const foo = {}; const expected = {one: 1, two: 2, three: 3}; - assert.deepEqual(Obj.assign(foo, {one: 1}, {two: 2}, {three: 3}), expected, 'Obj.assign should return result'); + assert.deepEqual(obj.assign(foo, {one: 1}, {two: 2}, {three: 3}), expected, 'obj.assign should return result'); assert.deepEqual(foo, expected, 'foo should be equal to result'); }); @@ -199,23 +199,55 @@ QUnit.module('utils/obj.assign', function() { red: 'nope' }; - assert.deepEqual(Obj.assign(foo, baz), expected, 'Obj.assign should return result'); + assert.deepEqual(obj.assign(foo, baz), expected, 'obj.assign should return result'); assert.deepEqual(foo, expected, 'foo is overridden'); }); QUnit.test('negative tests', function(assert) { const expected = {foo: 11}; - assert.deepEqual(Obj.assign({}, expected, undefined), expected, 'assign should undefined'); - assert.deepEqual(Obj.assign({}, expected, null), expected, 'assign should ignore null'); - assert.deepEqual(Obj.assign({}, expected, []), expected, 'assign should ignore Array'); - assert.deepEqual(Obj.assign({}, expected, ''), expected, 'assign should ignore string'); - assert.deepEqual(Obj.assign({}, expected, 11), expected, 'assign should ignore number'); - assert.deepEqual(Obj.assign({}, expected, new RegExp()), expected, 'assign should ignore RegExp'); - assert.deepEqual(Obj.assign({}, expected, new Date()), expected, 'assign should ignore Date'); - assert.deepEqual(Obj.assign({}, expected, true), expected, 'assign should ignore boolean'); - assert.deepEqual(Obj.assign({}, expected, () => {}), expected, 'assign should ignore function'); + assert.deepEqual(obj.assign({}, expected, undefined), expected, 'assign should undefined'); + assert.deepEqual(obj.assign({}, expected, null), expected, 'assign should ignore null'); + assert.deepEqual(obj.assign({}, expected, []), expected, 'assign should ignore Array'); + assert.deepEqual(obj.assign({}, expected, ''), expected, 'assign should ignore string'); + assert.deepEqual(obj.assign({}, expected, 11), expected, 'assign should ignore number'); + assert.deepEqual(obj.assign({}, expected, new RegExp()), expected, 'assign should ignore RegExp'); + assert.deepEqual(obj.assign({}, expected, new Date()), expected, 'assign should ignore Date'); + assert.deepEqual(obj.assign({}, expected, true), expected, 'assign should ignore boolean'); + assert.deepEqual(obj.assign({}, expected, () => {}), expected, 'assign should ignore function'); }); }); }); +QUnit.test('utils/obj.merge', function(assert) { + const ob1 = { + a: true, + b: {b1: true, b2: true, b3: true}, + c: true + }; + + const ob2 = { + // override value + a: false, + // merge sub-option values + b: { b1: true, b2: false, b4: true }, + // add new option + d: true + }; + + const ob3 = obj.merge(ob1, ob2); + + assert.deepEqual(ob3, { + a: false, + b: {b1: true, b2: false, b3: true, b4: true}, + c: true, + d: true + }, 'options objects merged correctly'); +}); + +QUnit.test('utils/obj.merge: should ignore non-objects', function(assert) { + const target = {a: 1}; + + assert.deepEqual(obj.merge(target, true), target, 'ignored non-object input'); +}); +