GenieFramework / Stipple.jl

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

global model stopped working #249

Closed yakir12 closed 8 months ago

yakir12 commented 8 months ago

The following simple example doesn't work with a global model (pressing the Hi button doesn't print anything):

using GenieFramework
@genietools

@app MyApp begin
    @in say_hi = false
    @onchange say_hi println("Hi!")
end myhandlers

ui() = button("Hi", @click("say_hi = !say_hi"))

global model = init(MyApp, debounce = 0) |> myhandlers

route("/") do
    page(model, ui) |> html
end

up()

I'm on:

(tmp) pkg> st
Status `~/tmp/Project.toml`
  [a59fdf5c] GenieFramework v1.26.8

julia> versioninfo()
Julia Version 1.10.0
Commit 3120989f39b (2023-12-25 18:01 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 8 × Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, skylake)
  Threads: 11 on 8 virtual cores
yakir12 commented 8 months ago

I've managed to get it to work again by moving the definition and assignment of the global model variable into the route function-body:

using GenieFramework
@genietools

@app MyApp begin
    @in say_hi = false
    @onchange say_hi println("Hi!")
end myhandlers

ui() = button("Hi", @click("say_hi = !say_hi"))

route("/") do
    global model = init(MyApp, debounce = 0) |> myhandlers
    page(model, ui) |> html
end

up()

Not sure why, what, how... I'll leave this open for reference.

hhaensel commented 8 months ago

I found out the same thing. That's totally strange and there may be situations where you don't want to rebuild your model upon reload of the page. So we have to tackle this. Thanks for bringing it up here.

hhaensel commented 8 months ago

It must have to do with channels, as subscription doesn't succeed

hhaensel commented 8 months ago

It's definitely the channels, that get confused. Calling init outside of the route doesn't set up the channels.

@essenciary any idea why this could be?

hhaensel commented 8 months ago

after calling init() globally:

julia> Genie.channels()
1-element Vector{Genie.Router.Channel}:
 [WS] /autoreload/subscribe => #7 | :autoreload_subscribe

after calling init within route()

julia> Genie.channels()
6-element Vector{Genie.Router.Channel}:
 [WS] /SXZAKEVHLPKYSQVNKWOYQXUDXCGMCFVL/events => #52 | :sxzakevhlpkysqvnkwoyqxudxcgmcfvl_events
 [WS] /SXZAKEVHLPKYSQVNKWOYQXUDXCGMCFVL/keepalive => #51 | :sxzakevhlpkysqvnkwoyqxudxcgmcfvl_keepalive
 [WS] /SXZAKEVHLPKYSQVNKWOYQXUDXCGMCFVL/watchers => #50 | :sxzakevhlpkysqvnkwoyqxudxcgmcfvl_watchers
 [WS] /SXZAKEVHLPKYSQVNKWOYQXUDXCGMCFVL/unsubscribe => #27 | :sxzakevhlpkysqvnkwoyqxudxcgmcfvl_unsubscribe
 [WS] /SXZAKEVHLPKYSQVNKWOYQXUDXCGMCFVL/subscribe => #26 | :sxzakevhlpkysqvnkwoyqxudxcgmcfvl_subscribe
 [WS] /autoreload/subscribe => #7 | :autoreload_subscribe
hhaensel commented 8 months ago

How can a version change of Julia change lead to such strange behaviour ... Probably the new syntax parser. We need to find out and file an issue with Julia probably

essenciary commented 8 months ago

It's definitely the channels, that get confused. Calling init outside of the route doesn't set up the channels.

@essenciary any idea why this could be?

Yes, it's by design. It's part of the recent fix that stops new channels being created with every page refresh. A channel that is not part of a request is not bound to any user so it's useless.

yakir12 commented 8 months ago

When I played around with this (albeit not as thoroughly as you), I found out that changing Julia to 1.9.4 didn't help. Are you sure that this anomaly is absent in 1.9.4?

essenciary commented 8 months ago

What's the purpose of the global model? To share the same model between all the users? For that we can set the channel explicitly via ENV["CHANNEL__"].

I'll take a look to see how I can make the global model work as well.

yakir12 commented 8 months ago

What's the purpose of the global model? To share the same model between all the users? For that we can set the channel explicitly via ENV["CHANNEL__"].

That sounds interesting!

yakir12 commented 8 months ago

What's the purpose of the global model?

In my specific use-case, I use notify inside a route to tell the user if and why an upload failed, as well as update a model variable when the upload succeeds.I also use the global model to update the loading btn indicator. I'm not sure how to do that without a global model.

I'm using Genie as an interface between what ultimately is only one user at a time and some hardware. There should only be one user, and there is only one hardware the user can interact with. It's an arena with LEDs and an RPI camera (the RPI is the server running Genie), and the user controls those things to run behavioral experiments on dung beetles.

A global model makes sense in this case, but I'm completely open to suggestions and improvements if you think there are better ways to set this up.

essenciary commented 8 months ago

@yakir12 Then it doesn't sound like you would need global models (which should be avoided unless really necessary). This is the recommended way of building apps currently and this is what your failing MWE looks like refactored for the current API:

using GenieFramework
@genietools

@app begin
    @in say_hi = false
    @onchange say_hi println("Hi!")
end

ui() = button("Hi", @click("say_hi = !say_hi"))
@page("/", ui)

up()

Works well.

image
yakir12 commented 8 months ago

I can see https://github.com/GenieFramework/StippleUI.jl/commit/c2f47b383e86cfd628f699c765d78d40a7c6f9f2, so I think I'll wait for that and then try it out. I think that that would be the last thing I'm using a global model for and then I'll be globally free! Thanks!

essenciary commented 8 months ago

@yakir12 This is solved in Stipple 0.27.29 -- although it's still breaking.

You will need to update the code to pass an extra argument to Stipple.init, namely always_register_channels = true.

Ex:

init(MyApp; debounce = 0, always_register_channels = true) 
essenciary commented 8 months ago

Alternatively, the option can be set up globally:

Stipple.ALWAYS_REGISTER_CHANNELS[] = true
yakir12 commented 8 months ago

Thank you @essenciary! Just to be clear, by setting always_register_channels to true one makes sure that all users share the same instance of the model...? (please let me know if you'd rather I post these kind of questions elsewhere, discord)

essenciary commented 8 months ago

No, this is a bit of an ugly quick hack to enable a feature that global models need - which is eager registration of some channels upon model init. Basically this makes your previously breaking example code, work.

But I think overall this needs more work as indeed, it's confusing and I don't think users would understand when to set it. Maybe @hhaensel has some idea.

Otherwise @hhaensel we can have this set to true by default. It's a bit ugly cause for "regular" use we'll have this "init" channel which is not used by any connection. But might be the least problematic issue.

essenciary commented 8 months ago

@yakir12 @hhaensel in another twist of events I've released a new patch to make this enabled/true by default as more people reported breakages. Which means that the config no longer needs to be explicitly set for backwards compatibility.

yakir12 commented 8 months ago

(for what it's worth, I'm committed to avoiding using a global model, and the last frontier is the file uploader. I'm running into some minor trouble with that (reported on discord), but once I figure out how to solve that, I'll hopefully won't need to use a global model).

essenciary commented 8 months ago

Do you have a link to the discord thread? Things can get busy there...

yakir12 commented 8 months ago

Yeah https://discord.com/channels/774897545717219328/1194948279587307600/1194948279587307600 but you shouldn't need to respond to individual inquiries like that... Though I'd be grateful !!

essenciary commented 8 months ago

I replied - not my area of expertise I'm afraid so I can't provide a quick answer.