Skip to content

Commit

Permalink
feat: don't throw when re-registering a plugin unless it's a player m…
Browse files Browse the repository at this point in the history
…ethod (#4140)
  • Loading branch information
misteroneill authored and gkatsev committed Mar 2, 2017
1 parent 0f57341 commit 326398d
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 8 deletions.
7 changes: 5 additions & 2 deletions src/js/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import evented from './mixins/evented';
import stateful from './mixins/stateful';
import * as Events from './utils/events';
import * as Fn from './utils/fn';
import log from './utils/log';
import Player from './player';

/**
Expand Down Expand Up @@ -305,8 +306,10 @@ class Plugin {
throw new Error(`Illegal plugin name, "${name}", must be a string, was ${typeof name}.`);
}

if (pluginExists(name) || Player.prototype.hasOwnProperty(name)) {
throw new Error(`Illegal plugin name, "${name}", already exists.`);
if (pluginExists(name)) {
log.warn(`A plugin named "${name}" already exists. You may want to avoid re-registering plugins!`);
} else if (Player.prototype.hasOwnProperty(name)) {
throw new Error(`Illegal plugin name, "${name}", cannot share a name with an existing player method!`);
}

if (typeof plugin !== 'function') {
Expand Down
20 changes: 14 additions & 6 deletions test/unit/plugin-static.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
/* eslint-env qunit */
import sinon from 'sinon';
import log from '../../src/js/utils/log';
import Player from '../../src/js/player';
import Plugin from '../../src/js/plugin';

Expand Down Expand Up @@ -53,12 +55,6 @@ QUnit.test('registerPlugin() illegal arguments', function(assert) {
'plugins must have a name'
);

assert.throws(
() => Plugin.registerPlugin('play'),
new Error('Illegal plugin name, "play", already exists.'),
'plugins cannot share a name with an existing player method'
);

assert.throws(
() => Plugin.registerPlugin('foo'),
new Error('Illegal plugin for "foo", must be a function, was undefined.'),
Expand All @@ -70,6 +66,18 @@ QUnit.test('registerPlugin() illegal arguments', function(assert) {
new Error('Illegal plugin for "foo", must be a function, was object.'),
'plugins must be functions'
);

assert.throws(
() => Plugin.registerPlugin('play', function() {}),
new Error('Illegal plugin name, "play", cannot share a name with an existing player method!'),
'plugins must be functions'
);

sinon.spy(log, 'warn');
Plugin.registerPlugin('foo', function() {});
Plugin.registerPlugin('foo', function() {});
assert.strictEqual(log.warn.callCount, 1, 'warn on re-registering a plugin');
log.warn.restore();
});

QUnit.test('getPlugin()', function(assert) {
Expand Down

0 comments on commit 326398d

Please sign in to comment.