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

Presence onJoin callback includes old metas in newPresence var #3475

Closed
indrekj opened this issue Jul 12, 2019 · 1 comment
Closed

Presence onJoin callback includes old metas in newPresence var #3475

indrekj opened this issue Jul 12, 2019 · 1 comment

Comments

@indrekj
Copy link
Contributor

indrekj commented Jul 12, 2019

Environment

  • Phoenix JS library v1.4.9

Issue

presence.onJoin((id, current, newPresence) => {
});

Given: I have a user with one tab (one meta)
When: I update the user (from phoenix presence in elixir)
Then: I expect newPresence to include one meta with the updated data
ACTUAL: newPresence has two metas, one old and one updated

If I force onJoin to next event loop like this:

presence.onJoin((id, current, newPresence) => {
  setTimeout(() => {
    // now do my thing
  });
});

then newPresence includes only one meta which is the updated meta, which I actually expect.

Digging in the code

As I understand then here https://github.com/phoenixframework/phoenix/blob/master/assets/js/phoenix.js#L1329 state[key] is set to newPresence. Then state[key].metas.unshift(...curMetas) adds current metas to the state[key]. This however also mutates the newPresence because the var was not cloned. Then onJoin(key, currentPresence, newPresence) is called where newPresence now has the old metas as well.

setTimeout works because this forces https://github.com/phoenixframework/phoenix/blob/master/assets/js/phoenix.js#L1337 to iterate over leaves which mutates newPresence to be correct again.

It seems to be that this is not a correct behavior. If it is, can you explain why?

The issue does not exist with onSync callback because that's called after both joins and leaves are processed. We however don't want to use onSync because we want unchanged presences to remain referentially equivalent which onSync does not provide as it clones the entire list. Our online user list has thousands of users and we want to leverage referential equivalence and also don't want to map over the whole list each time for fast react based dom updates.

@indrekj
Copy link
Contributor Author

indrekj commented Jul 12, 2019

Also, onJoin and onLeave both call a function with 3 arguments: key, currentPresence, newPresence OR leftPresence. The currentPresence however seems to be different for both of them:

onJoin currentPresence value is the same value as before the update.
onLeave currentPresence value is with join metas added and also leave metas removed.

So in onJoin case we get the state before the changes and in onLeave case we get the state after all the changes.

I'm ok submitting a PR but I'm not really sure what is the correct behavior here.

indrekj added a commit to indrekj/phoenix that referenced this issue Aug 5, 2019
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this also helped to get rid of the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

This addresses both phoenixframework#3509 and phoenixframework#3475
indrekj added a commit to salemove/phoenix that referenced this issue Aug 6, 2019
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this also helped to get rid of the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

This addresses both phoenixframework#3509 and phoenixframework#3475
indrekj added a commit to indrekj/phoenix that referenced this issue Sep 6, 2019
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this also helped to get rid of the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

This addresses both phoenixframework#3509 and phoenixframework#3475
indrekj added a commit to indrekj/phoenix that referenced this issue Sep 6, 2019
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this also helped to get rid of the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

This addresses both phoenixframework#3509 and phoenixframework#3475
indrekj added a commit to indrekj/phoenix that referenced this issue Sep 6, 2019
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this also helped to get rid of the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

This addresses both phoenixframework#3509 and phoenixframework#3475
indrekj added a commit to indrekj/phoenix that referenced this issue Sep 6, 2019
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this way we don't have to use the the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

onJoin and onLeave are still in the API because of backwards
compatibility but they're marked as deprecated.

This addresses both phoenixframework#3509 and phoenixframework#3475
indrekj added a commit to salemove/phoenix that referenced this issue Feb 5, 2020
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this also helped to get rid of the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

This addresses both phoenixframework#3509 and phoenixframework#3475
indrekj added a commit to indrekj/phoenix that referenced this issue Feb 6, 2020
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this way we don't have to use the the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

onJoin and onLeave are still in the API because of backwards
compatibility but they're marked as deprecated.

This addresses both phoenixframework#3509 and phoenixframework#3475
indrekj added a commit to indrekj/phoenix that referenced this issue Feb 6, 2020
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this way we don't have to use the the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

onJoin and onLeave are still in the API because of backwards
compatibility but they're marked as deprecated.

This addresses both phoenixframework#3509 and phoenixframework#3475
indrekj added a commit to indrekj/phoenix that referenced this issue Feb 14, 2020
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this way we don't have to use the the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

onJoin and onLeave are still in the API because of backwards
compatibility but they're marked as deprecated.

This addresses both phoenixframework#3509 and phoenixframework#3475
indrekj added a commit to salemove/phoenix that referenced this issue Apr 15, 2020
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this way we don't have to use the the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

onJoin and onLeave are still in the API because of backwards
compatibility but they're marked as deprecated.

This addresses both phoenixframework#3509 and phoenixframework#3475
bautrey37 pushed a commit to salemove/phoenix that referenced this issue Nov 16, 2021
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this way we don't have to use the the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

onJoin and onLeave are still in the API because of backwards
compatibility but they're marked as deprecated.

This addresses both phoenixframework#3509 and phoenixframework#3475
bautrey37 pushed a commit to salemove/phoenix that referenced this issue Jan 4, 2022
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this way we don't have to use the the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

onJoin and onLeave are still in the API because of backwards
compatibility but they're marked as deprecated.

This addresses both phoenixframework#3509 and phoenixframework#3475
laurglia pushed a commit to salemove/phoenix that referenced this issue Aug 18, 2022
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this way we don't have to use the the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

onJoin and onLeave are still in the API because of backwards
compatibility but they're marked as deprecated.

This addresses both phoenixframework#3509 and phoenixframework#3475
laurglia pushed a commit to salemove/phoenix that referenced this issue Aug 18, 2022
Syncing presences was very slow due to using clone(). For channels that
have couple of thousand of users where users are joining/leaving and we
also have updates, clone() used almost all the CPU.

onJoin and onLeave callbacks were merged into one onChange callback.
This is because previous onJoin and onLeave callbacks were very
confusing and this way we don't have to use the the clone function.
Previously onJoin and onLeave both were called with 3 arguments: key,
currentPresence and newPresence/leftPresence. The currentPresence
however was the same value as before the update for onJoin, but updated
presence (joins/leaves processed) for onLeave. So in onJoin case we get
the state before the changes and in onLeave case we get the state after
all the changes.

Also the old onJoin newPresence variables included metas that were in
leaves payload which made it hard to detect a join vs an update.

New onChange callback includes the previous presence (state before the
updates) and new presence (state after the updates) which makes it much
easier to process changes and see what has happened.

onJoin and onLeave are still in the API because of backwards
compatibility but they're marked as deprecated.

This addresses both phoenixframework#3509 and phoenixframework#3475
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant