adopt-liveview / feedback

0 stars 0 forks source link

Feedback about `multiple-pushes` #11

Open lunks opened 3 months ago

lunks commented 3 months ago

It could be a bit more didactic if in the recap you added diffs from the initial code to the final one.

Another question, why do we have to do js \\ %JS{} instead of %JS{} as the first argument on add_points?

lubien commented 3 months ago

Another question, why do we have to do js \\ %JS{} instead of %JS{} as the first argument on add_points?

The reason for that is that we could use both add_points(type, number) (js defaults to %JS{}) or chain like this: add_points(type, number) |> add_points(type, number)

Since add_points/3 will return a %JS{}, the second add_points call will receive the previous one and use it so its the same as:

js = add_points(type, number)
add_points(js, type, number)

Why do we need to pass the %JS{} around?

Every time you use JS.push or any other JS Commands function what you are actually creating is a data structure called %JS{}. When empty it looks like this: %Phoenix.LiveView.JS{ops: []}. It contains the list of operations that will be performed.

If we ignore the previous %JS{} we will only do the second operation which would not do what you wanted. Wanna check this bug? Edit your function to:

  defp add_points(js \\ %JS{}, team, amount) do
    # BUG: always ignore the `js` and use a new one below!
    JS.push(%JS{}, "add_points", value: %{team: team, amount: amount})
  end