Unregister Bonds at the start of the reactive run. (#2467)

* Use `PlutoRunner.move_vars` to deregister Bond elements * Use `GiveCellID` in `@bind`
fonsp · Feb 28, 2023 · 776cbf7 · 776cbf7
1 parent d7abb4e
commit 776cbf7
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 26 deletions.
diff --git a/src/evaluation/RunBonds.jl b/src/evaluation/RunBonds.jl
@@ -1,7 +1,7 @@
-function set_bond_values_reactive(; 
-    session::ServerSession, notebook::Notebook, 
-    bound_sym_names::AbstractVector{Symbol}, 
-    is_first_values::AbstractVector{Bool}=[false for x in bound_sym_names], 
+function set_bond_values_reactive(;
+    session::ServerSession, notebook::Notebook,
+    bound_sym_names::AbstractVector{Symbol},
+    is_first_values::AbstractVector{Bool}=[false for x in bound_sym_names],
     initiator=nothing,
     kwargs...
 )::Union{Task,TopologicalOrder}
@@ -39,13 +39,23 @@ function set_bond_values_reactive(;
 
     new_values = Any[notebook.bonds[bound_sym].value for bound_sym in syms_to_set]
     bond_value_pairs = zip(syms_to_set, new_values)
-
+
+    syms_to_set_set = Set{Symbol}(syms_to_set)
     function custom_deletion_hook((session, notebook)::Tuple{ServerSession,Notebook}, old_workspace_name, new_workspace_name, to_delete_vars::Set{Symbol}, methods_to_delete::Set{Tuple{UUID,FunctionName}}, to_reimport::Set{Expr}, invalidated_cell_uuids::Set{UUID}; to_run::AbstractVector{Cell})
-        to_delete_vars = Set([to_delete_vars..., syms_to_set...]) # also delete the bound symbols
-        WorkspaceManager.move_vars((session, notebook), old_workspace_name, new_workspace_name, to_delete_vars, methods_to_delete, to_reimport, invalidated_cell_uuids)
+        to_delete_vars = union(to_delete_vars, syms_to_set_set) # also delete the bound symbols
+        WorkspaceManager.move_vars(
+            (session, notebook),
+            old_workspace_name,
+            new_workspace_name,
+            to_delete_vars,
+            methods_to_delete,
+            to_reimport,
+            invalidated_cell_uuids,
+            syms_to_set_set,
+        )
         set_bond_value_pairs!(session, notebook, zip(syms_to_set, new_values))
     end
-    to_reeval = where_referenced(notebook, notebook.topology, Set{Symbol}(syms_to_set))
+    to_reeval = where_referenced(notebook, notebook.topology, syms_to_set_set)
 
     run_reactive_async!(session, notebook, to_reeval; deletion_hook=custom_deletion_hook, save=false, user_requested_run=false, run_async=false, bond_value_pairs, kwargs...)
 end

diff --git a/src/evaluation/WorkspaceManager.jl b/src/evaluation/WorkspaceManager.jl
@@ -596,7 +596,8 @@ function move_vars(
     to_delete::Set{Symbol},
     methods_to_delete::Set{Tuple{UUID,FunctionName}},
     module_imports_to_move::Set{Expr},
-    invalidated_cell_uuids::Set{UUID};
+    invalidated_cell_uuids::Set{UUID},
+    keep_registered::Set{Symbol}=Set{Symbol}();
     kwargs...
     )
 
@@ -611,6 +612,7 @@ function move_vars(
             $methods_to_delete,
             $module_imports_to_move,
             $invalidated_cell_uuids,
+            $keep_registered,
         )
     end)
 end

diff --git a/src/runner/PlutoRunner.jl b/src/runner/PlutoRunner.jl
@@ -529,9 +529,6 @@ function run_expression(
     cell_published_objects[cell_id] = Dict{String,Any}()
 
     # reset registered bonds
-    for s in get(cell_registered_bond_names, cell_id, Set{Symbol}())
-        delete!(registered_bond_elements, s)
-    end
     cell_registered_bond_names[cell_id] = Set{Symbol}()
 
     # If the cell contains macro calls, we want those macro calls to preserve their identity,
@@ -683,6 +680,7 @@ function move_vars(
     methods_to_delete::Set{Tuple{UUID,Vector{Symbol}}},
     module_imports_to_move::Set{Expr},
     invalidated_cell_uuids::Set{UUID},
+    keep_registered::Set{Symbol},
 )
     old_workspace = getfield(Main, old_workspace_name)
     new_workspace = getfield(Main, new_workspace_name)
@@ -706,6 +704,10 @@ function move_vars(
         if (symbol ∈ vars_to_delete) || (symbol ∈ name_symbols_of_funcs_with_no_methods_left)
             # var will be redefined - unreference the value so that GC can snoop it
 
+            if haskey(registered_bond_elements, symbol) && symbol ∉ keep_registered
+                delete!(registered_bond_elements, symbol)
+            end
+
             # free memory for other variables
             # & delete methods created in the old module:
             # for example, the old module might extend an imported function:
@@ -2012,8 +2014,8 @@ struct Bond
     Bond(element, defines::Symbol) = showable(MIME"text/html"(), element) ? new(element, defines, Base64.base64encode(rand(UInt8,9))) : error("""Can only bind to html-showable objects, ie types T for which show(io, ::MIME"text/html", x::T) is defined.""")
 end
 
-function create_bond(element, defines::Symbol)
-    push!(cell_registered_bond_names[currently_running_cell_id[]], defines)
+function create_bond(element, defines::Symbol, cell_id::UUID)
+    push!(cell_registered_bond_names[cell_id], defines)
     registered_bond_elements[defines] = element
     Bond(element, defines)
 end
@@ -2051,10 +2053,10 @@ The second cell will show the square of `x`, and is updated in real-time as the
 macro bind(def, element)    
 	if def isa Symbol
 		quote
-            $(load_integrations_if_needed)()
+			$(load_integrations_if_needed)()
 			local el = $(esc(element))
-            global $(esc(def)) = Core.applicable(Base.get, el) ? Base.get(el) : $(initial_value_getter_ref)[](el)
-			PlutoRunner.create_bond(el, $(Meta.quot(def)))
+			global $(esc(def)) = Core.applicable(Base.get, el) ? Base.get(el) : $(initial_value_getter_ref)[](el)
+			PlutoRunner.create_bond(el, $(Meta.quot(def)), $(GiveMeCellID()))
 		end
 	else
 		:(throw(ArgumentError("""\nMacro example usage: \n\n\t@bind my_number html"<input type='range'>"\n\n""")))

diff --git a/test/Bonds.jl b/test/Bonds.jl
@@ -44,8 +44,7 @@ import Distributed
                 # 1
                 Cell("""
                 begin
-                    import AbstractPlutoDingetjes
-                    const APD = AbstractPlutoDingetjes
+                    import AbstractPlutoDingetjes as APD
                     import AbstractPlutoDingetjes.Bonds
                 end
                 """),
@@ -205,9 +204,13 @@ import Distributed
                 Cell("@bind pv4 PossibleValuesTest((x+1 for x in 1:10))"),
                 # 34
                 Cell("@bind pv5 PossibleValuesTest(1:10)"),
+
+                # 35 - https://github.com/fonsp/Pluto.jl/issues/2465
+                Cell(""),
+                Cell("@bind ts2465 TransformSlider()"),
+                Cell("ts2465"),
             ])
-
-
+
         function set_bond_value(name, value, is_first_value=false)
             notebook.bonds[name] = Dict("value" => value)
             Pluto.set_bond_values_reactive(; session=🍭, notebook, bound_sym_names=[name],
@@ -225,8 +228,7 @@ import Distributed
         @test notebook.cells[10].output.body == "missing"
         set_bond_value(:x_simple, 1, true)
         @test notebook.cells[10].output.body == "1"
-
-
+
         update_run!(🍭, notebook, notebook.cells)
 
         @test noerror(notebook.cells[1])
@@ -271,7 +273,7 @@ import Distributed
         @test noerror(notebook.cells[32])
         @test noerror(notebook.cells[33])
         @test noerror(notebook.cells[34])
-        @test length(notebook.cells) == 34
+        @test length(notebook.cells) == 37
 
 
         @test Pluto.possible_bond_values(🍭, notebook, :x_new) == [1,2,3]
@@ -326,8 +328,30 @@ import Distributed
         @test notebook.cells[25].output.body == "1"
         set_bond_value(:x_counter, 7, false)
         @test notebook.cells[25].output.body == "2"
-
-
+
+        # https://github.com/fonsp/Pluto.jl/issues/2465
+        update_run!(🍭, notebook, notebook.cells[35:37])
+
+        @test noerror(notebook.cells[35])
+        @test noerror(notebook.cells[36])
+        @test noerror(notebook.cells[37])
+        @test notebook.cells[37].output.body == "\"x\""
+        @test isempty(notebook.cells[35].code)
+
+        # this should not deregister the TransformSlider
+        setcode!(notebook.cells[35], notebook.cells[36].code)
+        setcode!(notebook.cells[36], "")
+
+        update_run!(🍭, notebook, notebook.cells[35:36])
+        @test noerror(notebook.cells[35])
+        @test noerror(notebook.cells[36])
+        @test notebook.cells[37].output.body == "\"x\""
+
+        set_bond_value(:ts2465, 2, false)
+        @test noerror(notebook.cells[35])
+        @test noerror(notebook.cells[36])
+        @test notebook.cells[37].output.body == "\"xx\""
+
         WorkspaceManager.unmake_workspace((🍭, notebook))
         🍭.options.evaluation.workspace_use_distributed = false