Repository: livebook-dev/kino
Pull request #393 (2024-02-02 04:47:41)

Convert container contents to output lazily
For frames `Kino.Output.to_livebook/2` returns an output with the current set of outputs. Currently containers (grid/tabs/frame) convert their contents to output eagerly, and so if the content representation changes, a future container render is not going to know about it. Example:

```elixir
frame_inner = Kino.Frame.new()
grid = Kino.Layout.grid([frame_inner])

Kino.Frame.render(frame_inner, "content")
grid
```

This PR makes the conversion lazy, so that containers store their contents as is, and only call `Kino.Output.to_livebook/2` on render.


Patch:
diff --git a/lib/kino/frame.ex b/lib/kino/frame.ex
index a2c0939..8b83016 100644
--- a/lib/kino/frame.ex
+++ b/lib/kino/frame.ex
@@ -38,9 +38,6 @@ defmodule Kino.Frame do
             placeholder: boolean()
           }
 
-  @typedoc false
-  @type state :: %{outputs: list(Kino.Output.t())}
-
   @doc """
   Creates a new frame.
 
@@ -151,20 +148,20 @@ defmodule Kino.Frame do
   end
 
   @doc false
-  @spec get_outputs(t()) :: list(Kino.Output.t())
-  def get_outputs(frame) do
-    GenServer.call(frame.pid, :get_outputs)
+  @spec get_items(t()) :: list(term())
+  def get_items(frame) do
+    GenServer.call(frame.pid, :get_items)
   end
 
   @impl true
   def init(ref) do
-    {:ok, %{ref: ref, outputs: []}}
+    {:ok, %{ref: ref, items: []}}
   end
 
   @impl true
   def handle_cast({:clear, destination}, state) do
     put_update(destination, state.ref, [], :replace)
-    state = update_outputs(state, destination, fn _ -> [] end)
+    state = update_items(state, destination, fn _ -> [] end)
     {:noreply, state}
   end
 
@@ -172,26 +169,26 @@ defmodule Kino.Frame do
   def handle_call({:render, term, destination}, _from, state) do
     output = Kino.Render.to_livebook(term)
     put_update(destination, state.ref, [output], :replace)
-    state = update_outputs(state, destination, fn _ -> [output] end)
+    state = update_items(state, destination, fn _ -> [term] end)
     {:reply, :ok, state}
   end
 
   def handle_call({:append, term, destination}, _from, state) do
     output = Kino.Render.to_livebook(term)
     put_update(destination, state.ref, [output], :append)
-    state = update_outputs(state, destination, &[output | &1])
+    state = update_items(state, destination, &[term | &1])
     {:reply, :ok, state}
   end
 
-  def handle_call(:get_outputs, _from, state) do
-    {:reply, state.outputs, state}
+  def handle_call(:get_items, _from, state) do
+    {:reply, state.items, state}
   end
 
-  defp update_outputs(state, :default, update_fun) do
-    update_in(state.outputs, update_fun)
+  defp update_items(state, :default, update_fun) do
+    update_in(state.items, update_fun)
   end
 
-  defp update_outputs(state, _destination, _update_fun), do: state
+  defp update_items(state, _destination, _update_fun), do: state
 
   defp put_update(destination, ref, outputs, type) do
     output = %{type: :frame_update, ref: ref, update: {type, outputs}}
diff --git a/lib/kino/layout.ex b/lib/kino/layout.ex
index 0fd8792..8cfa409 100644
--- a/lib/kino/layout.ex
+++ b/lib/kino/layout.ex
@@ -3,11 +3,11 @@ defmodule Kino.Layout do
   Layout utilities for arranging multiple kinos together.
   """
 
-  defstruct [:type, :outputs, :info]
+  defstruct [:type, :items, :info]
 
   @opaque t :: %__MODULE__{
             type: :tabs | :grid,
-            outputs: list(Kino.Output.t()),
+            items: list(term()),
             info: map()
           }
 
@@ -31,9 +31,8 @@ defmodule Kino.Layout do
   def tabs(tabs) do
     {labels, terms} = Enum.unzip(tabs)
     labels = Enum.map(labels, &to_string/1)
-    outputs = Enum.map(terms, &Kino.Render.to_livebook/1)
     info = %{labels: labels}
-    %Kino.Layout{type: :tabs, outputs: outputs, info: info}
+    %Kino.Layout{type: :tabs, items: terms, info: info}
   end
 
   @doc """
@@ -65,7 +64,6 @@ defmodule Kino.Layout do
   @spec grid(list(term()), keyword()) :: t()
   def grid(terms, opts \\ []) do
     opts = Keyword.validate!(opts, columns: 1, boxed: false, gap: 8)
-    outputs = Enum.map(terms, &Kino.Render.to_livebook/1)
 
     info = %{
       columns: opts[:columns],
@@ -73,6 +71,6 @@ defmodule Kino.Layout do
       gap: opts[:gap]
     }
 
-    %Kino.Layout{type: :grid, outputs: outputs, info: info}
+    %Kino.Layout{type: :grid, items: terms, info: info}
   end
 end
diff --git a/lib/kino/render.ex b/lib/kino/render.ex
index fcf9577..ae63770 100644
--- a/lib/kino/render.ex
+++ b/lib/kino/render.ex
@@ -103,20 +103,23 @@ defimpl Kino.Render, for: Kino.Frame do
 
   def to_livebook(kino) do
     Kino.Bridge.reference_object(kino.pid, self())
-    outputs = Kino.Frame.get_outputs(kino)
+    outputs = kino |> Kino.Frame.get_items() |> Enum.map(&Kino.Render.to_livebook/1)
     %{type: :frame, ref: kino.ref, outputs: outputs, placeholder: kino.placeholder}
   end
 end
 
 defimpl Kino.Render, for: Kino.Layout do
   def to_livebook(%{type: :tabs} = kino) do
-    %{type: :tabs, outputs: kino.outputs, labels: kino.info.labels}
+    outputs = Enum.map(kino.items, &Kino.Render.to_livebook/1)
+    %{type: :tabs, outputs: outputs, labels: kino.info.labels}
   end
 
   def to_livebook(%{type: :grid} = kino) do
+    outputs = Enum.map(kino.items, &Kino.Render.to_livebook/1)
+
     %{
       type: :grid,
-      outputs: kino.outputs,
+      outputs: outputs,
       columns: kino.info.columns,
       gap: kino.info.gap,
       boxed: kino.info.boxed


Test patch:
diff --git a/test/kino/frame_test.exs b/test/kino/frame_test.exs
index da988b5..74a1ad0 100644
--- a/test/kino/frame_test.exs
+++ b/test/kino/frame_test.exs
@@ -29,7 +29,7 @@ defmodule Kino.FrameTest do
       %{type: :frame_update, update: {:replace, [%{type: :terminal_text, text: "\e[34m1\e[0m"}]}}
     )
 
-    assert Kino.Frame.get_outputs(frame) == []
+    assert Kino.Frame.get_items(frame) == []
   end
 
   test "render/2 sends output directly to clients when :temporary is true" do
@@ -42,7 +42,7 @@ defmodule Kino.FrameTest do
       update: {:replace, [%{type: :terminal_text, text: "\e[34m1\e[0m"}]}
     })
 
-    assert Kino.Frame.get_outputs(frame) == []
+    assert Kino.Frame.get_items(frame) == []
   end
 
   test "render/2 raises when :to and :temporary is disabled" do
@@ -87,4 +87,24 @@ defmodule Kino.FrameTest do
       update: {:append, [%{type: :terminal_text, text: "\e[34m1\e[0m"}]}
     })
   end
+
+  test "Kino.Render.to_livebook/1 returns the current value for a nested frame" do
+    frame = Kino.Frame.new()
+
+    frame_inner = Kino.Frame.new()
+
+    Kino.Frame.render(frame, frame_inner)
+
+    assert %{
+             type: :frame,
+             outputs: [%{type: :frame, outputs: []}]
+           } = Kino.Render.to_livebook(frame)
+
+    Kino.Frame.render(frame_inner, 1)
+
+    assert %{
+             type: :frame,
+             outputs: [%{type: :frame, outputs: [%{type: :terminal_text, text: "\e[34m1\e[0m"}]}]
+           } = Kino.Render.to_livebook(frame)
+  end
 end
diff --git a/test/kino/layout_test.exs b/test/kino/layout_test.exs
new file mode 100644
index 0000000..f5f2881
--- /dev/null
+++ b/test/kino/layout_test.exs
@@ -0,0 +1,47 @@
+defmodule Kino.LayoutTest do
+  use Kino.LivebookCase, async: true
+
+  describe "tabs" do
+    test "Kino.Render.to_livebook/1 returns the current value for a nested frame" do
+      frame_inner = Kino.Frame.new()
+
+      tabs = Kino.Layout.tabs(frame: frame_inner)
+
+      assert %{
+               type: :tabs,
+               outputs: [%{type: :frame, outputs: []}]
+             } = Kino.Render.to_livebook(tabs)
+
+      Kino.Frame.render(frame_inner, 1)
+
+      assert %{
+               type: :tabs,
+               outputs: [
+                 %{type: :frame, outputs: [%{type: :terminal_text, text: "\e[34m1\e[0m"}]}
+               ]
+             } = Kino.Render.to_livebook(tabs)
+    end
+  end
+
+  describe "grid" do
+    test "Kino.Render.to_livebook/1 returns the current value for a nested frame" do
+      frame_inner = Kino.Frame.new()
+
+      grid = Kino.Layout.grid([frame_inner])
+
+      assert %{
+               type: :grid,
+               outputs: [%{type: :frame, outputs: []}]
+             } = Kino.Render.to_livebook(grid)
+
+      Kino.Frame.render(frame_inner, 1)
+
+      assert %{
+               type: :grid,
+               outputs: [
+                 %{type: :frame, outputs: [%{type: :terminal_text, text: "\e[34m1\e[0m"}]}
+               ]
+             } = Kino.Render.to_livebook(grid)
+    end
+  end
+end
diff --git a/test/kino/tree_test.exs b/test/kino/tree_test.exs
index c1c8738..54cdc87 100644
--- a/test/kino/tree_test.exs
+++ b/test/kino/tree_test.exs
@@ -186,7 +186,7 @@ defmodule Kino.TreeTest do
   defp tree(input) do
     %Kino.Layout{
       type: :grid,
-      outputs: [%{type: :js, js_view: %{ref: ref}}]
+      items: [%Kino.JS{ref: ref}]
     } = Kino.Tree.new(input)
 
     send(Kino.JS.DataStore, {:connect, self(), %{origin: "client:#{inspect(self())}", ref: ref}})