Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore options/players Globals #2395

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions src/js/utils/create-deprecation-proxy.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
import log from './log.js';

/**
* Object containing the default behaviors for available handler methods.
*
* @private
* @type {Object}
*/
const defaultBehaviors = {
get(obj, key) {
return obj[key];
},
set(obj, key, value) {
obj[key] = value;
return true;
}
};

/**
* Expose private objects publicly using a Proxy to log deprecation warnings.
*
* Browsers that do not support Proxy objects will simply return the `target`
* object, so it can be directly exposed.
*
* @param {Object} target The target object.
* @param {Object} messages Messages to display from a Proxy. Only operations
* with an associated message will be proxied.
* @param {String} [messages.get]
* @param {String} [messages.set]
* @return {Object} A Proxy if supported or the `target` argument.
*/
export default (target, messages={}) => {
if (typeof Proxy === 'function') {
let handler = {};

// Build a handler object based on those keys that have both messages
// and default behaviors.
Object.keys(messages).forEach(key => {
if (defaultBehaviors.hasOwnProperty(key)) {
handler[key] = function() {
log.warn(messages[key]);
return defaultBehaviors[key].apply(this, arguments);
};
}
});

return new Proxy(target, handler);
}
return target;
};
25 changes: 25 additions & 0 deletions src/js/video.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import * as Dom from './utils/dom.js';
import * as browser from './utils/browser.js';
import extendsFn from './extends.js';
import merge from 'lodash-compat/object/merge';
import createDeprecationProxy from './utils/create-deprecation-proxy.js';

// Include the built-in techs
import Html5 from './tech/html5.js';
Expand Down Expand Up @@ -111,6 +112,18 @@ videojs.VERSION = '__VERSION__';
*/
videojs.getGlobalOptions = () => globalOptions;

/**
* For backward compatibility, expose global options.
*
* @deprecated
* @memberOf videojs
* @property {Object|Proxy} options
*/
videojs.options = createDeprecationProxy(globalOptions, {
get: 'Access to videojs.options is deprecated; use videojs.getGlobalOptions instead',
set: 'Modification of videojs.options is deprecated; use videojs.setGlobalOptions instead'
});

/**
* Set options that will apply to every player
* ```js
Expand Down Expand Up @@ -141,6 +154,18 @@ videojs.getPlayers = function() {
return Player.players;
};

/**
* For backward compatibility, expose players object.
*
* @deprecated
* @memberOf videojs
* @property {Object|Proxy} players
*/
videojs.players = createDeprecationProxy(Player.players, {
get: 'Access to videojs.players is deprecated; use videojs.getPlayers instead',
set: 'Modification of videojs.players is deprecated'
});

/**
* Get a component class object by name
* ```js
Expand Down
45 changes: 45 additions & 0 deletions test/unit/utils/create-deprecation-proxy.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import createDeprecationProxy from '../../../src/js/utils/create-deprecation-proxy.js';
import log from '../../../src/js/utils/log.js';

const proxySupported = typeof Proxy === 'function';

test('should return a Proxy object when supported or the target object by reference', function() {
let target = {foo: 1};
let subject = createDeprecationProxy(target, {
get: 'get message',
set: 'set message'
});

// Testing for a Proxy is really difficult because Proxy objects by their
// nature disguise the fact that they are in fact Proxy objects. So, this
// tests that the log.warn method gets called on property get/set operations
// to detect the Proxy.
if (proxySupported) {
sinon.stub(log, 'warn');

subject.foo; // Triggers a "get"
subject.foo = 2; // Triggers a "set"

equal(log.warn.callCount, 2, 'proxied operations cause deprecation warnings');
ok(log.warn.calledWith('get message'), 'proxied get logs expected message');
ok(log.warn.calledWith('set message'), 'proxied set logs expected message');

log.warn.restore();
} else {
strictEqual(target, subject, 'identical to target');
}
});

// Tests run only in Proxy-supporting environments.
if (proxySupported) {
test('no deprecation warning is logged for operations without a message', function() {
let subject = createDeprecationProxy({}, {
get: 'get message'
});

sinon.stub(log, 'warn');
subject.foo = 'bar'; // Triggers a "set," but not a "get"
equal(log.warn.callCount, 0, 'no deprecation warning expected');
log.warn.restore();
});
}
6 changes: 6 additions & 0 deletions test/unit/video.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import videojs from '../../src/js/video.js';
import TestHelpers from './test-helpers.js';
import Player from '../../src/js/player.js';
import globalOptions from '../../src/js/global-options.js';
import log from '../../src/js/utils/log.js';
import document from 'global/document';

q.module('video.js');
Expand Down Expand Up @@ -75,3 +76,8 @@ test('should expose plugin registry function', function() {
ok(player.foo, 'should exist');
equal(player.foo, pluginFunction, 'should be equal');
});

test('should expose options and players properties for backward-compatibility', function() {
ok(typeof videojs.options, 'object', 'options should be an object');
ok(typeof videojs.players, 'object', 'players should be an object');
});