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

Blink: output doesn't update with more than one observable #344

Closed
JobJob opened this issue Nov 27, 2019 · 7 comments
Closed

Blink: output doesn't update with more than one observable #344

JobJob opened this issue Nov 27, 2019 · 7 comments

Comments

@JobJob
Copy link
Member

JobJob commented Nov 27, 2019

Managed to whittle this down to a very simple repro. At the REPL:

using Blink, Interact
s = slider(1:10)
# ui = vbox(s, observe(s)); # updates fine when slider is moved
ui = vbox(s, observe(s), observe(s)); # neither outputs update when slider is moved
w = Window()
body!(w, ui)

When the slider is moved the observe(s) outputs don't update. The value of s[] is in sync with the slider though in Julia.

The same example in Jupyter Notebook (without the last two lines) works fine, it also works with Mux.

This isn't just happening in this contrived example. The actual code that I ran into this with has a PlotlyJS plot between the slider and some output.

JuliaGizmos/WebIO.jl#357 might also be related to this.

How do you even see what messages get sent to the Blink Window in order to debug this? Does this update happen on the js side, or is it slider->julia->window? What code should I look at to get started trying to fix this? It's been a while...


This is with Julia 1.1.0

(v1.1) pkg> st --manifest WebIO Blink Interact
    Status `~/.julia/environments/v1.1/Manifest.toml`
  [bf4720bc] AssetRegistry v0.1.0
  [9e28174c] BinDeps v0.8.10
  [ad839575] Blink v0.12.0
  [70588ee8] CSSUtil v0.1.0
  [34da2185] Compat v2.2.0
  [de31a74c] FunctionalCollections v0.5.0
  [c601a237] Interact v0.10.3
  [d3863d7c] InteractBase v0.10.3
  [97c1335a] JSExpr v0.5.1
  [682c06a0] JSON v0.21.0
  [bcebb21b] Knockout v0.2.3
  [50d2b5c4] Lazy v0.13.2
  [1914dd2f] MacroTools v0.5.2
  [ffc61752] Mustache v0.5.13
  [a975b10e] Mux v0.7.0
  [510215fc] Observables v0.2.3
  [bac558e1] OrderedCollections v1.1.0
  [189a3867] Reexport v0.2.0
  [ae029012] Requires v0.5.2
  [0f1e0344] WebIO v0.8.11
  [104b5d7c] WebSockets v1.5.2
  [cc8bc4a8] Widgets v0.6.2
  [2a0f44e3] Base64
  [8ba89e20] Distributed
  [56ddb016] Logging
  [44cfe95a] Pkg
  [9a3f8284] Random
  [6462fe0b] Sockets
  [cf7118a7] UUIDs
@JobJob
Copy link
Member Author

JobJob commented Nov 28, 2019

I added some debugging output on the Julia side of WebIO and tracked this down to something going wrong on the isopen call here (probably a deadlock):

I noticed that Blink's isopen changed slightly when the webio integration got moved to Blink.jl previously it was called on the Page (BlinkConnection defined here)

And now it's called on the Window (WebIOBlinkComm defined here)

So I made the obvious change, and it works:

Base.isopen(comm::WebIOBlinkComm) = active(comm.window.content)

I have no idea what the difference is between those two, or the exact cause of the problem, but it fixes my cases, and seems reasonable.

Will PR to Blink if everyone/anyone's happy? (besides me, I'm already happy 😄)

@JobJob
Copy link
Member Author

JobJob commented Dec 1, 2019

Just checked and it also fixes JuliaGizmos/WebIO.jl#357 for me.

It'd be good to have a test for this in WebIO, I saw that most of test/communication.jl is commented out. oh I see there's test/multiple-connections.jl

@rafaqz
Copy link

rafaqz commented Apr 10, 2020

I'm still experiencing this. Observables holding an image update in atom or served with mux, but don't update with a Blink.jl electron window. Any ideas?

@JobJob
Copy link
Member Author

JobJob commented Apr 13, 2020

Ahh bummer. @rafaqz does add Blink#jb/fix-isopen from the package REPL fix this for you?

I figured this had probably been fixed in @travigd's recent-ish comms overhaul so didn't follow this up. (Aside: commanding work in this org btw @travigd ❤️)

Anyway, @travigd was there a reason to change the Blink isopen call from

Base.isopen(comm::WebIOBlinkComm) = active(comm.window.content)

to

Base.isopen(comm::WebIOBlinkComm) = active(comm.window)

when the webio integration got moved to Blink.jl?

If not I can PR to Blink from add my branch there.

@twavv
Copy link
Member

twavv commented Apr 13, 2020

was there a reason to change the Blink isopen call

I don't think so, though I'm curious why active(window) and active(window.content) are different.

Feel free to make the PR. :^)

commanding work in this org btw @travigd ❤️

Thanks! I just wish I had more time to dedicate - clearly there are still quite a few issues around the Gizmos ecosystem, but $DAYJOB is eating up all of my time.

@rafaqz
Copy link

rafaqz commented Apr 14, 2020

@JobJob add Blink#jb/fix-isopen fixes the problem for me.

Would be great to get your changes into master for Blink.jl.

@JobJob
Copy link
Member Author

JobJob commented Apr 16, 2020

Thanks for checking @rafaqz


Thanks! I just wish I had more time to dedicate - clearly there are still quite a few issues around the Gizmos ecosystem, but $DAYJOB is eating up all of my time.

😄 Know the feeling very well

@twavv twavv closed this as completed Apr 17, 2020
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

3 participants