diff --git a/.travis.yml b/.travis.yml index 9e4a53e..3cdb7a5 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,9 +1,10 @@ language: node_js node_js: - - 0.8 - - 0.10 - - 0.11 + - 6 + - 8 + - 9 + - 10 branches: only: @@ -11,5 +12,3 @@ branches: matrix: fast_finish: true - allow_failures: - - node_js: 0.11 diff --git a/README.md b/README.md index 98815f3..2c15050 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,7 @@ Promisify Node -------------- -**Stable: 0.1.5** +**Stable: 0.5.0** [![Build Status](https://travis-ci.org/nodegit/promisify-node.png?branch=master)](https://travis-ci.org/nodegit/promisify-node) @@ -70,6 +70,32 @@ myObj.myMethod({ msg: "Failure!" }, null).then(null, function(err) { }); ``` +Wrap without mutating the original: +```javascript +var promisify = require("promisify-node"); + +var myObj = { + myMethod: function(a, b, cb) { + cb(a, b); + } +}; + +// Store the original method to check later +var originalMethod = myObj.myMethod; + +// Now store the result, since the 'true' value means it won't mutate 'myObj'. +var promisifiedObj = promisify(myObj, undefined, true); + +// Intentionally cause a failure by passing an object and inspect the message. +promisifiedObj.myMethod({ msg: "Failure!" }, null).then(null, function(err) { + console.log(err.msg); +}); + +// The original method is still intact +assert(myObj.myMethod === originalMethod); +assert(promisifiedObj.myMethod !== myObj.myMethod); +``` + ### Tests ### Run the tests after installing dependencies with: diff --git a/index.js b/index.js index 9db5830..3b2161c 100644 --- a/index.js +++ b/index.js @@ -1,5 +1,7 @@ const Promise = require("nodegit-promise"); const args = require("./utils/args"); +const cloneFunction = require("./utils/cloneFunction"); +const objectAssign = require("object-assign"); // Unfortunately this list is not exhaustive, so if you find that a method does // not use a "standard"-ish name, you'll have to extend this list. @@ -12,16 +14,32 @@ var callbacks = ["cb", "callback", "callback_", "done"]; * @param {*} exports - Should be a function or an object, identity other. * @param {Function} test - Optional function to identify async methods. * @param {String} parentKeyName - Tracks the keyName in a digestable format. + * @param {Boolean} noMutate - if set to true then all reference properties are + * cloned to avoid mutating the original object. * @returns {*} exports - Identity. */ -function processExports(exports, test, cached, parentKeyName) { - // Return early if this object has already been processed. - if (cached.indexOf(exports) > -1) { +function processExports(exports, test, cached, parentKeyName, noMutate) { + if(!exports) { return exports; } - // Record this object in the cache. - cached.push(exports); + if(noMutate || typeof exports === "function") { + // When not mutating we have to cache the original and the wrapped clone. + var cacheResult = cached.filter(function(c) { return c.original === exports; }); + if(cacheResult.length) { + return cacheResult[0].wrapped; + } + } else { + // Return early if this object has already been processed. + if (cached.indexOf(exports) > -1) { + return exports; + } + } + + // Record this object in the cache, if it is not a function. + if(typeof exports != "function") { + cached.push(exports); + } // Pass through if not an object or function. if (typeof exports != "object" && typeof exports != "function") { @@ -29,63 +47,97 @@ function processExports(exports, test, cached, parentKeyName) { } var name = exports.name + "#"; + var target; // If a function, simply return it wrapped. if (typeof exports === "function") { + var wrapped = exports; + var isAsyncFunction = false; + + // Check the callback either passes the test function, or accepts a callback. + if ((test && test(exports, exports.name, parentKeyName)) + // If the callback name exists as the last argument, consider it an + // asynchronous function. Brittle? Fragile? Effective. + || (callbacks.indexOf(args(exports).slice(-1)[0]) > -1)) { + // Assign the new function in place. + wrapped = Promise.denodeify(exports); + + isAsyncFunction = true; + } else if(noMutate) { + // If not mutating, then we need to clone the function, even though it isn't async. + wrapped = cloneFunction(exports); + } + + // Set which object we'll mutate based upon the noMutate flag. + target = noMutate ? wrapped : exports; + + // Here we can push our cloned/wrapped function and original onto cache. + cached.push({ + original: exports, + wrapped: wrapped + }); + // Find properties added to functions. for (var keyName in exports) { - exports[keyName] = processExports(exports[keyName], test, cached, name); + target[keyName] = processExports(exports[keyName], test, cached, name, noMutate); } // Find methods on the prototype, if there are any. - if (Object.keys(exports.prototype).length) { - processExports(exports.prototype, test, cached, name); - } - - // If a filter function exists, use this to determine if the function - // is asynchronous. - if (test) { - // Pass the function itself, its keyName, and the parent keyName. - if (!test(exports, exports.name || parentKeyName, parentKeyName)) { - return exports; - } + if (exports.prototype && Object.keys(exports.prototype).length) { + // Attach the augmented prototype. + wrapped.prototype = processExports(exports.prototype, test, cached, name, noMutate); } - // If the callback name exists as the last argument, consider it an - // asynchronous function. Brittle? Fragile? Effective. - else if (callbacks.indexOf(args(exports).slice(-1)[0]) === -1) { - return exports; + // Ensure attached properties to the previous function are accessible. + // Only do this if it's an async (wrapped) function, else we're setting + // __proto__ to itself, which isn't allowed. + if(isAsyncFunction) { + wrapped.__proto__ = exports; } - // Assign the new function in place. - var wrapped = Promise.denodeify(exports); - - // Attach the augmented prototype. - wrapped.prototype = exports.prototype; + return wrapped; + } - // Ensure attached properties to the previous function are accessible. - wrapped.__proto__ = exports; + // Make a shallow clone if we're not mutating and set it as the target, else just use exports + target = noMutate ? objectAssign({}, exports) : exports; - return wrapped; + // We have our shallow cloned object, so put it (and the original) in the cache + if(noMutate) { + cached.push({ + original: exports, + wrapped: target + }); } - Object.keys(exports).map(function(keyName) { + Object.keys(target).map(function(keyName) { // Convert to values. - return [keyName, exports[keyName]]; - }).forEach(function(keyVal) { + return [keyName, target[keyName]]; + }).filter(function(keyVal) { var keyName = keyVal[0]; var value = keyVal[1]; // If an object is encountered, recursively traverse. if (typeof value === "object") { - processExports(exports, test, cached, keyName + "."); + processExports(value, test, cached, keyName + ".", noMutate); + } else if (typeof value === "function") { + // If a filter function exists, use this to determine if the function + // is asynchronous. + if (test) { + // Pass the function itself, its keyName, and the parent keyName. + return test(value, keyName, parentKeyName); + } + + return true; } + }).forEach(function(keyVal) { + var keyName = keyVal[0]; + var func = keyVal[1]; // Wrap this function and reassign. - exports[keyName] = processExports(value, test, cached, parentKeyName); + target[keyName] = processExports(func, test, cached, parentKeyName, noMutate); }); - return exports; + return target; } /** @@ -93,20 +145,23 @@ function processExports(exports, test, cached, parentKeyName) { * * @param {*} name - Can be a module name, object, or function. * @param {Function} test - Optional function to identify async methods. + * @param {Boolean} noMutate - Optional set to true to avoid mutating the target. * @returns {*} exports - The resolved value from require or passed in value. */ -module.exports = function(name, test) { +module.exports = function(name, test, noMutate) { var exports = name; // If the name argument is a String, will need to resovle using the built in // Node require function. if (typeof name === "string") { exports = require(name); + // Unless explicitly overridden, don't mutate when requiring modules. + noMutate = !(noMutate === false); } // Iterate over all properties and find asynchronous functions to convert to // promises. - return processExports(exports, test, []); + return processExports(exports, test, [], undefined, noMutate); }; // Export callbacks to the module. diff --git a/package.json b/package.json index 39798fc..fc6cc57 100644 --- a/package.json +++ b/package.json @@ -1,14 +1,19 @@ { "name": "promisify-node", - "version": "0.1.5", + "version": "0.5.0", "description": "Wrap Node-callback functions to return Promises.", "main": "index.js", "dependencies": { - "nodegit-promise": "~1.0.0" + "nodegit-promise": "^4.0.0", + "object-assign": "^4.1.1" }, "devDependencies": { - "mocha": "~1.18.2", - "istanbul": "~0.2.7" + "mocha": "^5.2.0", + "istanbul": "^0.4.5" + }, + "repository": { + "type": "git", + "url": "https://github.com/nodegit/promisify-node.git" }, "scripts": { "test": "istanbul cover _mocha -- test" diff --git a/test/examples/fn-export.js b/test/examples/fn-export.js new file mode 100644 index 0000000..8f7d76d --- /dev/null +++ b/test/examples/fn-export.js @@ -0,0 +1,2 @@ +module.exports = function(d, cb) { setTimeout(cb, d); }; +module.exports.x = function(d, cb) { setTimeout(cb, d); }; diff --git a/test/examples/proto-export.js b/test/examples/proto-export.js new file mode 100644 index 0000000..e32c9ef --- /dev/null +++ b/test/examples/proto-export.js @@ -0,0 +1,3 @@ +function A(d) { this.d = d; } +A.prototype.a = function(cb) { setTimeout(cb, this.d); }; +module.exports = A; diff --git a/test/index.js b/test/index.js index 25157dc..a8e505b 100644 --- a/test/index.js +++ b/test/index.js @@ -1,7 +1,13 @@ var promisify = require("../"); -var assert = require("assert"); +var assert = require("assert"), + fsOriginal = require('fs'); describe("Promisify", function() { + function isPromisified(fn, ctx) { + var result = fn && fn.apply(ctx, Array.prototype.slice.call(arguments, 2)); + return result && (typeof result.then === 'function'); + } + it("can convert a basic async function", function() { function test(cb) { cb(null, true); @@ -30,6 +36,41 @@ describe("Promisify", function() { return fs.readFile(__dirname + "/../LICENSE"); }); + + it("doesn't mutate objects for other consumers", function() { + var fsp = promisify("fs"); + var fs2 = require("fs"); + + assert(fsOriginal.readFile !== fsp.readFile, "pre-required mutated"); + assert(fsOriginal.readFile === fs2.readFile, "post-required mutated"); + assert(fsp.readFile !== fs2.readFile, "post-required mutated"); + }); + + it("doesn't mutate functions for other consumers", function() { + var fn = require(__dirname + "/examples/fn-export.js"); + var fnx = fn.x; + var fnp = promisify(__dirname + "/examples/fn-export.js"); + var fn2 = require(__dirname + "/examples/fn-export.js"); + + assert(fn.x !== fnp, "pre-required mutated"); + assert(fn2.x !== fnp, "post-required mutated"); + assert(fn.x === fnx, "function property mutated"); + assert(fnp.x !== fn, "function property not replaced"); + }); + + it("doesn't mutate prototypes for other consumers", function() { + var A = require(__dirname + "/examples/proto-export.js"); + var a = new A(5); + var Ap = promisify(__dirname + "/examples/proto-export.js"); + var ap = new Ap(5); + var A2 = require(__dirname + "/examples/proto-export.js"); + var a2 = new A2(5); + + assert(isPromisified(ap.a, ap), "prototype method not promisified"); + assert(a.a !== ap.a, "pre-required mutated"); + assert(a2.a !== ap.a, "post-required mutated"); + assert(a2.a === a.a, "post-required mutated"); + }); }); describe("asynchronous method inference", function() { @@ -57,6 +98,14 @@ describe("Promisify", function() { return obj.a(); }); + it("can infer callback-accepting arrow functions", function() { + var obj = promisify({ + a: (cb) => { later(cb); } + }); + + return obj.a(); + }); + it("can infer callback-accepting functions by argument list", function() { var obj = promisify({ a: function(callback) { later(callback); } @@ -98,11 +147,83 @@ describe("Promisify", function() { a: function a() { arguments[0](); } }; - promisify(Test, function(func, keyName) { - return keyName === "a"; + promisify(Test, function(func, keyName, parentKeyName) { + return func.name === "a"; }); return new Test().a(); }); + + it("can deal with the same async function being present in an object more than once", function() { + var a = { + a: function(d, cb) { + cb && cb(null, d); + } + }; + + a.b = a.a; + + a = promisify(a); + + assert(a.a(5) !== undefined, "function not wrapped"); + assert(typeof a.a(5).then === "function", "function not wrapped"); + assert(a.b(5) !== undefined, "duplicate function not wrapped"); + assert(typeof a.b(5).then === "function", "duplicate function not wrapped"); + }); + + it("can deal with cyclical function properties", function() { + var a = function(d, cb) { + cb && cb(null, d); + }; + + a.a = a; + + a = promisify(a); + + assert(a(5) !== undefined, "function not wrapped"); + assert(typeof a(5).then === "function", "function not wrapped"); + assert(a.a(5) !== undefined, "function property not wrapped"); + assert(typeof a.a(5).then === "function", "function property not wrapped"); + }); + }); + + + describe("no mutate", function() { + it("can promisify an object without mutating it", function() { + var a = { + a: function(cb) { cb(); } + }; + + var b = promisify(a, undefined, true); + + assert(isPromisified(b.a, b), "method not promisified"); + assert(a.a !== b.a, "object mutated"); + }); + + it("can promisify a function's properties without mutating it", function() { + var a = function(cb){ cb(null, 1); }; + a.a = function(cb) { cb(); }; + + var b = promisify(a, undefined, true); + + assert(isPromisified(b), "method not promisified"); + assert(isPromisified(b.a, b), "method property not promisified"); + assert(a.a !== b, "method property mutated"); + assert(a.a !== b.a, "method property mutated"); + }); + + it("can promisify a constructor without mutating it", function() { + var A = function(){ }; + A.a = function(cb) { cb(); }; + A.prototype.a = function(cb) { cb(null, 2); }; + + var B = promisify(A, undefined, true); + var b = new B(); + + assert(isPromisified(B.a, B), "method property not promisified"); + assert(isPromisified(b.a, b), "prototype method not promisified"); + assert(A.a !== B.a, "method property mutated"); + assert(A.prototype.a !== b.a, "prototype mutated"); + }); }); }); diff --git a/utils/args.js b/utils/args.js index c18275c..970ac67 100644 --- a/utils/args.js +++ b/utils/args.js @@ -6,8 +6,8 @@ */ module.exports = function(func) { // First match everything inside the function argument parens. - var args = func.toString().match(/function\s.*?\(([^)]*)\)/)[1]; - + var args = func.toString().match(/([^(])*\(([^)]*)\)/)[2]; + // Split the arguments string into an array comma delimited. return args.split(", ").map(function(arg) { // Ensure no inline comments are parsed and trim the whitespace. diff --git a/utils/cloneFunction.js b/utils/cloneFunction.js new file mode 100644 index 0000000..d93a1a2 --- /dev/null +++ b/utils/cloneFunction.js @@ -0,0 +1,17 @@ +/** + * Clones a function, including copying any properties of the function to the clone. + * + * @param {Function} func - The function to clone. + */ +module.exports = function cloneFn(func) { + var temp; + // Check for the memoized value on the function in-case we get called to wrap the same function + // (or already wrapped function) again. + return func.__cloneFn || (temp = function() { + return func.apply(this, arguments); + }) && + // Assign __proto__ as a quick way to copy function properties. + (temp.__proto__ = func) && + // Lastly, set a cache var on the original and clone, and return the result. + (func.__cloneFn = temp.__cloneFn = temp); +};