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

allow cyclic reactors as long as equilibrium is reached #57

Closed
wants to merge 1 commit into from

Conversation

wvanderdeijl
Copy link
Contributor

This has been discussed before in #22 and #8
Hopefully this proposed solution is something you can accept.

I'll try to describe the (simplified) use case why we need this:

  1. we have a (client side) atom that defines which graphs (aka data sets) are in memory. This is initialised with an initial set.
  2. we also have queries on this client side data set (aka derivables) that define which data sets should be available on the client
  3. when the query for data sets returns new results, we fetch these graphs/data sets from the server and add these to the atom from the first step (a cyclic reactor)
  4. once we add more data sets to the atom the derivable and reactor might trigger again as more data sets are needed. After 1 or 2 iterations this reaches an equilibrium

@ds300
Copy link
Owner

ds300 commented Mar 6, 2017

There is nothing wrong with the workflow you describe. That is an asynchronous cycle because it involves going away and querying another system like the server in your example. However, even when the query can be done synchronously, for example if you have a local cache, DerivableJS requires that you insert an asynchronous step in there using setImmediate or equivalent. This should be easy to do in all cases.

It might seem like this is required because of implementation details, but in fact it is because synchronous cyclical reactions are dangerous. They allow inconsistency to creep in to the state graph, and I can't let that kind of thing slide, sorry. You've already read my other explanations of this so I won't go into more detail, and I'll have to decline this PR. Thanks for the contribution anyway. It is good to know that people are making use of the library :)

@ds300 ds300 closed this Mar 6, 2017
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

Successfully merging this pull request may close these issues.

None yet

2 participants