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

Push pending events before redirecting #2923

Merged
merged 1 commit into from
Nov 17, 2023
Merged

Conversation

Gazler
Copy link
Member

@Gazler Gazler commented Nov 17, 2023

Previously, there was no way to handle both push_event and push_redirect occuring in the same callback. The redirect would prevent the events from being sent. This commit checks the diff for any new events, and if they exist, sends them over the channel.

@@ -837,6 +841,11 @@ defmodule Phoenix.LiveView.Channel do
end
end

defp push_pending_events_on_redirect(state, socket) do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider moving this function to the Diff module.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the function makes sense here because it depends on push maybe we could add a get_push_events function to Diff which returns either {"diff", %{e: events}} or nil?

Presumably the goal here is to avoid the duplication of "diff" and @events?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. get_push_events sounds good to me!

Copy link
Member Author

@Gazler Gazler Nov 17, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Gazler Gazler force-pushed the gr-push-events-on-redirect branch from 4cf29a0 to 8aa91ac Compare November 17, 2023 10:17
Previously, there was no way to handle both `push_event` and
`push_redirect` occuring in the same callback. The redirect would
prevent the events from being sent. This commit checks the diff for any
new events, and if they exist, sends them over the channel.
@Gazler Gazler force-pushed the gr-push-events-on-redirect branch from 8aa91ac to ec3566e Compare November 17, 2023 10:18
Copy link
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ship it!

@Gazler Gazler merged commit cb7db8b into main Nov 17, 2023
@mcrumm mcrumm deleted the gr-push-events-on-redirect branch November 17, 2023 18:56
@mcrumm mcrumm restored the gr-push-events-on-redirect branch November 17, 2023 18:56
@mcrumm mcrumm deleted the gr-push-events-on-redirect branch November 17, 2023 18:56
@mcrumm mcrumm restored the gr-push-events-on-redirect branch November 17, 2023 18:56
@mcrumm mcrumm deleted the gr-push-events-on-redirect branch January 23, 2024 17:28
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

Successfully merging this pull request may close these issues.

2 participants