Skip to content

Commit

Permalink
refactor: expose tech but warn without safety var (#3916)
Browse files Browse the repository at this point in the history
`Player#tech` can now be called without passing an object into it. It no longer requires passing an object into it, so, current code will not break.
If nothing is passed in, a warning will be logged about knowing what you're doing. If anything is passed in, the warning is silenced.
  • Loading branch information
gkatsev committed Jan 18, 2017
1 parent 73b6316 commit 8622b26
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 32 deletions.
34 changes: 14 additions & 20 deletions src/js/player.js
Expand Up @@ -6,6 +6,7 @@ import Component from './component.js';

import document from 'global/document';
import window from 'global/window';
import tsml from 'tsml';
import * as Events from './utils/events.js';
import * as Dom from './utils/dom.js';
import * as Fn from './utils/fn.js';
Expand Down Expand Up @@ -920,32 +921,25 @@ class Player extends Component {
}

/**
* Return a reference to the current {@link Tech}, but only if given an object with the
* `IWillNotUseThisInPlugins` property having a true value. This is try and prevent misuse
* of techs by plugins.
* Return a reference to the current {@link Tech}.
* It will print a warning by default about the danger of using the tech directly
* but any argument that is passed in will silence the warning.
*
* @param {Object} safety
* An object that must contain `{IWillNotUseThisInPlugins: true}`
*
* @param {boolean} safety.IWillNotUseThisInPlugins
* Must be set to true or else this function will throw an error.
* @param {*} [safety]
* Anything passed in to silence the warning
*
* @return {Tech}
* The Tech
*/
tech(safety) {
if (safety && safety.IWillNotUseThisInPlugins) {
return this.tech_;
}
const errorText = `
Please make sure that you are not using this inside of a plugin.
To disable this alert and error, please pass in an object with
\`IWillNotUseThisInPlugins\` to the \`tech\` method. See
https://github.com/videojs/video.js/issues/2617 for more info.
`;

window.alert(errorText);
throw new Error(errorText);
if (safety === undefined) {
log.warn(tsml`
Using the tech directly can be dangerous. I hope you know what you're doing.
See https://github.com/videojs/video.js/issues/2617 for more info.
`);
}

return this.tech_;
}

/**
Expand Down
33 changes: 21 additions & 12 deletions test/unit/player.test.js
Expand Up @@ -1209,29 +1209,38 @@ QUnit.test('you can clear error in the error event', function(assert) {
});

QUnit.test('Player#tech will return tech given the appropriate input', function(assert) {
const oldLogWarn = log.warn;
let warning;

log.warn = function(_warning) {
warning = _warning;
};

const tech_ = {};
const returnedTech = Player.prototype.tech.call({tech_}, {IWillNotUseThisInPlugins: true});
const returnedTech = Player.prototype.tech.call({tech_}, true);

assert.equal(returnedTech, tech_, 'We got back the tech we wanted');
assert.notOk(warning, 'no warning was logged');

log.warn = oldLogWarn;
});

QUnit.test('Player#tech alerts and throws without the appropriate input', function(assert) {
let alertCalled;
const oldAlert = window.alert;
QUnit.test('Player#tech logs a warning when called without a safety argument', function(assert) {
const oldLogWarn = log.warn;
const warningRegex = new RegExp('https://github.com/videojs/video.js/issues/2617');
let warning;

window.alert = () => {
alertCalled = true;
log.warn = function(_warning) {
warning = _warning;
};

const tech_ = {};

assert.throws(function() {
Player.prototype.tech.call({tech_});
}, new RegExp('https://github.com/videojs/video.js/issues/2617'),
'we threw an error');
Player.prototype.tech.call({tech_});

assert.ok(warningRegex.test(warning), 'we logged a warning');

assert.ok(alertCalled, 'we called an alert');
window.alert = oldAlert;
log.warn = oldLogWarn;
});

QUnit.test('player#reset loads the Html5 tech and then techCalls reset', function(assert) {
Expand Down

0 comments on commit 8622b26

Please sign in to comment.