From 8622b2648e6ad75b9d968e2f2027c6baab27d03f Mon Sep 17 00:00:00 2001 From: Gary Katsevman Date: Wed, 18 Jan 2017 00:40:24 -0500 Subject: [PATCH] refactor: expose tech but warn without safety var (#3916) `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. --- src/js/player.js | 30 ++++++++++++------------------ test/unit/player.test.js | 33 +++++++++++++++++++++------------ 2 files changed, 33 insertions(+), 30 deletions(-) diff --git a/src/js/player.js b/src/js/player.js index 4ed368258..0803f0aed 100644 --- a/src/js/player.js +++ b/src/js/player.js @@ -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'; @@ -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_; + 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. + `); } - 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); + return this.tech_; } /** diff --git a/test/unit/player.test.js b/test/unit/player.test.js index c37477621..65ad9a5df 100644 --- a/test/unit/player.test.js +++ b/test/unit/player.test.js @@ -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(alertCalled, 'we called an alert'); - window.alert = oldAlert; + assert.ok(warningRegex.test(warning), 'we logged a warning'); + + log.warn = oldLogWarn; }); QUnit.test('player#reset loads the Html5 tech and then techCalls reset', function(assert) {