Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

@app is scope leaky #223

Open
essenciary opened this issue Sep 21, 2023 · 1 comment
Open

@app is scope leaky #223

essenciary opened this issue Sep 21, 2023 · 1 comment

Comments

@essenciary
Copy link
Member

essenciary commented Sep 21, 2023

When we have something like this:

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

  # more code here
  • df is scoped inside the ReactiveModel and is not accessible at module scope
  • however, DATA leaks into the module scope -- which is surprising and not good for performance

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
Copy link
Member

hhaensel commented Oct 1, 2023

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
  1. 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants