diff --git a/src/js/utils/merge-options.js b/src/js/utils/merge-options.js index 6a3a114413..6ca3b22dd9 100644 --- a/src/js/utils/merge-options.js +++ b/src/js/utils/merge-options.js @@ -11,38 +11,50 @@ function isPlain(obj) { } /** - * Merge two options objects, recursively merging **only* * plain object - * properties. Previously `deepMerge`. + * Merge customizer. video.js simply overwrites non-simple objects + * (like arrays) instead of attempting to overlay them. + * @see https://lodash.com/docs#merge + */ +const customizer = function(destination, source) { + // If we're not working with a plain object, copy the value as is + // If source is an array, for instance, it will replace destination + if (!isPlain(source)) { + return source; + } + + // If the new value is a plain object but the first object value is not + // we need to create a new object for the first object to merge with. + // This makes it consistent with how merge() works by default + // and also protects from later changes the to first object affecting + // the second object's values. + if (!isPlain(destination)) { + return mergeOptions({}, source); + } +}; + +/** + * Merge one or more options objects, recursively merging **only** + * plain object properties. Previously `deepMerge`. * - * @param {Object} object The destination object - * @param {...Object} source One or more objects to merge into the first - * @returns {Object} The updated first object + * @param {...Object} source One or more objects to merge + * @returns {Object} a new object that is the union of all + * provided objects * @function mergeOptions */ -export default function mergeOptions(object={}) { - - // Allow for infinite additional object args to merge - Array.prototype.slice.call(arguments, 1).forEach(function(source){ - - // Recursively merge only plain objects - // All other values will be directly copied - merge(object, source, function(a, b) { - - // If we're not working with a plain object, copy the value as is - if (!isPlain(b)) { - return b; - } - - // If the new value is a plain object but the first object value is not - // we need to create a new object for the first object to merge with. - // This makes it consistent with how merge() works by default - // and also protects from later changes the to first object affecting - // the second object's values. - if (!isPlain(a)) { - return mergeOptions({}, b); - } - }); - }); - - return object; +export default function mergeOptions() { + // contruct the call dynamically to handle the variable number of + // objects to merge + let args = Array.prototype.slice.call(arguments); + + // unshift an empty object into the front of the call as the target + // of the merge + args.unshift({}); + + // customize conflict resolution to match our historical merge behavior + args.push(customizer); + + merge.apply(null, args); + + // return the mutated result object + return args[0]; } diff --git a/src/js/video.js b/src/js/video.js index 643507648e..ed557a1021 100644 --- a/src/js/video.js +++ b/src/js/video.js @@ -142,7 +142,7 @@ videojs.options = createDeprecationProxy(globalOptions, { * @method setGlobalOptions */ videojs.setGlobalOptions = function(newOptions) { - return mergeOptions(globalOptions, newOptions); + return merge(globalOptions, newOptions); }; /** @@ -264,7 +264,7 @@ videojs.TOUCH_ENABLED = browser.TOUCH_ENABLED; videojs.extends = extendsFn; /** - * Merge two options objects recursively + * Merge options objects recursively * Performs a deep merge like lodash.merge but **only merges plain objects** * (not arrays, elements, anything else) * Other values will be copied directly from the second object. @@ -288,9 +288,7 @@ videojs.extends = extendsFn; * // result.bar.b = [4,5,6]; * ``` * - * @param {Object} The options object whose values will be overriden - * @param {Object} The options object with values to override the first - * @param {Object} Any number of additional options objects + * @param {...Object} source One or more objects to merge * * @return {Object} a new object with the merged values * @mixes videojs diff --git a/test/unit/player.test.js b/test/unit/player.test.js index 78e49ec60f..fe193fdd9b 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -142,7 +142,7 @@ test('should asynchronously fire error events during source selection', function { 'src': 'http://vjs.zencdn.net/v/oceans.mp4', 'type': 'video/mp4' } ] }); - ok(player.options_['techOrder'][0] === 'foo', 'Foo listed as the only tech'); + equal(player.options_['techOrder'][0], 'foo', 'Foo listed as the only tech'); player.on('error', function(e) { ok(player.error().code === 4, 'Source could not be played error thrown'); diff --git a/test/unit/utils/merge-options.test.js b/test/unit/utils/merge-options.test.js index 2e8aec9f8e..f1c1fe6b8e 100644 --- a/test/unit/utils/merge-options.test.js +++ b/test/unit/utils/merge-options.test.js @@ -27,3 +27,16 @@ test('should merge options objects', function(){ d: true }, 'options objects merged correctly'); }); + +test('should not mutate the first argument', function(){ + let obj = { + prop: { + a: 'a' + } + }; + let merged = mergeOptions(obj, { prop: { b: 'b' } }); + + ok(obj !== merged, 'did not return the original'); + deepEqual(merged, { prop: { a: 'a', b: 'b' } }, 'merged the sub object'); + ok(!('b' in obj.prop), 'did not mutate the original'); +}); diff --git a/test/unit/video.test.js b/test/unit/video.test.js index b4ee5bd09d..a03902c3dc 100644 --- a/test/unit/video.test.js +++ b/test/unit/video.test.js @@ -59,6 +59,15 @@ test('should add the value to the languages object with lower case lang code', f deepEqual(result, globalOptions['languages'][code.toLowerCase()], 'should also match'); }); +test('should allow setting global options', function() { + videojs.setGlobalOptions({ + nonsense: true + }); + + equal(videojs.getGlobalOptions().nonsense, true, 'set the new option'); + deepEqual(videojs.options, videojs.getGlobalOptions(), 'updated the options property'); +}); + test('should expose plugin registry function', function() { var pluginName, pluginFunction, player;