Skip to content

Comments

Added support to merge params when use LiveView#1

Open
diegonogueira wants to merge 1 commit intojulp:masterfrom
diegonogueira:master
Open

Added support to merge params when use LiveView#1
diegonogueira wants to merge 1 commit intojulp:masterfrom
diegonogueira:master

Conversation

@diegonogueira
Copy link

No description provided.

@julp
Copy link
Owner

julp commented Jul 1, 2023

Hi and thanks for your interest,

this functionality should have been added by 379c6d4 (which is not part of any release yet): do you use master and have encountered any issue with it? Is it to add filtering to these parameters? (as of now, you're right, no filtering is applied to parameters from LiveViews/through the :params option)

There is a potential issue with the code you suggest since :params is initialized to nil by default, not a[n empty] map, this will likely crash in filter_params/2. It requires to discard nil first or change this default value to %{}.

Also, the two heads:

  defp query_params(%Plug.Conn{}, %{merge_params: false}) do
    %{}
  end

And:

  defp query_params(%Phoenix.LiveView.Socket{}, %{merge_params: false}) do
    %{}
  end

Could be replaced by a single one:

  defp query_params(_conn_or_socket_or_endpoint, %{merge_params: false}) do
    %{}
  end

And it seems query_params/2 would get the same @spec twice.

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