RList.cons silently fails in JS

Ecosystem
,
1

After some time, RLists don’t update anymore in a browser (tested on Edge and Firefox). There’s no error in the console, so I don’t know what went wrong. Any idea?

Here’s a reproducer:

open Js_of_ocaml
open Js_of_ocaml_tyxml.Tyxml_js
open ReactiveData
open Html

let app () =
  let l, h = RList.create [] in
  let out = l |> RList.map (fun x -> li [txt x]) |> R.Html.ul in
  let counter = ref 0 in
  div [
    button [txt "append"] ~a:[a_onclick (fun _ ->
      RList.cons (string_of_int !counter) h;
      incr counter;
      false
    )];
    out;
  ]


let _ =
  let open Dom_html in
  Dom_events.listen document Event.domContentLoaded (fun _ _ ->
    let add () =
      let elt = (To_dom.of_div (app ()) :> Dom.node Js.t) in
      ignore @@ document##.body##appendChild elt
    in
    add ();
    add ();
    true
  )

Thanks!

2

I’m not familiar with Rlist but since 4.0.1 js_of_ocaml has a proper implementation of weak reference semantics.

So you may now have to keep references on certain root values you used to simply declare and ignore to avoid them being gc’d.

1 Like
3

If so, what should I keep a reference to? It still fails when app, l, and h are global:

let l, h = RList.create []

let app =
  let out = l |> RList.map (fun x -> li [txt x]) |> R.Html.ul in
  let counter = ref 0 in
  div [
    button [txt "append"] ~a:[a_onclick (fun _ ->
      RList.cons (string_of_int !counter) h;
      incr counter;
      false
    )];
    out;
  ]
4

I’ve followed its execution using a debugger. The internal list gets updated, but DOM nodes aren’t. I guess this is related to this comment in tyxml_js.ml…

1 Like
5

I’ve tried fixing the aforementioned comments in this commit, it still doesn’t work. I don’t know why: the same technique works in this sample CLI program…

Any idea?

6

Maybe aggressive dead code elimination ? Try to make the closure go through Sys.opaque_indentity.

7

Like this?

-    ignore (React.E.retain (event nodes) (fun () -> ignore s))
+    ignore (React.E.retain (event nodes) (Sys.opaque_identity (fun () -> ignore s)))

It has no effect. When digging up the output JS, it looks the same with and without using Sys.opaque_identity:

    function update_children(dom,nodes)
     {removeChildren(dom);
      var _j_=0;
      function _k_(param,msg)
       {if(0 === msg[0])
        ;} //...
      caml_call3(ReactiveData[1][11],_k_,nodes,_j_);
      function _l_(param){return 0}
      var _m_=caml_call1(ReactiveData[1][13],nodes);
      caml_call2(React[1][3],_m_,_l_);
      return 0}
8

Are you using the latest js_of_ocaml? Recently a bug was fixed involving overly zealous dead code elimination: caml_js_get and caml_js_delete (can) have side-effects by alainfrisch · Pull Request #1311 · ocsigen/js_of_ocaml · GitHub. The fix has been included in the latest release of jsoo.

Cheers,
Nicolas

9

I compile my sample program in an up-to-date tree of js_of_ocaml. I’m reinstalling the latest version with opam, just in case… Thanks for the heads-up!

10

(fun () -> ignore s) is equivalent to (fun () -> ()) and is not enough to keep a reference to s.

The following should do the trick

-    let _s : unit React.S.t = fold (fun () msg -> merge_msg dom msg) nodes () in
+    let s : unit React.S.t = fold (fun () msg -> merge_msg dom msg) nodes () in
+    let (`R _) =
+      React.E.retain (event nodes) (fun () -> ignore (React.S.map (fun _ -> ()) s))
+    in
11

Unfortunately, it still doesn’t. I had resorted to the same trick there and using ignore was enough for the thunk to capture the variable: does it indicate that the issue is related to js_of_ocaml optimizations?

12

Are you sure you’re not hitting something else ? React.*.retain raises Invalid_argument on constant signal and “never” event.

Try this


            let (`R _) =
              try React.S.retain a (fun () -> ignore (React.S.map (fun _ -> ()) s))
              with Invalid_argument _ -> `R (fun () -> ())
            in

@dbuenzli, Is there is a better way than catching Invalid_argument ?

13

I guess I would get an error in the browser’s console (there’s nothing). Besides, RList.create doesn’t use never and returns a Mutable list, so I don’t see any problem there.

14

It doesn’t seem so :frowning: S.is_const and E.is_never could be added, please file an issue if you want it.

15

Oops, I should not let library functions crash on constant lists…

16

Thinking about it more, retain is probably not enough. We probably want to hold on the signal as long as the the dom element is alive

17

I’ve pushed a semi-automated reproducer, so we don’t have to repeatedly press a button anymore. It looks time-dependent; that’s consistent with what I read about WeakRef on MDN.

18

In Brr’s reactive dom I use mutation observers for gc’ing signal and event observers.

19

The Reactive dom demo on ocsigen.org is affected by the same issue.

20

Would you mind opening an issue on js_of_ocaml GitHub page