Skip to content
This repository was archived by the owner on Feb 7, 2024. It is now read-only.

Fix presence channels emitting too much events #530

Merged
merged 2 commits into from
Sep 16, 2020
Merged

Fix presence channels emitting too much events #530

merged 2 commits into from
Sep 16, 2020

Conversation

stayallive
Copy link
Contributor

@stayallive stayallive commented Sep 15, 2020

So after #527 I noticed the counts were still wrong in my app. Dangit I thought this must be my stupid JavaScript code not counting correctly. As it turns out it was not 😄 (that does not happen often).

Pusher only fires member_added and member_removed events after the first and last socket connection for a user ID. This library would always emit a member_added event regardless how many times the user was already connected (same for member_removed). This PR aims to fix that behaviour.

Please go over it to make sure it's correct. The tests should prove that it works, but please make sure I've interpreted the Pusher docs correctly and the tests validate the correct behaviour.

This might technically be a breaking change so not sure this could be 1.8 although it would be very nice to still have this in 1.x before the big refactor of 2.x.


There is 1 important consideration for this PR (related to #401). The channel member user_info is updated on every new connection, so technically be re-subscribing to the presence channel you can update your user info, I think this is fine, but Pusher handles this differently and it would be trivial to implement that and make the user info "immutable" until all sockets for a user have been disconnected like Pusher... bit torn about this one. Could possibly become a config option?

@rennokki
Copy link
Collaborator

One thing I couldn't figure out - why do you want more than one connection to the same app key, for the same presence channel? This seems like an anti-pattern.

@stayallive
Copy link
Contributor Author

This is very common if a user opens multiple browser tabs or windows or is logged in on multiple devices.

That creates multiple connections to the same app key (because same app) to the same presence channel for the same user.

Or did I misunderstood what you meant?

@rennokki
Copy link
Collaborator

Oh, yes, now it makes sense. Let me try it myself and see how it behaves.

@rennokki
Copy link
Collaborator

Seems fine to me, I was also able to replicate the issue & fix it in 2.x thanks to your PR. :D

@rennokki rennokki merged commit 805fd5e into beyondcode:master Sep 16, 2020
@stayallive
Copy link
Contributor Author

Awesome, thanks for fixing this in v2 too ❤️

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

Successfully merging this pull request may close these issues.

2 participants