Closed mainej closed 2 years ago
(sorry for the late response)
Hey @mainej, I just read over your motivation/description of this change, as well as the code. This sounds like quite interesting and could help untangle the dark corner of this lib. Thanks a ton!
I'll take a deeper thought and reply you soon :)
Besides my inline comments above, there are two things to get this merged:
Thanks for looking this over @lucywang000. I'll plan to get the PR updated early this coming week.
@lucywang000 I think I've addressed all your comments, so this is ready for review again. Let me know if I've missed anything.
Thanks a lot for this amazing patch!
Thanks for the feedback @lucywang000, and thanks for cutting a release!
I started to pull on the thread described in #9 and got snagged on services. In services, the circular dependency chain is longer. Instead of fsm -> scheduler and back around to -> fsm, we have service -> fsm -> scheduler, and back around to -> service.
I decided to step back and articulate the problem a bit better. I've re-described the problem below, and this PR demonstrates a potential solution.
Problem statement
There are a few situations where this library is asked to store state as it changes over time, including services and the re-frame integration. In the case of a service, transitioned state is saved within the service and passed on to any listeners. In the case of the re-frame integration, state is persisted in re-frame's app-db. To introduce a more generic term, both services and integrations interact with a stateful environment.
Notably, services and integrations are the two situations which permit use of a scheduler. This makes sense—you wouldn't schedule transitions unless you expected the future revised state to interact with the world somehow.
Let's describe a scheduler in more detail. A scheduler has two purposes. First, it provides an interface to schedule a transition at a future time. Second, when that transition runs, it affects its environment somehow. That is, it must store or transmit the new state into a stateful environment.
So, a service or integration and its scheduler both operate on a stateful environment. They must operate in lock-step, each updating the environment in the same way. The first handles transitions initiated externally by an app and the second triggers transitions internally after a delay, but both must affect their environment the same way.
Implementation Description
This commit introduces the idea of a data
store
: the stateful environment that is updated by transitions. It is a common interface for a service/integration and a scheduler to collaborate around. The two can share a reference to a store, to ensure they both update the environment in the same way. Since the service/integration and scheduler no longer need references to each other, the circular dependency between them described in #9 is broken.As an aside, this commit also reverts a change introduced in #8 (the expansion of the
make-scheduler
interface to accept a context-id function). In hindsight, that change was trying to teach a scheduler to interact with its environment in the same way that the surrounding service/integration does. With the introduction of astore
, that coordination is now more strictly enforced. This PR and #8 are both intended to simplify the creation of new integrations, though I think this PR does a more thorough job.