Bonsai (incorrectly) pruning updates in Bonsai.Edge?

[haoxuany]

Reproducible code:

let main () =
  let open Bonsai_web in
  let module N = Vdom.Node in
  let textcomp (fetch : (unit -> int Effect.t) Value.t) =
    let%sub status, set_status = Bonsai.state None in
    let%sub () = 
      Bonsai.Edge.lifecycle
        ~after_display:(
          let%map status = status
          and set_status = set_status
          and fetch = fetch in
          match status with
          | None ->
             let%bind.Effect result = fetch () in
             set_status (Some result)
          | _ -> Effect.return ()
        )
        ()
    in
    let%arr status = status in
    match status with
    | None -> N.text "Not fetched"
    | Some i -> N.text ("Fetched: " ^ (Int.to_string i))
  in
  let%sub number, set_number = Bonsai.state 0 in
  let%sub text = textcomp (Value.map ~f:(fun i -> fun () -> Effect.return i) number) in
  let%arr text = text and number = number and set_number = set_number in
  N.div [
      text;
      N.button
        ~attrs:[ Vdom.Attr.on_click (fun _ -> set_number (number + 1)) ]
        [ N.text (Int.to_string number) ];
    ]

This examples renders a text component and a button, and upon clicking the button, should in theory update the number state, along with the effect function that is passed for the text component to load. However, when running the example, clicking on the button updates the button but not the inner text component. (apparently those updates were completely pruned, even though the text here does show 0 which means that the lifecycle function did update the first time the component was created.)

Is this a bug within Bonsai or would you know if I'm doing something obviously wrong?

(I have a hunch that it might have to do with potentially equality comparison for Value but that's purely a shot in the dark, I'm not familiar with how Bonsai is written exactly).

(Note that this example is necessarily contrived so that it is minimally reproducible. In the actual code I have it'll actually take account of all scenarios here. The broader context is that I have these components that first performs some effect [almost always an rpc call], and instead of binding to receive the response, first renders some "loading" screen while the data is fetching, and then updates the ui after the the data is fetched.)

[TyOverby]

Sorry, just seeing this now! I think the issue might be that after_display runs on every frame and is wiping out the state that you're setting.

I think that if you replaced

      Bonsai.Edge.lifecycle
        ~after_display:(

with

      Bonsai.Edge.lifecycle
        ~on_activate:(

you'd get the desired behavior

[haoxuany]

Hi,

Thanks for the response! Unfortunately with that change the behavior is exactly the same.

I was briefly testing this to confirm some suspicions while reading some of bonsai code (mind, I've only looked at it very briefly), but as a shot in the dark I didn't quite understand what this was doing and if that was what's causing the issue:

https://github.com/janestreet/bonsai/blob/81f3b276269b548549872f77de5af01352cf90b8/src/lifecycle.ml#L38

(my suspicions being that this equality relying on how the js compiler handles equality for functions, however, I wasn't able to produce a case that confirms my suspicions, since I need to write a Effect.t that's dependent on a value and that never gets pruned).

I can take another look when I have more time, but would you have any thoughts? Thanks.

[TyOverby]

Ah! I sat down and got the code running, and found the actual issue! After the first after_display callback is invoked, status is Some 0, so the | _ -> Effect.return () branch is hit for every subsequent frame.

I rewrote your example to make it a bit more obvious what's happening. Aside from some slight style tweaks to fit my taste, my two main changes are:

1. adds a delay to the "fetching".
2. adds a call to Effect.print_s in the Some _ case in the handler

open! Core
open! Bonsai_web
open! Bonsai.Let_syntax

let do_some_compute =
  let open Async_kernel in
  let f i =
    let%map () = after (Time_ns.Span.of_sec (Random.float 2.0)) in
    i * 2
  in
  Effect.of_deferred_fun f
;;

let main =
  let%sub number, set_number = Bonsai.state 0 in
  let%sub text =
    let%sub status, set_status = Bonsai.state None in
    let%sub after_display =
      let%arr status and set_status and number = number in
      match status with
      | None ->
        let%bind.Effect result = do_some_compute number in
        set_status (Some result)
      | Some _ -> Effect.print_s [%message "status is already set; ignoring" [%here]]
    in
    let%sub () = Bonsai.Edge.lifecycle ~after_display () in
    match%arr status with
    | None -> Vdom.Node.text "Not fetched"
    | Some i -> Vdom.Node.textf "Fetched: %d" i
  in
  let%arr text = text
  and number = number
  and set_number = set_number in
  Vdom.Node.div
    [ text
    ; Vdom.Node.button
        ~attrs:[ Vdom.Attr.on_click (fun _ -> set_number (number + 1)) ]
        [ Vdom.Node.textf "%d" number ]
    ]
;;

let () = Bonsai_web.Start.start main

If you run this, you'll see the following:

1. Not fetched [0]
2. 2 seconds later: Fetched 0 [0]
3. Chrome's dev tools console begins spamming the "status is already set" message once per frame forever.

Maybe you expected that updating the number state would cause the status state to be reset, but this is not how Bonsai works. After the computation graph is initialized, it never gets reset.

Now, as for how to fix this, well there's a simple but naive solution that just involves changing Bonsai.Edge.lifecycle ~on_display to Bonsai.Edge.on_change:

open! Core
open! Bonsai_web
open! Bonsai.Let_syntax

let do_some_compute =
  let open Async_kernel in
  let f i =
    let%map () = after (Time_ns.Span.of_sec (Random.float 2.0)) in
    i * 2
  in
  Effect.of_deferred_fun f
;;

let main =
  let%sub number, set_number = Bonsai.state 0 in
  let%sub text =
    let%sub status, set_status = Bonsai.state None in
    let%sub callback =
      let%arr set_status = set_status in
      fun number ->
        let%bind.Effect result = do_some_compute number in
        set_status (Some result)
    in
    let%sub () = Bonsai.Edge.on_change ~equal:[%equal: int] number ~callback in
    match%arr status with
    | None -> Vdom.Node.text "Not fetched"
    | Some i -> Vdom.Node.textf "Fetched: %d" i
  in
  let%arr text = text
  and number = number
  and set_number = set_number in
  Vdom.Node.div
    [ text
    ; Vdom.Node.button
        ~attrs:[ Vdom.Attr.on_click (fun _ -> set_number (number + 1)) ]
        [ Vdom.Node.textf "%d" number ]
    ]
;;

let () = Bonsai_web.Start.start main

But this has it's own problems! Because the responses from the fake request can overlap and come back in a different order than they were sent in, you can mash on the button and end up in a steady state where the final response does not correspond to the most recent request.

This race condition is significantly harder to protect against, and would probably require seqnums to track subsequent requests and responses, but the good news is that I already implemented this logic in Bonsai.Edge.Poll.effect_on_change! You can use it like this:

open! Core
open! Bonsai_web
open! Bonsai.Let_syntax

let do_some_compute =
  let open Async_kernel in
  let f i =
    let%map () = after (Time_ns.Span.of_sec (Random.float 2.0)) in
    i * 2
  in
  Effect.of_deferred_fun f
;;

let main =
  let%sub number, set_number = Bonsai.state 0 in
  let%sub text =
    let%sub status =
      Bonsai.Edge.Poll.effect_on_change
        Bonsai.Edge.Poll.Starting.empty
        ~effect:(Value.return do_some_compute)
        ~equal_input:[%equal: int]
        number
    in
    match%arr status with
    | None -> Vdom.Node.text "Not fetched"
    | Some i -> Vdom.Node.textf "Fetched: %d" i
  in
  let%arr text = text
  and number = number
  and set_number = set_number in
  Vdom.Node.div
    [ text
    ; Vdom.Node.button
        ~attrs:[ Vdom.Attr.on_click (fun _ -> set_number (number + 1)) ]
        [ Vdom.Node.textf "%d" number ]
    ]
;;

let () = Bonsai_web.Start.start main

[haoxuany]

Hi,

Thanks for the extremely detailed and helpful explanation! Really appreciate taking the time for this!

Maybe you expected that updating the number state would cause the status state to be reset, but this is not how Bonsai works. After the computation graph is initialized, it never gets reset.

Yeah, I think that was precisely my confusion. In the original code, textcomp has the type of a function, and hence just looking at the type I assumed that it will work exactly like ML functions when called with different values.

Out of sheer curiosity, would you be able to quickly explain how this works beneath the hood? I presume that let%sub probably does a lot more than it meets the eye, since the code above using let%sub text = textcomp (Value.map ~f:(fun i -> fun () -> Effect.return i) number) behaves as if the textcomp variable is directly substituted with the function binding and then beta reduced (which I thought wasn't the most intuitive when taking account of effects like Bonsai.state).

[TyOverby]

Under the hood, Bonsai is really a compiler that takes a graph built out of Computation.t and Value.t and lowers it to a dynamic Incremental graph. This means that the Bonsai graph is that it's fully static and can't change itself at runtime.

This is enforced by the 'a Value.t type, which isn't a 'a; it's closer a node in an AST that will, after compilation, incrementally produce values of type 'a.

[haoxuany]

Ah see, I think that makes sense. Thank you very much! I'll close this out.