SimonDanisch / Bonito.jl

Serving JS to the browser
MIT License
204 stars 29 forks source link

Bonito Slider ignores Observable's element type #249

Open francoislaurent opened 3 weeks ago

francoislaurent commented 3 weeks ago

The value attribute of a DOM.input(; type="range") and the index attribute of a Slider are supposed to be of type Observable{Int}, but listeners receive an UInt8. I would expect they receive an Int instead.

This might be a bug in Observables.jl, but my experience with observables has been fairly stable so far. Type conversion systematically happens, or Julia complains.

My MRE is as follows:

using Observables
using Bonito

index1 = Observable{Int}(1)

oninput = js"(event) => { $index1.notify(parseInt(event.srcElement.value)) }"

slider1 = DOM.input(; type="range", min=1, max=10, value=index1, step=1, oninput=oninput)

on(index1) do i
    @assert i isa Int "Expected Int, got $(typeof(i))"
end

# type conversation happens on a normal setindex!
index1[] = UInt8(1) # no error
@assert eltype(index1) === Int

slider2 = Slider(1:10)

index2 = slider2.index

@assert eltype(index2) === Int

on(index2) do i
    @assert i isa Int "Expected Int, got $(typeof(i))"
end

App() do session
    Bonito.jsrender(session, DOM.div(slider1, slider2))
end

This shows a webpage with two sliders. It also ensures the observables (index1 and index2)'s element type is Int and registers listeners that test whether the type of the input value is an Int as expected.

Clicking on any of the sliders makes the test fail and generates the exact same error message (appart from one line number):

┌ Warning: error while processing received msg
│   exception =
│    AssertionError: Expected Int, got UInt8
│    Stacktrace:
│      [1] (::var"#1#2")(i::UInt8)
│        @ Main ~/Projects/MREs/BonitoSliderCorruptsObservable/mre.jl:11
│      [2] #invokelatest#2
│        @ ./essentials.jl:892 [inlined]
│      [3] invokelatest
│        @ ./essentials.jl:889 [inlined]
│      [4] update_nocycle!(obs::Observable{Int64}, value::Any)
│        @ Bonito ~/.julia/packages/Bonito/1lY3Q/src/rendering/observables.jl:31
│      [5] #invokelatest#2
│        @ ./essentials.jl:892 [inlined]
│      [6] invokelatest
│        @ ./essentials.jl:889 [inlined]
│      [7] process_message(session::Session{WebSocketConnection}, bytes::Vector{UInt8})
│        @ Bonito ~/.julia/packages/Bonito/1lY3Q/src/serialization/protocol.jl:36
│      [8] run_connection_loop(server::Server, session::Session{WebSocketConnection}, connection::WebSocketConnection)
│        @ Bonito ~/.julia/packages/Bonito/1lY3Q/src/connection/websocket.jl:100
│      [9] (::WebSocketConnection)(context::@NamedTuple{routes::Bonito.HTTPServer.Routes, application::Server, request::HTTP.Messages.Request, match::String}, websocket::HTTP.WebSockets.WebSocket)
│        @ Bonito ~/.julia/packages/Bonito/1lY3Q/src/connection/websocket.jl:135
│     [10] apply_handler(::WebSocketConnection, ::@NamedTuple{routes::Bonito.HTTPServer.Routes, application::Server, request::HTTP.Messages.Request, match::String}, ::HTTP.WebSockets.WebSocket)
│        @ Bonito.HTTPServer ~/.julia/packages/Bonito/1lY3Q/src/HTTPServer/implementation.jl:83
│     [11] delegate(routes::Bonito.HTTPServer.Routes, application::Server, request::HTTP.Messages.Request, args::HTTP.WebSockets.WebSocket)
│        @ Bonito.HTTPServer ~/.julia/packages/Bonito/1lY3Q/src/HTTPServer/implementation.jl:102
│     [12] #17
│        @ ~/.julia/packages/Bonito/1lY3Q/src/HTTPServer/implementation.jl:156 [inlined]
│     [13] upgrade(f::Bonito.HTTPServer.var"#17#19"{Server, HTTP.Streams.Stream{HTTP.Messages.Request, HTTP.Connections.Connection{Sockets.TCPSocket}}}, http::HTTP.Streams.Stream{HTTP.Messages.Request, HTTP.Connections.Connection{Sockets.TCPSocket}}; suppress_close_error::Bool, maxframesize::Int64, maxfragmentation::Int64, nagle::Bool, quickack::Bool, kw::@Kwargs{binary::Bool})
│        @ HTTP.WebSockets ~/.julia/packages/HTTP/sJD5V/src/WebSockets.jl:456
│     [14] upgrade
│        @ ~/.julia/packages/HTTP/sJD5V/src/WebSockets.jl:425 [inlined]
│     [15] stream_handler(application::Server, stream::HTTP.Streams.Stream{HTTP.Messages.Request, HTTP.Connections.Connection{Sockets.TCPSocket}})
│        @ Bonito.HTTPServer ~/.julia/packages/Bonito/1lY3Q/src/HTTPServer/implementation.jl:155
│     [16] #invokelatest#2
│        @ ./essentials.jl:892 [inlined]
│     [17] invokelatest
│        @ ./essentials.jl:889 [inlined]
│     [18] (::Bonito.HTTPServer.var"#27#29"{Server})(stream::HTTP.Streams.Stream{HTTP.Messages.Request, HTTP.Connections.Connection{Sockets.TCPSocket}})
│        @ Bonito.HTTPServer ~/.julia/packages/Bonito/1lY3Q/src/HTTPServer/implementation.jl:287
│     [19] #invokelatest#2
│        @ ./essentials.jl:892 [inlined]
│     [20] invokelatest
│        @ ./essentials.jl:889 [inlined]
│     [21] handle_connection(f::Function, c::HTTP.Connections.Connection{Sockets.TCPSocket}, listener::HTTP.Servers.Listener{Nothing, Sockets.TCPServer}, readtimeout::Int64, access_log::Nothing)
│        @ HTTP.Servers ~/.julia/packages/HTTP/sJD5V/src/Servers.jl:469
│     [22] (::HTTP.Servers.var"#16#17"{Bonito.HTTPServer.var"#27#29"{Server}, HTTP.Servers.Listener{Nothing, Sockets.TCPServer}, Set{HTTP.Connections.Connection}, Int64, Nothing, ReentrantLock, Base.Semaphore, HTTP.Connections.Connection{Sockets.TCPSocket}})()
│        @ HTTP.Servers ~/.julia/packages/HTTP/sJD5V/src/Servers.jl:401
└ @ Bonito ~/.julia/packages/Bonito/1lY3Q/src/connection/websocket.jl:103
  [824d6782] Bonito v3.1.2
  [510215fc] Observables v0.5.5
SimonDanisch commented 3 weeks ago

Yeah this is due to our serialization protocol... I tried once to recover original Julia types, but it was a bit hairy, so I decided this should be good enough. If you really need an Int, one should just convert. We could also convert eagerly in user facing some places like slider

frankier commented 1 week ago

This also affects this example: https://simondanisch.github.io/Bonito.jl/stable/widgets.html#WidgetsBase.NumberInput-widgets where there will be a MethodError since the type will not always be Float64.