Skip to content

Commit

Permalink
Don't invoke getters while promisifying
Browse files Browse the repository at this point in the history
  • Loading branch information
@petkaantonov
petkaantonov committed Nov 29, 2013
1 parent ee872e9 commit beb26fb
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 40 deletions.
6 changes: 4 additions & 2 deletions src/es5.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ if (isES5) {
defineProperty: Object.defineProperty,
keys: Object.keys,
getPrototypeOf: Object.getPrototypeOf,
isArray: Array.isArray
isArray: Array.isArray,
isES5: isES5
};
}

Expand Down Expand Up @@ -60,6 +61,7 @@ else {
keys: ObjectKeys,
defineProperty: ObjectDefineProperty,
freeze: ObjectFreeze,
getPrototypeOf: ObjectGetPrototypeOf
getPrototypeOf: ObjectGetPrototypeOf,
isES5: isES5
};
}
111 changes: 76 additions & 35 deletions src/promisify.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
module.exports = function( Promise ) {
var THIS = {};
var util = require( "./util.js");
var es5 = require("./es5.js");
var errors = require( "./errors.js" );
var nodebackForResolver = require( "./promise_resolver.js" )
._nodebackForResolver;
Expand All @@ -13,6 +14,67 @@ var notEnumerableProp = util.notEnumerableProp;
var deprecated = util.deprecated;
var ASSERT = require( "./assert.js" );


var roriginal = new RegExp( BEFORE_PROMISIFIED_SUFFIX + "$" );
var hasProp = {}.hasOwnProperty;
function isPromisified( fn ) {
return fn.__isPromisified__ === true;
}
var inheritedMethods = (function() {
if (es5.isES5) {
//Guaranteed since ES-5
var create = Object.create;
var getOwnPropertyDescriptor = Object.getOwnPropertyDescriptor;
return function(cur) {
var original = cur;
var ret = [];
var visitedKeys = create(null);
//Need to reinvent for-in because getOwnPropertyDescriptor
//only works if the property is exactly on the object
while (cur !== null) {
var keys = es5.keys(cur);
for( var i = 0, len = keys.length; i < len; ++i ) {
var key = keys[i];
//For shadowing
if (visitedKeys[key] ||
roriginal.test(key) ||
hasProp.call(original, key + BEFORE_PROMISIFIED_SUFFIX)
) {
continue;
}
visitedKeys[key] = true;
var desc = getOwnPropertyDescriptor(cur, key);
if (desc != null &&
typeof desc.value === "function" &&
!isPromisified(desc.value)) {
ret.push(key, desc.value);
}
}
cur = es5.getPrototypeOf(cur);
}
return ret;
};
}
else {
return function(obj) {
var ret = [];
/*jshint forin:false */
for (var key in obj) {
if (roriginal.test(key) ||
hasProp.call(obj, key + BEFORE_PROMISIFIED_SUFFIX)) {
continue;
}
var fn = obj[key];
if (typeof fn === "function" &&
!isPromisified(fn)) {
ret.push(key, fn);
}
}
return ret;
};
}
})();

Promise.prototype.error = function Promise$_error( fn ) {
return this.caught( RejectionError, fn );
};
Expand Down Expand Up @@ -113,47 +175,26 @@ var makeNodePromisified = canEvaluate
: makeNodePromisifiedClosure;

function f(){}
function isPromisified( fn ) {
return fn.__isPromisified__ === true;
}
var hasProp = {}.hasOwnProperty;
var roriginal = new RegExp( BEFORE_PROMISIFIED_SUFFIX + "$" );
function _promisify( callback, receiver, isAll ) {
if( isAll ) {
var changed = 0;
var o = {};
for( var key in callback ) {
if( !roriginal.test( key ) &&
!hasProp.call( callback,
( key + BEFORE_PROMISIFIED_SUFFIX ) ) &&
typeof callback[ key ] === "function" ) {
var fn = callback[key];
if( !isPromisified( fn ) ) {
changed++;
var originalKey = key + BEFORE_PROMISIFIED_SUFFIX;
var promisifiedKey = key + AFTER_PROMISIFIED_SUFFIX;
notEnumerableProp( callback, originalKey, fn );
o[ promisifiedKey ] =
makeNodePromisified( originalKey, THIS, key );
}
}
}
if( changed > 0 ) {
for( var key in o ) {
if( hasProp.call( o, key ) ) {
callback[key] = o[key];
}
}
//Right now the above loop will easily turn the
//object into hash table in V8
//but this will turn it back. Yes I am ashamed.
f.prototype = callback;
var methods = inheritedMethods(callback);
for (var i = 0, len = methods.length; i < len; i+= 2) {
var key = methods[i];
var fn = methods[i+1];
var originalKey = key + BEFORE_PROMISIFIED_SUFFIX;
var promisifiedKey = key + AFTER_PROMISIFIED_SUFFIX;
notEnumerableProp(callback, originalKey, fn);
callback[promisifiedKey] =
makeNodePromisified(originalKey, THIS, key);
}

//Right now the above loop will easily turn the
//object into hash table in V8
//but this will turn it back. Yes I am ashamed.
if (methods.length > 16) f.prototype = callback;
return callback;
}
else {
return makeNodePromisified( callback, receiver, void 0 );
return makeNodePromisified(callback, receiver, void 0);
}
}

Expand Down
33 changes: 30 additions & 3 deletions test/mocha/promisify.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -395,7 +395,9 @@ describe("promisify on objects", function(){
});


describe( "Promisify from prototype to object", function(){
describe( "Promisify from prototype to object", function() {
var getterCalled = 0;

function makeClass() {
var Test = (function() {

Expand All @@ -408,6 +410,29 @@ describe( "Promisify from prototype to object", function(){

};

if (Object.defineProperty) {
Object.defineProperty(method, "thrower", {
enumerable: true,
configurable: true,
get: function() {
throw new Error("getter called");
},
set: function() {
throw new Error("setter called");
}
});
Object.defineProperty(method, "counter", {
enumerable: true,
configurable: true,
get: function() {
getterCalled++;
},
set: function() {
throw new Error("setter called");
}
});
}

return Test;})();

return Test;
Expand All @@ -424,7 +449,7 @@ describe( "Promisify from prototype to object", function(){
assert( typeof a.testAsync === "function" );
assert( a.hasOwnProperty("testAsync"));
assert.deepEqual( Object.getOwnPropertyNames(Test.prototype).sort(), origKeys );

assert(getterCalled === 0);
done();
});

Expand All @@ -439,7 +464,7 @@ describe( "Promisify from prototype to object", function(){
assert( typeof a.testAsync === "function" );
assert.deepEqual( Object.getOwnPropertyNames(Test.prototype.test).sort(), origKeys );
assert( Promise.promisify( a.test ) !== a.testAsync );

assert(getterCalled === 0);
done();
});

Expand All @@ -453,6 +478,7 @@ describe( "Promisify from prototype to object", function(){
Promise.promisifyAll( instance );
assert.deepEqual( origKeys, Object.getOwnPropertyNames(Test.prototype).sort() );
assert.notDeepEqual( origInstanceKeys, Object.getOwnPropertyNames(instance).sort() );
assert(getterCalled === 0);
done();
});

Expand All @@ -468,6 +494,7 @@ describe( "Promisify from prototype to object", function(){

assert.deepEqual( Object.getOwnPropertyNames(Test.prototype).sort(), origKeys );
assert( instance.test__beforePromisified__ === instance.test );
assert(getterCalled === 0);
done();
});

Expand Down

0 comments on commit beb26fb

Please sign in to comment.