dbuenzli / react

Declarative events and signals for OCaml
http://erratique.ch/software/react
ISC License
134 stars 14 forks source link

Better detection of forbidden recursive update steps #10

Open dbuenzli opened 10 years ago

dbuenzli commented 10 years ago

In general it is not allowed to update primitives from an update step. If this happens and two update steps get intermingled it seems we get assertions errors from react.ml which lead to believe there's a bug in react itself. Can we report better errors while keeping react free of any global data structure ?

Besides better guidelines should be provided on how to avoid that. I think the best way of handling this is through interaction with a monadic concurrency library. Instead of having side effecting events at the outputs of the reactive system we should convert occurences to futures and that future representing the occurence should always and unconditionally immediately defer/yield which will bring the update step to an end (rather than the update step potentially continuing to execute in the future value which may trigger primitive updates and blow the whole system).

hhugo commented 10 years ago

I've just hit Assert_failure src/react.ml,411,54 while updating a signal/event during a step.

I got around it using fix val iter : ('a -> unit) -> 'a event -> unit

let iter f e =
  React.E.fix (fun e' ->
      let _ = React.E.map f e' in
      e,()
    )

What are pro and cons of doing this compare to delaying the update with Lwt ?

dbuenzli commented 10 years ago

You mean f sends primitive events ? You should not do that either.

hhugo commented 10 years ago

yes f update events/signals

dbuenzli commented 10 years ago

Sorry forbidden.

hhugo commented 10 years ago

When saying You should not do that either, you mean this

let iter f_may_update_event e =
  let _ = React.E.map (fun x ->
    Lwt.async (fun () ->
      Lwt_js.sleel 0. >>= fun () ->
      f_may_update_event x
    )) e in
  ()

is forbidden ?

dbuenzli commented 10 years ago

No this should be right.

The rule is very simple: you just need to yield at the end of the update cycle if you want to update a primitive/create a new step. It is actually even better if you yield any side effects to the concurrency library, as this makes a good separation of concerns (purely functional reactive logic/effects on the world). I think that Lwt_react doesn't make that very clear and seems to provide tools to shoot yourself in the foot (or that I'm not smart enough to fully understand). Personally I have the feeling (to be confirmed yet) that something like that should be enough and the right way for integrating with a monadic concurrency library.