diff --git a/modules/ve/test/ve.Factory.test.js b/modules/ve/test/ve.Factory.test.js index 8bd2920308..0b654e4ac5 100644 --- a/modules/ve/test/ve.Factory.test.js +++ b/modules/ve/test/ve.Factory.test.js @@ -9,10 +9,11 @@ QUnit.module( 've.Factory' ); /* Stubs */ -ve.FactoryObjectStub = function VeFactoryObjectStub( a, b, c ) { +ve.FactoryObjectStub = function VeFactoryObjectStub( a, b, c, d ) { this.a = a; this.b = b; this.c = c; + this.d = d; }; /* Tests */ @@ -24,24 +25,36 @@ QUnit.test( 'register', 1, function ( assert ) { factory.register( 'factory-object-stub', 'not-a-function' ); }, Error, - 'throws an exception when trying to register a non-function value as a constructor' + 'Throws an exception when trying to register a non-function value as a constructor' ); } ); -QUnit.test( 'create', 2, function ( assert ) { - var factory = new ve.Factory(); +QUnit.test( 'create', 3, function ( assert ) { + var obj, + factory = new ve.Factory(); + assert.throws( function () { factory.create( 'factory-object-stub', 23, 'foo', { 'bar': 'baz' } ); }, Error, - 'throws an exception when trying to create a object of an unregistered type' + 'Throws an exception when trying to create a object of an unregistered type' ); + factory.register( 'factory-object-stub', ve.FactoryObjectStub ); + + obj = factory.create( 'factory-object-stub', 16, 'foo', { 'baz': 'quux' }, 5 ); + assert.deepEqual( - factory.create( 'factory-object-stub', 16, 'foo', { 'baz': 'quux' } ), - new ve.FactoryObjectStub( 16, 'foo', { 'baz': 'quux' } ), - 'creates objects of a registered type and passes through arguments' + obj, + new ve.FactoryObjectStub( 16, 'foo', { 'baz': 'quux' }, 5 ), + 'Creates an object of the registered type and passes through arguments' + ); + + assert.strictEqual( + obj instanceof ve.FactoryObjectStub, + true, + 'Creates an object that is an instanceof the registered constructor' ); } ); diff --git a/modules/ve/ve.Factory.js b/modules/ve/ve.Factory.js index f2d681c04a..f901ea61ca 100644 --- a/modules/ve/ve.Factory.js +++ b/modules/ve/ve.Factory.js @@ -52,22 +52,31 @@ ve.Factory.prototype.register = function ( type, constructor ) { * Type is used to look up the constructor to use, while all additional arguments are passed to the * constructor directly, so leaving one out will pass an undefined to the constructor. * - * WARNING: JavaScript does not allow using new and .apply together, so we just pass through 3 - * arguments here since we know we only need that many at this time. If we need more in the future - * we should change this to suit that use case. Because of undefined pass through, there's no harm - * in adding more. - * * @method - * @param {String} type Object type - * @param {Mixed} [...] Up to 3 additional arguments to pass through to the constructor - * @returns {Object} The new object + * @param {string} type Object type. + * @param {mixed} [...] Arguments to pass to the constructor. + * @returns {Object} The new object. * @throws 'Unknown object type' */ -ve.Factory.prototype.create = function ( type, a, b, c ) { - if ( type in this.registry ) { - return new this.registry[type]( a, b, c ); +ve.Factory.prototype.create = function ( type ) { + var args, obj, + constructor = this.registry[type]; + + if ( constructor === undefined ) { + throw new Error( 'Unknown object type: ' + type ); } - throw new Error( 'Unknown object type: ' + type ); + + // Convert arguments to array and shift the first argument (type) off + args = Array.prototype.slice.call( arguments, 1 ); + + // We can't use the "new" operator with .apply directly because apply needs a + // context. So instead just do what "new" does: Create an object that inherits from + // the constructor's prototype (which also makes it an "instanceof" the constructor), + // then invoke the constructor with the object as context, and return it (ignoring + // the constructor's return value). + obj = ve.createObject( constructor.prototype ); + constructor.apply( obj, args ); + return obj; }; /**