New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support node-style callbacks #125

Merged
merged 3 commits into from Mar 24, 2013

Conversation

Projects
None yet
4 participants
@abesto
Contributor

abesto commented Mar 23, 2013

Hey! I started playing with Bacon after attending your workshop at mloc.js. I really like it so far. Here's one addition I'd find useful: support for Node.js-style callbacks. This makes bacon.js much easier to use on the server side.

I realize that this is a bit like the jQuery adapter so maybe it should be in a separate package, but the readme explicitly mentions that bacon.js works fine on the server; this looks like a basic requirement for node.js usage.

Hope you like the idea; please give feedback on the implementation if you'd prefer some other solution :)

Keep up the great work! 馃憤

@raimohanska

This comment has been minimized.

Contributor

raimohanska commented Mar 23, 2013

Looks good!

Could you give a couple of "real-life" examples of the usage?

@abesto

This comment has been minimized.

Contributor

abesto commented Mar 23, 2013

tl;dr Using the Redis client library

Of course. Here's one where I use it: https://github.com/abesto/shopping-list/blob/master/routes/tab.js

This is basically an end-point of a REST API. Walking through the create method line by line (assume redis is a redis client instance):

  1. increase the "next free id" of the resource, pass on the last value (this will be the id of the object we create now)
  2. generate the redis key to the newly created object (includes the id we just got back from redis)
  3. create the object data based on the id from redis and the request
  4. write the data to redis (Bacon.redis is a helper function to create partial functions useful in .flatMapLatest
  5. add the newly created object to the list of known objects
  6. respond to the client. note how Bacon.Observable.prototype.respond (another helper) is the only place where we have explicit error handling. bacon.js extended with fromNodeCallback hides explicit node-style error handling similarly to the Either monad in Haskell (if you're into that stuff. If not, then just: it's pretty cool)

Edit: oh, you mean in the readme?

@00dani

This comment has been minimized.

Contributor

00dani commented Mar 24, 2013

I'm not sure I like the name fromNodeCallback. True, Node callbacks do use the (err, res) format, but they're not the only ones who ever do that.

Perhaps call it fromErrCallback, to be more general?

@abesto

This comment has been minimized.

Contributor

abesto commented Mar 24, 2013

@00Davo I'd prefer to call it fromNodeStyleCallback, but maybe that's a bit too long. You're right in that Node.js is not the only place where we see callbacks like this, but I've often seen it referred to as "node-style callback". Even if it wasn't introduced by node, it's certanly become a standard due to the popularity of node.

To me, the fromErrCallback name means that the method takes several callbacks, one of which is an error callback, and we're creating a stream from that.

Bottom line: I still prefer fromNodeCallback, but I won't take it personally if we go with another name.

@raimohanska

This comment has been minimized.

Contributor

raimohanska commented Mar 24, 2013

I guess "node-style callbacks" are well-known enough to warrant for the name fromNodeCallback.

The example code is pretty cool and I also like the fact that you only need to deal with errors in a single spot. And yes, I'm a kind-of-a Haskellist too, having written reactive-bacon before starting to work on bacon.js.

I hope you don't mind if I do some code review for you. The code looks very good, but some simplifications could probably be made. Looking at the update function, you could use Bacon.constant to create a constant Property, instead of Bacon.once. So, on line 46, you could write

var key = Bacon.constant(tabKey(req.params.id))

Now because key is really just a constant value, you could still simplyfy to

var key = tabKey(req.params.id)
var json = Bacon.redis('get')(key)

Also, on the last line, you have

Bacon.combineWith([obj, update], fst).respond(res);

While you could have instead just

update.respond(res);

Right?

Oh, and you can use plain flatMap instead of flatMapLatest in most cases. Probably all cases in this file.

The bottom line is, I'm pretty convinced we should include fromNodeCallback. Could you come up with a simple example that could be included in the README?

@abesto

This comment has been minimized.

Contributor

abesto commented Mar 24, 2013

Wow, thanks for the suggestions :) You're right on all counts. I guess bacon.js is such a shiny hammer, even constants look like nails.

I've added two simple examples (with and without error), using the example for fromCallback to keep things simple. How do you like it?

@raimohanska

This comment has been minimized.

Contributor

raimohanska commented Mar 24, 2013

Yeah, it's quite a hammer :) I recently bought a Fein Multimaster and been trying to fit it to all kinds of jobs I've done just fine without the new tool.

Actually I was thinking of more realistic examples, involving some real Node.js APIs. That would also serve to demonstrate the partial application capabilities.

@abesto

This comment has been minimized.

Contributor

abesto commented Mar 24, 2013

How about this? Uses a real node.js api, shows partial application and error handling, and still quite simple to understand.

@raimohanska

This comment has been minimized.

Contributor

raimohanska commented Mar 24, 2013

That's it! Merging.

A blog post on server-side Bacon would be nice too. It would be nice to demonstrate how you can avoid the infamous callback hell, while taking care of error handling in a reliable and simple way. The problem is that I haven't done any serious node dev for some time and feel a bit lacking in credibility there, like "if I did node dev, this is how I'd do it".. Well, I should probably do my next server in node+bacon.

raimohanska added a commit that referenced this pull request Mar 24, 2013

Merge pull request #125 from abesto/master
Support node-style callbacks

@raimohanska raimohanska merged commit 23b44d8 into baconjs:master Mar 24, 2013

1 check passed

default The Travis build passed
Details
@abesto

This comment has been minimized.

Contributor

abesto commented Mar 24, 2013

I'm planning to rewrite the whole shoppinglist thing with bacon. I'll probably write a post about what I learned in the process, thanks for the idea! (and the merge :))

@00dani

This comment has been minimized.

Contributor

00dani commented Mar 25, 2013

I'm happy with the name fromNodeCallback given the understanding that it's short for Node-style callback. ^_^

I'd also very much like to see an example of Bacon.js being applied on server-side with Node, so that'd be good.

@kristianmandrup

This comment has been minimized.

kristianmandrup commented Nov 5, 2013

Hi,

I'm trying to use BaconJS for the first time for the following use case:

for (model in models) {
  store.subscribe(model, function(err, data) {
  });
}

Looking at http://abesto.net/bacon-js-on-the-server/ and http://howtonode.org/node-redis-fun

r.incr( 'nextid' , function( err, id ) {

id = Bacon.fromNodeCallback(redis.incr, 'tab').toProperty()

So the above would be:

var subscribed = {};
for (model in models) {
  subscribed[model] = Bacon.fromNodeCallback(store.subscribe, model).toProperty();
}

do some flatMap on properties?

@raimohanska

This comment has been minimized.

Contributor

raimohanska commented Nov 7, 2013

Your code above creates a hash of Properties. What are you trying to achieve? Also, Issues are not really a support forum. I suggest you use Stack Overflow, which is better for sharing problems/solutions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment