GenieFramework / Stipple.jl

The reactive UI library for interactive data applications with pure Julia.
MIT License
324 stars 27 forks source link

@app is scope leaky #223

Open essenciary opened 1 year ago

essenciary commented 1 year ago

When we have something like this:

@app begin
  DATA = sort!(DataFrame(rand(10, 2), ["x1", "x2"]))
  @out df = DATA

  # more code here

I hoped it would be straightforward and wrapped the @app macro output into a let ... end block but it turns out that handlers are in a different scope and stuff breaks as handlers can't access the vars in the scope introduced by the let block:

ERROR: UndefVarError: `DATA` not defined
Stacktrace:
 [1] var"##Main_ReactiveModel!#292"()
   @ Main ./util.jl:567
 [2] #invokelatest#2
   @ ./essentials.jl:819 [inlined]
 [3] invokelatest
   @ ./essentials.jl:816 [inlined]
 [4] |>
   @ ./operators.jl:907 [inlined]
 [5] init(::Type{Main_ReactiveModel}; vue_app_name::String, endpoint::String, channel::Any, debounce::Int64, transport::Module, core_theme::Bool)
   @ Stipple ~/.julia/dev/Stipple/src/Stipple.jl:430
 [6] init
   @ ~/.julia/dev/Stipple/src/Stipple.jl:419 [inlined]
 [7] init_from_storage(::Type{Main_ReactiveModel}; channel::Nothing, kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
   @ Stipple.ModelStorage.Sessions ~/.julia/dev/Stipple/src/ModelStorage.jl:15
 [8] init_from_storage(::Type{Main_ReactiveModel})
   @ Stipple.ModelStorage.Sessions ~/.julia/dev/Stipple/src/ModelStorage.jl:11
 [9] top-level scope
   @ ~/.julia/dev/Stipple/src/ReactiveTools.jl:607
in expression starting at /Users/adrian/.julia/dev/StippleUI/demos/tables/app.jl:40

Well, other things needing patching before getting to this point so it looks like scoping is a bit messy.

hhaensel commented 1 year ago

If I'm right, this is due to historic devolopment of the ReactiveTools API. We started off with the desire to dynamically add variables to an existing model without a surrounding @appmacro. This was solved by a module-related storage. Then we aimed to support also ancient model syntax and relied on the model fields rather than on a storage. But this needed an evaluation parts of the code before the calculation of the macro expressions. That has quite some pitfalls and I'd probably design the API - at least the app macro - a bit differently now with all the knowledge that I gained during programming. Things would be a bit easier, if we'd split @app and @handlers as we had done in the past. Or we would need to introduce a storage for named models, which I had proposed initially but then somehow gave up. The point is that for the correct rewriting of handlers we need to know the full set of variable names of the model. So we either need to have defined the model type before writing the handlers, or we need to write the storage also for named models to keep track of the variable names. So cleaning up model scope is probably related with rewriting non-negligible parts of the macro code.

Having said all this, constructions like the one above

@app begin
  DATA = sort!(DataFrame(rand(10, 2), ["x1", "x2"]))
  @out df = DATA
   <...>
end

are currently not handled correctly. There are two ways to achieve that sort of behaviour:

1) include extra code in a manual route, e.g.

route("/") do
    model = @init
    DATA = sort!(DataFrame(rand(10, 2), ["x1", "x2"]))
    model.df[!] = DATA # The `[!]` is only needed if you have a handler attached to `df`, otherwise `model.df` is sufficient
    page(model, ui) |> html
    @onchange isready notify(df) # to transfer the updated value of the model to the front-end after the setup
end

2) include the code in the isready handler

@app begin
    @out df = DataFrame()
    <...>
    @onchange isready begin
        DATA = sort!(DataFrame(rand(10, 2), ["x1", "x2"]))
        df = DATA # here it is ok to trigger a handler, because everything is already setup, so no `[!]` is necessary
    end

end