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

Socket.io @SubscribeMessage should return acknowledgment instead of new emit? #982

Closed
MickL opened this issue Aug 17, 2018 · 5 comments
Closed

Comments

@MickL
Copy link
Contributor

MickL commented Aug 17, 2018

I'm submitting a...


[ ] Regression 
[ ] Bug report
[X] Feature request
[ ] Documentation issue or request
[ ] Support request

Current behavior

@SubscribeMessage return statement does a new client.emit() instead of sending a response(acknowledgement):

    @SubscribeMessage('someMessage')
    onSomeMessage(client, data: any) {
        return {event: 'someResponse', data: 'Im a separate message' };
    }

In contrast something like @Get() returns the response:

  findAll(@Param('id') id) {
      return `Returning cat with id ${id}...`;
  }

Expected behavior

Wouldn't it make sense (logically + for consistency) to have the return statement actually return a response (acknowledgement) to the received message?

   @SubscribeMessage('someMessage')
    onSomeMessage(client, data: any) {
        return 'Response to someMessage';
    }

And for sending separate messages explicitly emit a new message?

   @SubscribeMessage('someMessage')
    onSomeMessage(client, data: any) {
       client.emit(event: 'otherMessage', 'Send a different message'); // Or somehow different it this way we cant use interceptors
    }
@AndyGura
Copy link

@MickL I wrote separate module to do it:
https://www.npmjs.com/package/nestjs-socket-handlers-with-ack

Looking forward to see this functionality built-in in NestJS

@kamilmysliwiec
Copy link
Member

It is something that applies solely to socket.io. Since we need to support different platforms as well, we have to provide a more generic solution. However, this sounds reasonable:

  1. When returned value is nil, don't emit anything.
  2. When returned value is an object with event property, emit an event.
  3. When returned value is an object without event property, send response (acknowledgement)

@MickL
Copy link
Contributor Author

MickL commented Aug 23, 2018

Sounds perfect to me!

@kamilmysliwiec
Copy link
Member

Done in 5.3.0 :)

@lock
Copy link

lock bot commented Sep 24, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants