Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

adding standard node callback support #2

Open
wants to merge 1 commit into from

2 participants

@thesmart

adding method "thenNode" which takes a standard node callback (e.g. function(err, result))

Take a look and let me know what you think.

@thesmart thesmart updating test to work in Node as of 0.8.15
adding method "thenNode" which takes a standard node callback (e.g. function(err, result))
275381a
@MaxMotovilov

Not sure I like opt_scope -- none of the other methods in the API has the extra auto-binding parameter and, arguably, p.thenNode( mycb.bind( myobj ) ) is more transparent to a reader than p.thenNode( mycb, myobj ) even if slightly longer. Counterarguments?

Also, by design of the API, promise.when(p,...) is a more universal form of p.then(...) (i.e. it works in an obvious way with non-promise arguments). In order to extend this to the other callback type, we'd also have to add a whenNode() API.

Finally, your proposed API does not support the third argument of then(), the progressCallback.

I suspect that it would be easier to overcome these objections if, instead of creating an entirely different method for node-style callbacks, we have somehow determined that an argument passed into then() (or when()) is a node-style callback and thus should take place of both callback and errback. So, how about this:

function _Pair( cb, eb ) {
  this.callback = cb;
  this.errback = eb;
}

exports.nodeCallback = function( cb, opt_scope ) {
   return new _Pair(
      cb.bind( opt_scope || null, null ),
      cb.bind( opt_scope || null )
   );
}

and then have then() check for instanceof _Pair and branch accordingly?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Dec 12, 2012
  1. @thesmart

    updating test to work in Node as of 0.8.15

    thesmart authored
    adding method "thenNode" which takes a standard node callback (e.g. function(err, result))
This page is out of date. Refresh to see the latest.
Showing with 34 additions and 2 deletions.
  1. +14 −0 promise.js
  2. +20 −2 test-promise.js
View
14 promise.js
@@ -123,6 +123,20 @@ Promise.prototype.wait = function(){
return exports.wait(this);
};
+/**
+ * When promise is resolved or rejected, call a single callback in the style of Node callbacks
+ *
+ * @param {Function} nodeCallback The callback function (e.g. function(err, result))
+ * @param {*=} opt_scope Optional scope to set in the callback (default: null)
+ * @return {*}
+ */
+Promise.prototype.thenNode = function(nodeCallback, opt_scope) {
+ return this.then(
+ nodeCallback.bind(opt_scope ? opt_scope : null, null), // no err
+ nodeCallback.bind(opt_scope ? opt_scope : null) // err is first param
+ );
+};
+
Deferred.prototype = Promise.prototype;
// A deferred provides an API for creating and resolving a promise.
exports.Promise = exports.Deferred = exports.defer = defer;
View
22 test-promise.js
@@ -2,11 +2,29 @@ sys = require("sys");
var fs = require('./fs-promise');
// open a file and read it
-fs.open("fs-promise.js", process.O_RDONLY).then(function(fd){
+fs.open("fs-promise.js", 'r').then(function(fd){
return fs.read(fd, 4096);
}).then(function(args){
sys.puts(args[0]);
});
// does the same thing
-fs.readFile("fs-promise.js").addCallback(sys.puts);
+
+// does the same thing
+fs.readFile("fs-promise.js").thenNode(function(err, result) {
+ if (err) {
+ console.error(err);
+ } else {
+ console.info('thenNode result success');
+ }
+});
+
+// forced error
+fs.readFile("foobar.js").thenNode(function(err, result) {
+ if (err) {
+ console.info('thenNode err success');
+ } else {
+ console.error('err should be passed');
+ }
+});
Something went wrong with that request. Please try again.