Proposal: Make frame render functions return the frame.

[christhekeele]

This accomplishes three things in the name of pipe-ability:

• Makes Kino.Frame.render consistent with Kino.render
  This also lets you do things like this:

  import Kino.Shorts
  - frame = frame() |> Kino.render
  - frame |> Kino.Frame.render(markdown("> ***Sumbit an event***"))
  + frame = frame() |> Kino.render() |> Kino.Frame.render(markdown("> ***Sumbit an event***"))

• Makes Kino.Frame.append(frame, term) more chainable
  For example:

  import Kino.Shorts
  frame() |> Kino.render
  |> Kino.Frame.render(markdown("> ***Sumbit an event***"))
  |> Kino.Frame.append(markdown("- Or any term really"))
  |> Kino.Frame.append(markdown("- I'm a form, not a cop"))

• Makes Kino.Frame.clear(frame) chainable
  For example:

  # for some existing frame and form...
  Kino.listen(form, fn event ->
    frame
    |> Kino.Frame.clear()
    |> Kino.Frame.render(markdown("`\#{inspect event}`"))
  end)

This implementation wraps the GenServer.calls with with, which technically may return something other than :ok, but effectively makes the same assumptions that the previous typespec did that it will always be :ok. We could just match on :ok (like in clear's cast) or even ignore the result of the call, but this is the most backwards-compatible with any existing code today that tries to handle non-:ok GenServer.call results (I would be suprised if any such code was out there, though).

[josevalim]

We can't do these changes, because it means any code today that does Kino.Frame.render(...) (or any of the functions above) at the end, will now embed the result into the frame and as an output. I am almost sure we chose :ok on purpose, exactly to avoid this.

[christhekeele]

We can't do these changes, because it means any code today that does Kino.Frame.render(...) (or any of the functions above) at the end, will now embed the result into the frame and as an output.

I see. The problem would be if you create a frame in one cell, then update/render it in another. Without a Kino.nothing ending the first cell it'll be implicitly rendered.

I tend to co-locate my frames with the code necessary to render into them in the same cell, which (without a fluent API) necessarily means the frame can't be the implicit output. (I have to assign it, render with :ok, then reference it in ex. a listener with its own return value.) So the frame never made it to the end of my cells, and this edge case did not occur to me. (At least, until I started this PR, there's a reason why I proposed the pipeable nothing at the same time: to frame() |> render() |> output_nothing()).

But yeah, dividing the frame creation and rendering between cells could lead to double-embedding issues, makes sense.

I am almost sure we chose :ok on purpose, exactly to avoid this.

Would it make sense to have them return a more intention-revealing Kino.nothing/0 to safe-guard against double rendering instead? When reading the code, relying on the GenServer APIs to return :ok did not signal this intent to me, neither did the docs/typespecs. Whereas something like this with a note in the @doc might be more instructive:

[christhekeele]

We can't do these changes, because it means any code today that does Kino.Frame.render(...) (or any of the functions above) at the end, will now embed the result into the frame and as an output.

I'm not convinced that this DX is a bad thing: if you are creating a frame in one cell and using it in another, you are probably experimenting with frames (and might want to see the intermediary output). As opposed to building a re-renderable UI in a single cell (and necessarily co-locating listeners after frame initialization, where you need a handle to it). A simple Kino.nothing() would fix if this is undesired. But it would be a possibly breaking change and need to be released accordingly.

[christhekeele]

Side note: if we do decide on this, releasing as a potentially breaking change, we can probably drop the with on the GenServer.call returns and simply match on :ok. Since we are more loudly modifying the return value of this function and asking people to check their call sites for assumptions, we can also narrow the expected return of the call from term() to :ok knowing folk won't be trying to handle term() themselves.