Skip to content
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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How would I send a callback styled response to a client side event emitter #5

Open
knowthen opened this issue Feb 22, 2015 · 14 comments
Milestone

Comments

@knowthen
Copy link

I just noticed koa.io today, pretty slick!
I think I will do a screencast on this soon.

I was looking for a way to do a callback in response to an event.
With normal socket.io it would be done like below 'cb'

socket.on('someEvent', function (data, cb) {
  // do something with data...
  cb(null, 'client done processing data...');
});

I looked through the code and it doesn't seem to be possible, unless I'm missing something.

It would be pretty cool to be able to do something like

app.io.route('someEvent', function *(next, data){
  // do something with data...
  this.body = 'client done processing data...';
});

and have the message 'client done processing data...' get sent back as the callback to the clientside emitter.

Your thoughts?

Thanks!

@neverfox
Copy link

I'm wondering the same thing. This is a bit of a dealbreaker if it's not possible, because it would require refactoring all of my client code.

@bambalaus
Copy link

Is this project still active? I'll be happy to give this issue a go and look into how to implement it, I was working on a similar idea then found your repository, that, while not exactly what I'm looking for, seems quite useful and I would be happy to contribute back

@rashfael
Copy link
Contributor

rashfael commented Jun 7, 2015

I currently solve this problem like this: https://github.com/rashfael/koa.io/commit/3caf742fb1c3163335f3f5eee9cfbe0b1c629e7c

I can then do:

app.io.route('someEvent', function *(next, data) {
  // do something with data...
  this.ack('client done processing data...');
 });

This is not the koa/generators way, and @knowthen's proposal sounds much better and looks doable, but I started digging into ES6 and koa just days ago and haven't figured out all the bits yet.

@bambalaus
Copy link

Well, few weeks of node under the belt here, so not really an expert... Said that, I think that what @knowthen propose is doable.
Based on your change in the router file, you just need to add a setter function in the socket object created by koa.io. Something like:

Socket.prototype.__defineSetter__('body', function (ack_params) {
  this.ack(ack_params);
});

then, you should be able to do this in your app:

app.io.route('someEvent', function *(next, data){
  // do something with data...
  this.body = 'client done processing data...';
});

I will update my github fork when I got access to my dev box. Please bear in mind that I have no idea of performance or other consequences of this approach, but it does seem in line with the logic in the rest of the koa.io code

@rashfael
Copy link
Contributor

rashfael commented Jun 8, 2015

Using a setter is a good idea, but I think we should not directly invoke the socket.io ack in the setter. I just had a look into the response handling in koa, and it looks like it is solved as a privileged middleware (https://github.com/koajs/koa/blob/0ad06c9810079174660fa4948ca183d9b90efd3c/lib/application.js#L179-L220), it always gets invoked first. We probably should adapt the same approach.

@rashfael
Copy link
Contributor

rashfael commented Jun 8, 2015

I prototyped this here: https://github.com/rashfael/koa.io/commit/153f1c8c5c25729f3cefeb77888db92b8d2fca55
While implementing this, I noticed that co.gen is used differently than in koa, and because of this, control wasn't returned to routes after a yield? Can someone verify if this is indeed a bug introduced by 6abce9e, or am I just holding it wrong?

@bambalaus
Copy link

cool. It's the same approach taken in the app.callback() function of koa, so design wise seems spot on.
I think you are right about the bug, looking at the code and the co library wiki, now co.wrap return a promises and the callback should be called with .then(). But, this is just reading the code, so some koa/co expert should really confirm this

@hallas
Copy link

hallas commented Jun 8, 2015

@bambalaus co.wrap returns a function that returns a promise, that function takes an argument which is the resolved value of the promise it returns.

@bambalaus
Copy link

@hallas do you mean that it does the same? As mentioned I'm not a nodejs expert at all, but If this is the case I think that it would be better to stick to the usage pattern as defined in the official co wiki (with .then()), as it is used in koa.js as well. It would make it easier to understand.

@mattiasrunge
Copy link

Any progress on implementing this support in some form? Without this functionallity is is a bit of a dealbreaker for me as well.

@ariporad
Copy link
Contributor

@mattiasrunge: Not AFAIK, but I will try to got to it soon, probobly this weekend or next

@ariporad ariporad mentioned this issue Oct 25, 2015
9 tasks
@ariporad
Copy link
Contributor

Ok, this will defiantly be in v0.1.0. Here's what I'm thinking of, syntax wise (I'm open to suggestions though):

app.io.route('getStatus', function* (next, data) {

    // do something

    this.body = 'awesome';
});

@rashfael
Copy link
Contributor

The parameters for the acknowledgment in socket.io are in an array. Callbacks accept multiple parameters. I often use the (error, data) pattern.

To reflect this, should this.body be an array?

this.body = [null, {_id: 3, text: 'some content'}];

For usability, we could automatically wrap a single value into an array.

@tunnckoCore tunnckoCore added this to the 0.1.0 milestone May 21, 2016
@tunnckoCore
Copy link

@rashfael PRs welcome and we can continue discussion there?

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

No branches or pull requests

9 participants