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

Better detection of forbidden recursive update steps #10

Open
dbuenzli opened this issue May 9, 2014 · 6 comments
Open

Better detection of forbidden recursive update steps #10

dbuenzli opened this issue May 9, 2014 · 6 comments

Comments

@dbuenzli
Copy link
Owner

dbuenzli commented May 9, 2014

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
Copy link

hhugo commented Jul 29, 2014

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
Copy link
Owner Author

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

@hhugo
Copy link

hhugo commented Jul 29, 2014

yes f update events/signals

@dbuenzli
Copy link
Owner Author

Sorry forbidden.

@hhugo
Copy link

hhugo commented Jul 29, 2014

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
Copy link
Owner Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants