Remove reactive inputs (#649)

This commit is contained in:
Jonatan Kłosko 2021-10-27 13:35:24 +02:00 committed by GitHub
parent 5c0267b547
commit c1a6bc1aa8
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
10 changed files with 38 additions and 172 deletions

View file

@ -111,8 +111,8 @@ defmodule Livebook.LiveMarkdown.Export do
value: value
}
|> put_unless_default(
Map.take(cell, [:reactive, :props]),
Map.take(Cell.Input.new(), [:reactive, :props])
Map.take(cell, [:props]),
Map.take(Cell.Input.new(), [:props])
)
|> Jason.encode!()

View file

@ -213,9 +213,9 @@ defmodule Livebook.LiveMarkdown.Import do
defp build_notebook([{:cell, :input, data} | elems], cells, sections, messages) do
case parse_input_attrs(data) do
{:ok, attrs} ->
{:ok, attrs, input_messages} ->
cell = Notebook.Cell.new(:input) |> Map.merge(attrs)
build_notebook(elems, [cell | cells], sections, messages)
build_notebook(elems, [cell | cells], sections, messages ++ input_messages)
{:error, message} ->
build_notebook(elems, cells, sections, [message | messages])
@ -273,15 +273,23 @@ defmodule Livebook.LiveMarkdown.Import do
defp parse_input_attrs(data) do
with {:ok, type} <- parse_input_type(data["type"]) do
warnings =
if data["reactive"] == true do
[
"found a reactive input, but those are no longer supported, you can use automatically reevaluating cell instead"
]
else
[]
end
{:ok,
%{
type: type,
name: data["name"],
value: data["value"],
# Fields with implicit value
reactive: Map.get(data, "reactive", false),
props: data |> Map.get("props", %{}) |> parse_input_props(type)
}}
}, warnings}
end
end

View file

@ -6,7 +6,7 @@ defmodule Livebook.Notebook.Cell.Input do
# It consists of an input that the user may fill
# and then read during code evaluation.
defstruct [:id, :type, :name, :value, :reactive, :props]
defstruct [:id, :type, :name, :value, :props]
alias Livebook.Utils
alias Livebook.Notebook.Cell
@ -16,7 +16,6 @@ defmodule Livebook.Notebook.Cell.Input do
type: type(),
name: String.t(),
value: String.t(),
reactive: boolean(),
props: props()
}
@ -39,7 +38,6 @@ defmodule Livebook.Notebook.Cell.Input do
type: :text,
name: "input",
value: "",
reactive: false,
props: %{}
}
end
@ -100,22 +98,6 @@ defmodule Livebook.Notebook.Cell.Input do
def default_props(:select), do: %{options: [""]}
def default_props(_type), do: %{}
@doc """
Checks if the input changed in terms of content.
"""
@spec invalidated?(t(), t()) :: boolean()
def invalidated?(cell, prev_cell) do
cell.value != prev_cell.value or cell.name != prev_cell.name
end
@doc """
Checks if the input change should trigger reactive update.
"""
@spec reactive_update?(t(), t()) :: boolean()
def reactive_update?(cell, prev_cell) do
cell.reactive and cell.value != prev_cell.value and validate(cell) == :ok
end
@doc """
Parses input type from string.
"""

View file

@ -541,11 +541,6 @@ defmodule Livebook.Session.Data do
data
|> with_actions()
|> set_cell_attributes(cell, attrs)
|> then(fn {data, _} = data_actions ->
{:ok, updated_cell, _} = Notebook.fetch_cell_and_section(data.notebook, cell_id)
maybe_queue_bound_cells(data_actions, updated_cell, cell)
end)
|> maybe_evaluate_queued()
|> compute_snapshots_and_validity()
|> set_dirty()
|> wrap_ok()
@ -1141,23 +1136,6 @@ defmodule Livebook.Session.Data do
|> set!(notebook: Notebook.update_cell(data.notebook, cell.id, &Map.merge(&1, attrs)))
end
defp maybe_queue_bound_cells({data, _} = data_actions, %Cell.Input{} = cell, prev_cell) do
if Cell.Input.reactive_update?(cell, prev_cell) do
bound_cells = bound_cells_with_section(data, cell.id)
data_actions
|> reduce(bound_cells, fn data_actions, {bound_cell, section} ->
data_actions
|> queue_prerequisite_cells_evaluation(bound_cell)
|> queue_cell_evaluation(bound_cell, section)
end)
else
data_actions
end
end
defp maybe_queue_bound_cells(data_actions, _cell, _prev_cell), do: data_actions
defp set_runtime(data_actions, prev_data, runtime) do
{data, _} = data_actions = set!(data_actions, runtime: runtime)

View file

@ -13,7 +13,7 @@ defmodule LivebookWeb.SessionLive.InputCellSettingsComponent do
|> assign(assigns)
|> assign(:current_type, cell.type)
|> assign_new(:attrs, fn ->
Map.take(cell, [:name, :type, :reactive, :props])
Map.take(cell, [:name, :type, :props])
end)
|> assign_new(:valid, fn -> true end)
@ -43,10 +43,6 @@ defmodule LivebookWeb.SessionLive.InputCellSettingsComponent do
<input type="text" class="input" name="attrs[name]" value={@attrs.name} autofocus />
</div>
<.extra_fields type={@attrs.type} props={@attrs.props} myself={@myself} />
<.switch_checkbox
name="attrs[reactive]"
label="Reactive (reevaluates dependent cells on change)"
checked={@attrs.reactive} />
</div>
<div class="mt-8 flex justify-end space-x-2">
<%= live_patch "Cancel", to: @return_to, class: "button button-outlined-gray" %>
@ -161,7 +157,6 @@ defmodule LivebookWeb.SessionLive.InputCellSettingsComponent do
defp validate_attrs(data, prev_attrs) do
name = data["name"]
type = data["type"] |> String.to_existing_atom()
reactive = data["reactive"] == "true"
{props_valid?, props} =
if type == prev_attrs.type do
@ -172,7 +167,7 @@ defmodule LivebookWeb.SessionLive.InputCellSettingsComponent do
valid? = name != "" and props_valid?
{valid?, %{name: name, type: type, reactive: reactive, props: props}}
{valid?, %{name: name, type: type, props: props}}
end
defp validate_props(data, :range) do

View file

@ -50,8 +50,7 @@ defmodule Livebook.LiveMarkdown.ExportTest do
Notebook.Cell.new(:input)
| type: :text,
name: "length",
value: "100",
reactive: true
value: "100"
},
%{
Notebook.Cell.new(:elixir)
@ -107,7 +106,7 @@ defmodule Livebook.LiveMarkdown.ExportTest do
## Section 2
<!-- livebook:{"livebook_object":"cell_input","name":"length","reactive":true,"type":"text","value":"100"} -->
<!-- livebook:{"livebook_object":"cell_input","name":"length","type":"text","value":"100"} -->
```elixir
IO.gets("length: ")

View file

@ -29,7 +29,7 @@ defmodule Livebook.LiveMarkdown.ImportTest do
## Section 2
<!-- livebook:{"livebook_object":"cell_input","name":"length","reactive":true,"type":"text","value":"100"} -->
<!-- livebook:{"livebook_object":"cell_input","name":"length","type":"text","value":"100"} -->
```elixir
IO.gets("length: ")
@ -88,8 +88,7 @@ defmodule Livebook.LiveMarkdown.ImportTest do
%Cell.Input{
type: :text,
name: "length",
value: "100",
reactive: true
value: "100"
},
%Cell.Elixir{
source: """
@ -598,4 +597,20 @@ defmodule Livebook.LiveMarkdown.ImportTest do
~s{unrecognised input type "input_from_the_future", if it's a valid type it means your Livebook version doesn't support it}
] == messages
end
describe "backward compatibility" do
markdown = """
# My Notebook
## Section 1
<!-- livebook:{"livebook_object":"cell_input","name":"length","reactive":true,"type":"text","value":"100"} -->
"""
{_notebook, messages} = Import.notebook_from_markdown(markdown)
assert [
"found a reactive input, but those are no longer supported, you can use automatically reevaluating cell instead"
] == messages
end
end

View file

@ -68,50 +68,4 @@ defmodule Livebook.Notebook.Cell.InputText do
assert Input.validate(input) == {:error, "number too big"}
end
end
describe "invalidated?/2" do
test "returns false if only the type changes" do
input = %{Input.new() | type: :text}
updated_input = %{input | type: :url}
refute Input.invalidated?(updated_input, input)
end
test "returns true if the name changes" do
input = %{Input.new() | name: "Name"}
updated_input = %{input | name: "Full name"}
assert Input.invalidated?(updated_input, input)
end
test "returns true if the value changes" do
input = %{Input.new() | value: "Jake Peralta"}
updated_input = %{input | value: "Amy Santiago"}
assert Input.invalidated?(updated_input, input)
end
end
describe "reactive_change?/2" do
test "returns false if the input is not reactive" do
input = %{Input.new() | reactive: false, value: "Jake Peralta"}
updated_input = %{input | value: "Amy Santiago"}
refute Input.reactive_update?(updated_input, input)
end
test "returns true if the input is reactive and value changes" do
input = %{Input.new() | reactive: true, value: "Jake Peralta"}
updated_input = %{input | value: "Amy Santiago"}
assert Input.reactive_update?(updated_input, input)
end
test "returns false if the new value is invalid" do
input = %{Input.new() | reactive: true, type: :number, value: "10"}
updated_input = %{input | value: "invalid"}
refute Input.reactive_update?(updated_input, input)
end
end
end

View file

@ -47,8 +47,7 @@ defmodule Livebook.Notebook.Export.ElixirTest do
Notebook.Cell.new(:input)
| type: :text,
name: "length",
value: "100",
reactive: true
value: "100"
},
%{
Notebook.Cell.new(:elixir)

View file

@ -2933,7 +2933,6 @@ defmodule Livebook.Session.DataTest do
data_after_operations!([
{:insert_section, self(), 0, "s1"},
{:insert_cell, self(), "s1", 0, :input, "c1"},
{:set_cell_attributes, self(), "c1", %{reactive: false}},
# Insert three evaluated cells and bind the second one to the input
{:insert_cell, self(), "s1", 1, :elixir, "c2"},
{:insert_cell, self(), "s1", 2, :elixir, "c3"},
@ -2961,69 +2960,6 @@ defmodule Livebook.Session.DataTest do
}, _} = Data.apply_operation(data, operation)
end
test "given reactive input value change, triggers bound cells evaluation" do
data =
data_after_operations!([
{:insert_section, self(), 0, "s1"},
{:insert_cell, self(), "s1", 0, :input, "c1"},
{:set_cell_attributes, self(), "c1", %{reactive: true}},
# Insert three evaluated cells and bind the second and third one to the input
{:insert_cell, self(), "s1", 1, :elixir, "c2"},
{:insert_cell, self(), "s1", 2, :elixir, "c3"},
{:insert_cell, self(), "s1", 3, :elixir, "c4"},
{:set_runtime, self(), NoopRuntime.new()},
{:queue_cell_evaluation, self(), "c2"},
{:queue_cell_evaluation, self(), "c3"},
{:queue_cell_evaluation, self(), "c4"},
{:add_cell_evaluation_response, self(), "c2", @eval_resp, @eval_meta},
{:add_cell_evaluation_response, self(), "c3", @eval_resp, @eval_meta},
{:add_cell_evaluation_response, self(), "c4", @eval_resp, @eval_meta},
{:bind_input, self(), "c3", "c1"},
{:bind_input, self(), "c4", "c1"}
])
attrs = %{value: "stuff"}
operation = {:set_cell_attributes, self(), "c1", attrs}
assert {:ok,
%{
cell_infos: %{
"c2" => %{evaluation_status: :ready},
"c3" => %{evaluation_status: :evaluating},
"c4" => %{evaluation_status: :queued}
}
}, _} = Data.apply_operation(data, operation)
end
test "given reactive input value change, queues bound cell evaluation even if evaluating" do
data =
data_after_operations!([
{:insert_section, self(), 0, "s1"},
{:insert_cell, self(), "s1", 0, :input, "c1"},
{:set_cell_attributes, self(), "c1", %{reactive: true}},
{:insert_cell, self(), "s1", 1, :elixir, "c2"},
{:set_runtime, self(), NoopRuntime.new()},
{:queue_cell_evaluation, self(), "c2"},
{:bind_input, self(), "c2", "c1"}
])
attrs = %{value: "stuff"}
operation = {:set_cell_attributes, self(), "c1", attrs}
assert {:ok,
%{
cell_infos: %{
"c2" => %{evaluation_status: :evaluating}
},
section_infos: %{
"s1" => %{
evaluating_cell_id: "c2",
evaluation_queue: ["c2"]
}
}
}, _} = Data.apply_operation(data, operation)
end
test "setting reevaluate_automatically on stale cell marks it for evaluation" do
data =
data_after_operations!([