Skip to content

Commit b365c09

Browse files
author
Krzysztof Chrapka
committed
Use spread operator instead of apply when supported
Introduces non-backward compatible changes to automatic constructor detection in createFactory.
1 parent 4185eb1 commit b365c09

File tree

10 files changed

+278
-40
lines changed

10 files changed

+278
-40
lines changed

‎lib/WireProxy.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,4 +202,4 @@ define(function(require) {
202202

203203
});
204204
})(typeof define == 'function' && define.amd ? define : function(factory) { module.exports = factory(require); }
205-
);
205+
);

‎lib/es5Apply.js‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
(function(define) {
2+
'use strict';
3+
define(function() {
4+
/**
5+
* Carefully sets the instance's constructor property to the supplied
6+
* constructor, using Object.defineProperty if available. If it can't
7+
* set the constructor in a safe way, it will do nothing.
8+
*
9+
* @param instance {Object} component instance
10+
* @param ctor {Function} constructor
11+
*/
12+
function defineConstructorIfPossible(instance, ctor) {
13+
try {
14+
Object.defineProperty(instance, 'constructor', {
15+
value: ctor,
16+
enumerable: false
17+
});
18+
} catch(e) {
19+
// If we can't define a constructor, oh well.
20+
// This can happen if in envs where Object.defineProperty is not
21+
// available, or when using cujojs/poly or other ES5 shims
22+
}
23+
}
24+
25+
return function(func, thisObj, args) {
26+
var result = null;
27+
28+
if(thisObj && typeof thisObj[func] === 'function') {
29+
func = thisObj[func];
30+
}
31+
32+
// detect case when apply is called on constructor and fix prototype chain
33+
if (thisObj === func) {
34+
thisObj = Object.create(func.prototype);
35+
defineConstructorIfPossible(thisObj, func);
36+
func.apply(thisObj, args);
37+
result = thisObj;
38+
} else {
39+
result = func.apply(thisObj, args);
40+
}
41+
42+
return result;
43+
};
44+
});
45+
})(typeof define === 'function' && define.amd ? define : function(factory) { module.exports = factory(); });

‎lib/es6Apply.js‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/* jshint esversion: 6 */
2+
(function(define) {
3+
'use strict';
4+
define(function() {
5+
return function(func, thisObj, args) {
6+
var result = null;
7+
8+
if(thisObj === func || (thisObj && thisObj.constructor === func)) {
9+
/* jshint newcap: false */
10+
result = new func(...(args||[]));
11+
12+
// detect broken old prototypes with missing constructor
13+
if (result.constructor !== func) {
14+
Object.defineProperty(result, 'constructor', {
15+
enumerable: false,
16+
value: func
17+
});
18+
}
19+
} else if(thisObj && typeof thisObj[func] === 'function') {
20+
result = thisObj[func](...args);
21+
} else {
22+
result = func.apply(thisObj, args);
23+
}
24+
25+
return result;
26+
};
27+
});
28+
})(typeof define === 'function' && define.amd ? define : function(factory) { module.exports = factory(); });

‎lib/instantiate.js‎

Lines changed: 14 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@
88
(function(define){ 'use strict';
99
define(function() {
1010

11-
var undef;
11+
var undef, universalApply;
12+
13+
universalApply = require('./universalApply');
1214

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

2830
if (forceConstructor || (forceConstructor === undef && isConstructor(ctor))) {
29-
begotten = Object.create(ctor.prototype);
30-
defineConstructorIfPossible(begotten, ctor);
31-
ctorResult = ctor.apply(begotten, args);
31+
begotten = ctor;
32+
ctorResult = universalApply(ctor, begotten, args);
33+
3234
if(ctorResult !== undef) {
3335
begotten = ctorResult;
3436
}
35-
3637
} else {
37-
begotten = ctor.apply(undef, args);
38-
38+
begotten = universalApply(ctor, undef, args);
3939
}
4040

4141
return begotten === undef ? null : begotten;
4242
};
4343

44-
/**
45-
* Carefully sets the instance's constructor property to the supplied
46-
* constructor, using Object.defineProperty if available. If it can't
47-
* set the constructor in a safe way, it will do nothing.
48-
*
49-
* @param instance {Object} component instance
50-
* @param ctor {Function} constructor
51-
*/
52-
function defineConstructorIfPossible(instance, ctor) {
53-
try {
54-
Object.defineProperty(instance, 'constructor', {
55-
value: ctor,
56-
enumerable: false
57-
});
58-
} catch(e) {
59-
// If we can't define a constructor, oh well.
60-
// This can happen if in envs where Object.defineProperty is not
61-
// available, or when using cujojs/poly or other ES5 shims
62-
}
63-
}
64-
6544
/**
6645
* Determines whether the supplied function should be invoked directly or
6746
* should be invoked using new in order to create the object to be wired.
@@ -72,10 +51,13 @@ define(function() {
7251
*/
7352
function isConstructor(func) {
7453
var is = false, p;
75-
for (p in func.prototype) {
76-
if (p !== undef) {
77-
is = true;
78-
break;
54+
55+
if(!is) {
56+
for (p in func.prototype) {
57+
if (p !== undef) {
58+
is = true;
59+
break;
60+
}
7961
}
8062
}
8163

‎lib/invoker.js‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
(function(define) {
22
define(function() {
33

4+
var universalApply = require('./universalApply');
5+
46
return function(methodName, args) {
57
return function(target) {
6-
return target[methodName].apply(target, args);
8+
return universalApply(target[methodName], target, args);
79
};
810
};
911

1012
});
11-
})(typeof define === 'function' && define.amd ? define : function(factory) { module.exports = factory(); });
13+
})(typeof define === 'function' && define.amd ? define : function(factory) { module.exports = factory(); });

‎lib/plugin/basePlugin.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,14 +14,15 @@
1414
define(function(require) {
1515

1616
var when, object, functional, pipeline, instantiate, createInvoker,
17-
whenAll, obj, pluginInstance, undef;
17+
whenAll, obj, pluginInstance, undef, universalApply;
1818

1919
when = require('when');
2020
object = require('../object');
2121
functional = require('../functional');
2222
pipeline = require('../pipeline');
2323
instantiate = require('../instantiate');
2424
createInvoker = require('../invoker');
25+
universalApply = require('../universalApply');
2526

2627
whenAll = when.all;
2728

@@ -194,7 +195,6 @@ define(function(require) {
194195
if (!proxy.clone) {
195196
throw new Error('No clone function found for ' + componentDef.id);
196197
}
197-
198198
return proxy.clone(options);
199199
});
200200
});
@@ -249,7 +249,7 @@ define(function(require) {
249249
// We'll either use the module directly, or we need
250250
// to instantiate/invoke it.
251251
return constructorName
252-
? module[constructorName].apply(module, args)
252+
? universalApply(constructorName, module, args)
253253
: typeof module == 'function'
254254
? instantiate(module, args, isConstructor)
255255
: Object.create(module);

‎lib/universalApply.js‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
(function(){
2+
'use strict';
3+
4+
(function(define){
5+
6+
function evaluates (statement) {
7+
try {
8+
/* jshint evil: true */
9+
eval(statement);
10+
/* jshint evil: false */
11+
return true;
12+
} catch (err) {
13+
return false;
14+
}
15+
}
16+
17+
// we have to know it synchronously, we are unable to load this module in asynchronous way
18+
// we cannot defer `define` and we cannot load module, that would not compile in browser
19+
// so we can't delegate this check to another module
20+
function isSpreadAvailable() {
21+
return evaluates('Math.max(...[ 5, 10 ])');
22+
}
23+
24+
var requires = [];
25+
if (typeof('process') !== 'undefined' && 'ES_VERSION' in process.env) {
26+
requires.push('./es'+process.env.ES_VERSION+'Apply');
27+
} else {
28+
if(isSpreadAvailable()) {
29+
requires.push('./es6Apply');
30+
} else {
31+
requires.push('./es5Apply');
32+
}
33+
}
34+
35+
define('universalApply', requires, function(apply){
36+
return apply;
37+
});
38+
})(
39+
typeof define === 'function'
40+
? define
41+
: function(name, requires, factory) {
42+
module.exports = factory.apply(null, requires.map(require));
43+
}
44+
);
45+
})();

‎test/buster.js‎

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,42 @@
1+
'use strict';
2+
13
require('gent/test-adapter/buster');
24

5+
function evaluates (statement) {
6+
try {
7+
/* jshint evil: true */
8+
eval(statement);
9+
/* jshint evil: false */
10+
return true;
11+
} catch (err) {
12+
return false;
13+
}
14+
}
15+
16+
function isClassAvailable() {
17+
return evaluates('class es6TestClass_ibyechBaloodren7 {}');
18+
}
19+
20+
function isSpreadAvailable() {
21+
return evaluates('parseInt(...["20", 10])');
22+
}
23+
24+
var tests = ['node/**/*-test.js'];
25+
26+
console.log('class operator %savailable', isClassAvailable() ? '' : 'not ');
27+
console.log('spread operator %savailable', isSpreadAvailable() ? '' : 'not ');
28+
29+
if(
30+
isClassAvailable()
31+
&& isSpreadAvailable()
32+
&& !('ES_VERSION' in process.env && parseFloat(process.env.ES_VERSION) < 6)
33+
) {
34+
tests.push('node-es6/**/*-test.js');
35+
}
36+
337
module.exports['node'] = {
438
environment: 'node',
5-
tests: ['node/**/*-test.js']
39+
tests: tests
640
// TODO: Why doesn't this work?
741
//, testHelpers:['gent/test-adapter/buster']
8-
};
42+
};

‎test/node-es6/lib/plugin/basePlugin-test.js‎

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
/* jshint esversion: 6 */
2+
(function(buster, context) {
3+
'use strict';
4+
5+
var assert, refute, fail, sentinel;
6+
7+
assert = buster.assert;
8+
refute = buster.refute;
9+
fail = buster.fail;
10+
11+
sentinel = {};
12+
13+
function createContext(spec) {
14+
return context.call(null, spec, null, { require: require });
15+
}
16+
17+
class es6Class
18+
{
19+
constructor () {
20+
this.constructorRan = true;
21+
this.args = Array.prototype.slice.call(arguments);
22+
}
23+
24+
someMethod() {
25+
26+
}
27+
}
28+
29+
buster.testCase('es6/lib/plugin/basePlugin', {
30+
'clone factory': {
31+
'should call constructor when cloning an object with a constructor': function() {
32+
class FabulousEs6 {
33+
constructor () {
34+
this.instanceProp = 'instanceProp';
35+
}
36+
}
37+
FabulousEs6.prototype.prototypeProp = 'prototypeProp';
38+
39+
return createContext({
40+
fab: {
41+
create: FabulousEs6
42+
},
43+
copy: {
44+
clone: { $ref: 'fab' }
45+
}
46+
}).then(
47+
function(context) {
48+
assert.defined(context.copy, 'copy is defined');
49+
assert.defined(context.copy.prototypeProp, 'copy.prototypeProp is defined');
50+
assert.defined(context.copy.instanceProp, 'copy.instanceProp is defined');
51+
refute.same(context.copy, context.fab);
52+
},
53+
fail
54+
);
55+
}
56+
},
57+
58+
'create factory': {
59+
'should call es6 constructor': function() {
60+
return createContext({
61+
test: {
62+
create: {
63+
module: es6Class,
64+
isConstructor: true
65+
}
66+
}
67+
}).then(
68+
function(context) {
69+
assert(context.test.constructorRan);
70+
},
71+
fail
72+
);
73+
},
74+
75+
'should call es6 constructor functions with args': function() {
76+
return createContext({
77+
test: {
78+
create: {
79+
module: es6Class,
80+
isConstructor: true,
81+
args: [1, 'foo', 1.7]
82+
}
83+
}
84+
}).then(
85+
function(context) {
86+
assert(context.test instanceof es6Class);
87+
assert.equals(context.test.args, [1, 'foo', 1.7]);
88+
},
89+
fail
90+
);
91+
},
92+
}
93+
});
94+
})(
95+
require('buster'),
96+
require('../../../../lib/context')
97+
);

0 commit comments

Comments
 (0)