return thunk for generators

[tj]

so we can do:

  var res = yield request
    .get('http://google.com')
    .set('Accept', 'application/json')
    .query({ search: 'sloths' })
    .end();

  console.log(res.status);

[jkroso]

not that it makes much difference when using them with generators but it might be worth a look at making the request instances also promise instances.

  var res = yield request
    .get('http://google.com')
    .set('Accept', 'application/json')
    .query({ search: 'sloths' })

  console.log(res.status);

promises are just a little nicer for other abstractions which are usable now. whatever though thunks do the job in any case.

[shesek]

Looks like its for superagent to work with co? (which looks awesome, @visionmedia!).

If co were to support promises (which is basically just [1] replacing ret.value(next) with (ret.value.then||ret.value)(next)) its just a matter of aliasing the end method with then... but it looks like co is pretty opinionated about not using promises.

Edit: [1] actually, its not quite right - promises don't follow the (err, res) callback convention and use two callbacks instead. It would also be wrong to alias end as then for that reason.

[tj]

yeah I'm not a fan of promises, not going to go that route, this is a nice one line change

[tj]

plus .query() doesn't actually initiate the request so that would be sketchy

[jkroso]

no the generator thing would call then and that would start the request (or pipe, lol, if it preferred). if you like to minimize implicit stuff you can add .then() with no args after .query and nothing changes. I would love to hear a proper explanation of why you dislike promises, but fair enough if you just wan't to get on with it ;)

[tj]

I just don't really like that amount of "cruft" as a base, especially when a perfectly good alternative is to just return a function: https://github.com/visionmedia/co-exec/blob/master/index.js#L12

no fighting over which implementation to use or duplicate implementations etc. Even if there was one particular method to use I dont think .then() is a great one personally but it is what it is!

[jkroso]

no fighting over which implementation to use or duplicate implementations etc

good point people still disagree a lot on how to implement them.

I really wish this was the standard https://gist.github.com/jkroso/5689012#file-promise-js but instead most people use stuff like q which makes me cringe :(. I guess without agreement on what a promise should be thunks are a pretty good alternative

[tj]

yeah seems like too much boilerplate for something effectively every library would require, at least with thunks there's only one way you can do it haha

[spikebrehm]

Curious if @visionmedia has any renewed interest in supporting promises now that it's a sure thing for ES6?

[tj]

not really, impartial once they're actually implemented in browsers, but IMO it's a really silly thing to add to the language, easier to just return a function

[jkroso]

superagent requests pretty much already are promises so I think they may aswell become yieldable ones. The new ES6 promises won't help though since they aren't sub-classable :(.

BTW: I rewrote superagent to use promises and here is an example of which demonstrates why some people go for promises over thunks. Promises are more a type than mechanism. If you could distinguish thunks from other functions cleanly then I'd prefer them too.

[jonathanong]

can we get this in? are there any blockers?

[gjohnson]

Note to self: https://github.com/visionmedia/superagent/blob/master/lib/node/index.js#L428

[gjohnson]

@jonathanong started messing with this, it's my first go around with generators so still figuring out some things. Feel free to fix this branch: https://github.com/visionmedia/superagent/tree/thunkify

[hallas]

@gjohnson lets add some tests for the thunk branch

[arcmode]

I use this

var co = require('co');
var request = require('superagent');

co(function *(){
  var a = yield request.get('http://google.com').thunkify();
  var b = yield request.get('http://yahoo.com').thunkify();
  var c = yield request.get('http://cloudup.com').thunkify();
  console.log(a.statusCode);
  console.log(b.statusCode);
  console.log(c.statusCode);
})()

co(function *(){
  var a = request.get('http://google.com').thunkify();
  var b = request.get('http://yahoo.com').thunkify();
  var c = request.get('http://cloudup.com').thunkify();
  var res = yield [a, b, c];
  console.log(res);
})()

[ianstormtaylor]

+1 for thunks

[matthewmueller]

+1 for @visionmedia's original API. Not a huge fan of .thunkify(), you can just do:

var thunkify = require('node-thunkify');
request.get = thunkify(request.get);
yield request.get('http://google.com')

If you need it on the shorthand.

[kof]

1. Promises have a way bigger value proposition than thunks.
2. Performance overhead can be ignored in superagent use case.
3. We don't need to introduce a dependency to some particular implementation. We can let user pass the implementation once or use the global one.
4. We can still have the .end method which returns a promise, which is more explicit and backwards compatible.

[cncolder]

i have a new idea:

we can extend the prototype of superagent.Request as thenable.

Request.prototype.then = function(fulfilled, rejected) {
  return new Promise(function(resolve, reject) {
    this.end(function(err, res) {
      if (err) reject(err);
      else resolve(res);
    });
  }.bind(this)).then(fulfilled, rejected);
};

now we can get promise after then or yield.

request.get('localhost:3000')
.then(function(res) {
  assert(res.ok);
  return res;
})
.then(function(res) {
  assert.equal(res.text, 'Hello World');
});

co(function * () {
  var res = yield request.get('localhost:3000');
  assert(res.ok);
})();

[defunctzombie]

Dislike promises. Right now they are not widely supported enough and I don't see a reason to use them for this since a request is a one shot deal.

[nmn]

This has been a long and unproductive argument, which is mostly a thunks vs promises debate.

Some things to consider now:

1. Co now prefers promises over thunks
2. superagent-promise already provides a simple wrapper for superagent to return promises
3. Choosing one does not mean the other is bad.

Possible resolutions:

1. support thunks in the main lib, and support the development of superagent-promise to support both approaches.
2. Leave the main lib alone, and create a separate repo to wrap superagent and make it support thunks, and support superagent-promise
3. Add Support for both thunks and promises into the main lib, possibly with an extra method (toThunk/toPromise?)
4. Choose one approach as the only approach and keep arguing about it for eternity

[defunctzombie]

I don't care for thunks or promises right now in this lib. This is a one shot callback lib. Once people rally around the way async will be done going forward, then might revisit. My comment is not meant to be about one being better than the other, only about having more dependencies for this lib right now for no reason. Functions are simple and easy to understand.