Skip to content

Commit

Permalink
Use spread operator instead of apply when supported
Browse files Browse the repository at this point in the history
Introduces non-backward compatible changes to automatic constructor
detection in createFactory.
  • Loading branch information
Krzysztof Chrapka committed Apr 11, 2016
1 parent 4185eb1 commit 81e44b8
Show file tree
Hide file tree
Showing 10 changed files with 267 additions and 40 deletions.
2 changes: 1 addition & 1 deletion lib/WireProxy.js
Original file line number Diff line number Diff line change
Expand Up @@ -202,4 +202,4 @@ define(function(require) {

});
})(typeof define == 'function' && define.amd ? define : function(factory) { module.exports = factory(require); }
);
);
45 changes: 45 additions & 0 deletions lib/es5Apply.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
(function(define) {
'use strict';
define(function() {
/**
* Carefully sets the instance's constructor property to the supplied
* constructor, using Object.defineProperty if available. If it can't
* set the constructor in a safe way, it will do nothing.
*
* @param instance {Object} component instance
* @param ctor {Function} constructor
*/
function defineConstructorIfPossible(instance, ctor) {
try {
Object.defineProperty(instance, 'constructor', {
value: ctor,
enumerable: false
});
} catch(e) {
// If we can't define a constructor, oh well.
// This can happen if in envs where Object.defineProperty is not
// available, or when using cujojs/poly or other ES5 shims
}
}

return function(func, thisObj, args) {
var result = null;

if(thisObj && typeof thisObj[func] === 'function') {
func = thisObj[func];
}

// detect case when apply is called on constructor and fix prototype chain
if (thisObj === func) {
thisObj = Object.create(func.prototype);
defineConstructorIfPossible(thisObj, func);
func.apply(thisObj, args);
result = thisObj;
} else {
result = func.apply(thisObj, args);
}

return result;
};
});
})(typeof define === 'function' && define.amd ? define : function(factory) { module.exports = factory(); });
28 changes: 28 additions & 0 deletions lib/es6Apply.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/* jshint esversion: 6 */
(function(define) {
'use strict';
define(function() {
return function(func, thisObj, args) {
var result = null;

if(thisObj === func || (thisObj && thisObj.constructor === func)) {
/* jshint newcap: false */
result = new func(...(args||[]));

// detect broken old prototypes with missing constructor
if (result.constructor !== func) {
Object.defineProperty(result, 'constructor', {
enumerable: false,
value: func
});
}
} else if(thisObj && typeof thisObj[func] === 'function') {
result = thisObj[func](...args);
} else {
result = func.apply(thisObj, args);
}

return result;
};
});
})(typeof define === 'function' && define.amd ? define : function(factory) { module.exports = factory(); });
48 changes: 16 additions & 32 deletions lib/instantiate.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,9 @@
(function(define){ 'use strict';
define(function() {

var undef;
var undef, universalApply;

universalApply = require('./universalApply');

/**
* Creates an object by either invoking ctor as a function and returning the result,
Expand All @@ -26,42 +28,19 @@ define(function() {
var begotten, ctorResult;

if (forceConstructor || (forceConstructor === undef && isConstructor(ctor))) {
begotten = Object.create(ctor.prototype);
defineConstructorIfPossible(begotten, ctor);
ctorResult = ctor.apply(begotten, args);
begotten = ctor;
ctorResult = universalApply(ctor, begotten, args);

if(ctorResult !== undef) {
begotten = ctorResult;
}

} else {
begotten = ctor.apply(undef, args);

begotten = universalApply(ctor, undef, args);
}

return begotten === undef ? null : begotten;
};

/**
* Carefully sets the instance's constructor property to the supplied
* constructor, using Object.defineProperty if available. If it can't
* set the constructor in a safe way, it will do nothing.
*
* @param instance {Object} component instance
* @param ctor {Function} constructor
*/
function defineConstructorIfPossible(instance, ctor) {
try {
Object.defineProperty(instance, 'constructor', {
value: ctor,
enumerable: false
});
} catch(e) {
// If we can't define a constructor, oh well.
// This can happen if in envs where Object.defineProperty is not
// available, or when using cujojs/poly or other ES5 shims
}
}

/**
* Determines whether the supplied function should be invoked directly or
* should be invoked using new in order to create the object to be wired.
Expand All @@ -72,10 +51,15 @@ define(function() {
*/
function isConstructor(func) {
var is = false, p;
for (p in func.prototype) {
if (p !== undef) {
is = true;
break;

is = is || !!func.constructor;

if(!is) {
for (p in func.prototype) {
if (p !== undef) {
is = true;
break;
}
}
}

Expand Down
6 changes: 4 additions & 2 deletions lib/invoker.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
(function(define) {
define(function() {

var universalApply = require('./universalApply');

return function(methodName, args) {
return function(target) {
return target[methodName].apply(target, args);
return universalApply(target[methodName], target, args);
};
};

});
})(typeof define === 'function' && define.amd ? define : function(factory) { module.exports = factory(); });
})(typeof define === 'function' && define.amd ? define : function(factory) { module.exports = factory(); });
6 changes: 3 additions & 3 deletions lib/plugin/basePlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,15 @@
define(function(require) {

var when, object, functional, pipeline, instantiate, createInvoker,
whenAll, obj, pluginInstance, undef;
whenAll, obj, pluginInstance, undef, universalApply;

when = require('when');
object = require('../object');
functional = require('../functional');
pipeline = require('../pipeline');
instantiate = require('../instantiate');
createInvoker = require('../invoker');
universalApply = require('../universalApply');

whenAll = when.all;

Expand Down Expand Up @@ -194,7 +195,6 @@ define(function(require) {
if (!proxy.clone) {
throw new Error('No clone function found for ' + componentDef.id);
}

return proxy.clone(options);
});
});
Expand Down Expand Up @@ -249,7 +249,7 @@ define(function(require) {
// We'll either use the module directly, or we need
// to instantiate/invoke it.
return constructorName
? module[constructorName].apply(module, args)
? universalApply(constructorName, module, args)
: typeof module == 'function'
? instantiate(module, args, isConstructor)
: Object.create(module);
Expand Down
45 changes: 45 additions & 0 deletions lib/universalApply.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
(function(){
'use strict';

(function(define){

function evaluates (statement) {
try {
/* jshint evil: true */
eval(statement);
/* jshint evil: false */
return true;
} catch (err) {
return false;
}
}

// we have to know it synchronously, we are unable to load this module in asynchronous way
// we cannot defer `define` and we cannot load module, that would not compile in browser
// so we can't delegate this check to another module
function isSpreadAvailable() {
return evaluates('Math.max(...[ 5, 10 ])');
}

var requires = [];
if (typeof('process') !== 'undefined' && 'ES_VERSION' in process.env) {
requires.push('./es'+process.env.ES_VERSION+'Apply');
} else {
if(isSpreadAvailable()) {
requires.push('./es6Apply');
} else {
requires.push('./es5Apply');
}
}

define('universalApply', requires, function(apply){
return apply;
});
})(
typeof define === 'function'
? define
: function(name, requires, factory) {
module.exports = factory.apply(null, requires.map(require));
}
);
})();
25 changes: 23 additions & 2 deletions test/buster.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,29 @@
require('gent/test-adapter/buster');

function evaluates (statement) {
try {
/* jshint evil: true */
eval(statement);
/* jshint evil: false */
return true;
} catch (err) {
return false;
}
}

function isSpreadAvailable() {
return evaluates('Math.max(...[ 5, 10 ])');
}

var tests = ['node/**/*-test.js'];

if(isSpreadAvailable && !('ES_VERSION' in process.env && process.env.ES_VERSION < 6) ) {
tests.push('node-es6/**/*-test.js');
}

module.exports['node'] = {
environment: 'node',
tests: ['node/**/*-test.js']
tests: tests
// TODO: Why doesn't this work?
//, testHelpers:['gent/test-adapter/buster']
};
};
97 changes: 97 additions & 0 deletions test/node-es6/lib/plugin/basePlugin-test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
/* jshint esversion: 6 */
(function(buster, context) {
'use strict';

var assert, refute, fail, sentinel;

assert = buster.assert;
refute = buster.refute;
fail = buster.fail;

sentinel = {};

function createContext(spec) {
return context.call(null, spec, null, { require: require });
}

class es6Class
{
constructor () {
this.constructorRan = true;
this.args = Array.prototype.slice.call(arguments);
}

someMethod() {

}
}

buster.testCase('es6/lib/plugin/basePlugin', {
'clone factory': {
'should call constructor when cloning an object with a constructor': function() {
class FabulousEs6 {
constructor () {
this.instanceProp = 'instanceProp';
}
}
FabulousEs6.prototype.prototypeProp = 'prototypeProp';

return createContext({
fab: {
create: FabulousEs6
},
copy: {
clone: { $ref: 'fab' }
}
}).then(
function(context) {
assert.defined(context.copy, 'copy is defined');
assert.defined(context.copy.prototypeProp, 'copy.prototypeProp is defined');
assert.defined(context.copy.instanceProp, 'copy.instanceProp is defined');
refute.same(context.copy, context.fab);
},
fail
);
}
},

'create factory': {
'should call es6 constructor': function() {
return createContext({
test: {
create: {
module: es6Class,
isConstructor: true
}
}
}).then(
function(context) {
assert(context.test.constructorRan);
},
fail
);
},

'should call es6 constructor functions with args': function() {
return createContext({
test: {
create: {
module: es6Class,
isConstructor: true,
args: [1, 'foo', 1.7]
}
}
}).then(
function(context) {
assert(context.test instanceof es6Class);
assert.equals(context.test.args, [1, 'foo', 1.7]);
},
fail
);
},
}
});
})(
require('buster'),
require('../../../../lib/context')
);
Loading

0 comments on commit 81e44b8

Please sign in to comment.