Skip to content

Commit

Permalink
Throw when attempting to register a tech as a component
Browse files Browse the repository at this point in the history
  • Loading branch information
misteroneill committed Jan 4, 2017
1 parent 44655ae commit 8b717c7
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 17 deletions.
26 changes: 20 additions & 6 deletions src/js/component.js
Original file line number Diff line number Diff line change
Expand Up @@ -1542,13 +1542,27 @@ class Component {
* The `Component` that was registered.
*/
static registerComponent(name, ComponentToRegister) {
if (typeof name !== 'string' || !name.length) {
throw new Error(`illegal component name, "${name}"; must be a non-empty string`);
if (typeof name !== 'string' || !name) {
throw new Error(`Illegal component name, "${name}"; must be a non-empty string.`);
}

if (!Component.prototype.isPrototypeOf(ComponentToRegister.prototype) &&
Component !== ComponentToRegister) {
throw new Error(`illegal component constructor, "${name}"; must be a subclass of Component`);
const Tech = Component.getComponent('Tech');

// We need to make sure this check is only done if Tech has been registered.
const isTech = Tech && Tech.isTech(ComponentToRegister);
const isComp = Component === ComponentToRegister ||
Component.prototype.isPrototypeOf(ComponentToRegister.prototype);

if (isTech || !isComp) {
let reason;

if (isTech) {
reason = 'techs must be registered using Tech.registerTech()';
} else {
reason = 'must be a Component subclass';
}

throw new Error(`Illegal component, "${name}"; ${reason}.`);
}

name = toTitleCase(name);
Expand All @@ -1567,7 +1581,7 @@ class Component {
if (players &&
Object.keys(players).length > 0 &&
Object.keys(players).map((playerName) => players[playerName]).every(Boolean)) {
throw new Error('Can not register Player component after player has been created');
throw new Error('Can not register Player component after player has been created.');
}
}

Expand Down
2 changes: 0 additions & 2 deletions src/js/tech/flash.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import * as Dom from '../utils/dom.js';
import * as Url from '../utils/url.js';
import { createTimeRange } from '../utils/time-ranges.js';
import FlashRtmpDecorator from './flash-rtmp';
import Component from '../component';
import window from 'global/window';
import {assign} from '../utils/obj';

Expand Down Expand Up @@ -1075,6 +1074,5 @@ Flash.getEmbedCode = function(swf, flashVars, params, attributes) {
// Run Flash through the RTMP decorator
FlashRtmpDecorator(Flash);

Component.registerComponent('Flash', Flash);
Tech.registerTech('Flash', Flash);
export default Flash;
2 changes: 0 additions & 2 deletions src/js/tech/html5.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
* @file html5.js
*/
import Tech from './tech.js';
import Component from '../component';
import * as Dom from '../utils/dom.js';
import * as Url from '../utils/url.js';
import * as Fn from '../utils/fn.js';
Expand Down Expand Up @@ -1662,6 +1661,5 @@ Html5.nativeSourceHandler.dispose = function() {};
// Register the native source handler
Html5.registerSourceHandler(Html5.nativeSourceHandler);

Component.registerComponent('Html5', Html5);
Tech.registerTech('Html5', Html5);
export default Html5;
6 changes: 3 additions & 3 deletions src/js/tech/tech.js
Original file line number Diff line number Diff line change
Expand Up @@ -1187,9 +1187,9 @@ Tech.withSourceHandlers = function(_Tech) {

};

// The base Tech class needs to be registered as a Component. It is the only
// Tech that can be registered as a Component.
Component.registerComponent('Tech', Tech);
// Old name for Tech
// @deprecated
Component.registerComponent('MediaTechController', Tech);
Tech.registerTech('Tech', Tech);

export default Tech;
20 changes: 16 additions & 4 deletions test/unit/component.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,23 +46,35 @@ QUnit.test('registerComponent() throws with bad arguments', function(assert) {
function() {
Component.registerComponent(null);
},
new Error('illegal component name, "null"; must be a non-empty string'),
new Error('Illegal component name, "null"; must be a non-empty string.'),
'component names must be non-empty strings'
);

assert.throws(
function() {
Component.registerComponent('');
},
new Error('illegal component name, ""; must be a non-empty string'),
new Error('Illegal component name, ""; must be a non-empty string.'),
'component names must be non-empty strings'
);

assert.throws(
function() {
Component.registerComponent('TestComponent5', function() {});
},
new Error('illegal component constructor, "TestComponent5"; must be a subclass of Component'),
new Error('Illegal component, "TestComponent5"; must be a Component subclass.'),
'components must be subclasses of Component'
);

assert.throws(
function() {
const Tech = Component.getComponent('Tech');

class DummyTech extends Tech {}

Component.registerComponent('TestComponent5', DummyTech);
},
new Error('Illegal component, "TestComponent5"; techs must be registered using Tech.registerTech().'),
'components must be subclasses of Component'
);
});
Expand Down Expand Up @@ -661,7 +673,7 @@ QUnit.test('should use a defined content el for appending children', function(as
class CompWithContent extends Component {}

CompWithContent.prototype.createEl = function() {
// Create the main componenent element
// Create the main component element
const el = Dom.createEl('div');

// Create the element where children will be appended
Expand Down

0 comments on commit 8b717c7

Please sign in to comment.