-
Notifications
You must be signed in to change notification settings - Fork 187
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
repair socket io leave method #33
Conversation
Can you describe what you're trying to accomplish here in simple terms? Are you trying to unsubscribe from a single channel? From a single event on that channel? |
Answer: I'm trying to unsubscribe from one channel and I'm trying to remove all related callbacks for this channel from "socket.io" callback list. Additional information: I'm using angular routes. When i'm surfing between view states, I need to connect to another channels. And system has to manage callbacks correctly. Without this patch: On unsubscribe from a channel system will remove all callbacks (callbacks from another channels too). This patch gives you a correct way to manage your channel subscriptions and callbacks. I'm not sure: This patch is using 'underscore', but laravel echo has no this dependency. Of course you can rewrite it or add underscore (or lodash) to echo dependencies. If it's not a good answer, I can try to describe it better. |
Can you rewrite it without using underscore? |
Done. |
@taylorotwell Can you accept this PR? If something wrong, just tell me, and I will fix it. Right now it works without underscore. |
@@ -41,7 +41,7 @@ export class SocketIoPresenceChannel extends SocketIoChannel implements Presence | |||
*/ | |||
leaving(callback): SocketIoPresenceChannel { | |||
this.on('presence:leaving', (member) => { | |||
callback(member.user_info, this.channel); | |||
callback(member.user_info, this.channel.socket); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why you're changing all of these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't need to send in callback full channel object. So I changed it to socket object.
On this screen You can see a full channel object, that includes event list with registered callbacks and socket:
Before this patch you was sending sockets and you will send sockets now too.
Of course, I can delete these changes in "socketio-presence-channel.ts" file.
Hi, @taylorotwell . I know you have another things to do. But I need this update, because when I'm trying to unsubscribe from a channel, it also removes callbacks from another channels. So this update is important for me and for another 'laravel-echo' users. It is tested in real project and it works good. If you have any questions, I'll be good to answer. Can you accept this PR? |
I'm familiar with the situation you are dealing with when working with an SPA. I'll take a second look at this @taylorotwell. @pderiy in the mean time, just fork the repo and add your patch, and include locally if it is blocking your progress. |
This has merge conflicts. |
@taylorotwell , @tlaverdure fixed merge conflicts.
|
@taylorotwell , @tlaverdure The problem is not fixed in laravel-echo-server v1.0.2. U can simply test it.
You will receive data 2 times. Because it's still not fixed. But my PR will fix this problem. |
@pderiy you are right, there is still a case here that needs to be addressed. I'm going to submit a new PR based on what you've submitted that is a bit more concise. There is another issue that I caught as well that comes with killing listeners from different channels with the same event name. Will submit soon and I'll also check if we need this for Pusher, which I assume we will since Pusher doesn't automatically maintain the state of callbacks applied to a channel. |
@tlaverdure My patch removes only one callback. This patch is not killing listeners from different channels! Now laravel-echo have a list of channels and channel-listeners. When you need to unsubscribe from a channel it is going foreach loop and removes callbacks from used channel using "socket.io" method: it doesn't remove listeners from another channels! It is what you need. @tlaverdure you're right, this problem might be in Pusher. But Pusher might have another structure, so you have to write something else for pusher. But for socket.io this is a good way to resolve this problem. It remembers all callbacks for channel and then removes only needed callbacks from needed channels. @tlaverdure you can test it from my fork laravel-echo. Now I have resolved this problem and it perfectly works with laravel-echo-server v1.0.0. And if you will see this as good solution, I can submit a new PR. @tlaverdure Laravel-echo-server v1.0.* looks very good! Is much better than it was. Good job! |
…xes socket.io issues with laravel#33.
Thanks @tlaverdure . Your PR is fixing this problem! |
Hi, Taylor
3 days ago I sent to you PR with unsubscribe socket.io method override.
Method "this.socket.removeAllListeners();" doesn't work like I expected. This method deletes all callbacks from all channels. For example I have a reconnector and channel that listening new orders. When I'll leave from orders channel, this method will also remove a reconnector callbacks.
I wrote a method, that saves a collection of channel callbacks. And when user will try to unsubscrube from a channel, then all callbacks will be also deleted.
Now channel collection looks like:
this.channels = {
'channel-name' => [
'channel' => channel_object,
'events' => {
"event-name" => [ functions.... ]
}
]
}
My PR for now working perfect with "underscore". Of course you can do refactoring and remove underscore. These methods tested and work as expected. Right now, method "leave" works good with 'socket.io'. When user is trying to leave from a channel it unsubscribes and removes all callbacks only from one channel, by using method from "socket.io" library - removeListener( event, callback).
P.S. Sorry for my bad english