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

decouple app-db and re-frame #106

Closed
wants to merge 1 commit into from
Closed

decouple app-db and re-frame #106

wants to merge 1 commit into from

Conversation

darwin
Copy link

@darwin darwin commented Aug 15, 2015

This is just a proposal PR. I wasn't able to run tests (OS X) or update examples. I can put more work into this if the general idea is approved.

In Plastic I had to vendor re-frame, because I wanted to use my own app-db and router. Unfortunately router.cljs, handlers.cljs and subs.cljs are requiring re-frame's db.cljs directly which didn't give me a chance to cherry-pick them independently.

This PR removes direct dependency of router.cljs, handlers.cljs and subs.cljs on db.cljs. To preserve backward compatibility this dependency is moved to core.cljs which should be used as public api by majority of re-frame consumers anyways.

The second change is making automatic start of router loop optional. Again, it was moved to core.cljs. Library consumers who will not include core.cljs can still include router.cljs and kick-start router loop by calling (router-loop some-app-db) manually (or provide their own).

This PR does not touch undo.cljs because it is not exposed via core.cljs, I'm afraid decoupling it from app-db would break current API. Anyways, I don't need re-frame's undo at this point

@darwin
Copy link
Author

darwin commented Aug 15, 2015

Btw. this would allow me to use re-frame as a library in Plastic. My situation is a bit more complicated, because I'm running a web worker with its own re-frame instance. Some jobs dispatched on worker need some continuation on main thread, so I really need to implement my own routing.

If anyone is interested, my overlay over re-frame lives here:
https://github.com/darwin/plastic/blob/master/cljs/src/main/plastic/main/frame.cljs
https://github.com/darwin/plastic/blob/master/cljs/src/worker/plastic/worker/frame.cljs

@pkobrien
Copy link

Looks reasonable to me, fwiw.

@darwin
Copy link
Author

darwin commented Aug 16, 2015

I just tested to include re-frame with this PR applied. And still it is not suitable for me to consume it as a library. Another problem are id->fn, key->fn and other state vars littered around. I cannot create an isolated instance of re-frame with its state.

I need two instances of re-frame living in a single javascript context. This is for development, in dev mode I run both main and worker thread in main javascript context to ease development and debugging (cljs-devtools do not work for web workers and devtools in general are not UX optimized for multi-threaded development in javascript).

My current solution was to copy re-frame sources two times, each under its own namespace. This is how I got around state being hardcoded. But this led to unwanted code duplication.

@mike-thompson-day8
Copy link
Contributor

I'm not sure this will help with what you are trying to achieve but ... I have for a while now been considering putting all re-frame state into a single deftype and then making all re-frame.core API functions be defined in terms of that type.

(deftype re-frame [^:mutable app-db  
                   ^:mutable id-fn  
                   ^:mutable key-fn
                     etc .....]

     ;; re-frame core
     (dispatch [this event]  ...)
     (dispatch-sync [this event] ....)
     (register-sub [....] .....)
     (subscribe [...] ...)
     ... rest of the API
)

That would then allow us to create different re-frame instances. It would be backwards compatible because the existing re-frame.core API would be rewritten in terms of a singleton, global instance.

Having said this... the idea remains not 100% thought through (I'm not very experienced with deftype) and, given my current workloads, there's no chance of me doing this in the next couple of months (even though it probably isn't that hard).

@darwin
Copy link
Author

darwin commented Aug 16, 2015

Thanks for commenting. I thought about it last night and I decided to rewrite re-frame to fit my needs. I will try to preserve 0.4.1 API in re-frame.core.

Here is my current progress (it probably won't compile, just a proof-of-concept rewrite):
https://github.com/darwin/re-frame/blob/transducers/src/re_frame/frame.cljs
https://github.com/darwin/re-frame/blob/transducers/src/re_frame/core.cljs

Plans:

  1. one re-frame reframe instance is called "frame" and should be a value, all frame manipulations should be pure and return a new frame value

  2. frame should keep state and provide a transducer: [state, event] -> state
    this transducer is completely decoupled from underlying ratom or core.async channel. It can be used in app-specific ways. Default implementation in re-frame.core will implement 0.4.1-style dispatch and router-loop using core.async. Other implementations may apply this transducer differently. As a simple example imagine: given a seq of events, you can easily apply transducer to produce seq of corresponding app states. Or transduce seq of events into the final state. Or do arbitrary composition of frame transducer with app-specific transducers. For example you can easily filter events, prioritize them or do arbitrary algorithmic transformation on events or resulting states.

  3. middlewares are already transducer-like, I will try to not touch them, I will just drop pure middleware, which will be implemented on re-frame.core level. Middlewares should have no knowledge of underlying-mechanism of state application (i.e. no dependency on reagent/atom).

  4. again subscription subsystem should have no knowledge of underlying-mechanism of state application (no dependency on reagent/atom).

darwin added a commit to darwin/re-frame that referenced this pull request Aug 16, 2015
This rewrite was motivated by day8#106.

In essence re-frame provides a transducer: state, event -> state.
This transducer is able to apply algorithmic transformations on series of events
and build final state up. This allows complete decoupling of re-frame handlers,
subscriptions and middlewares logic from mechanisms how events are queued,
processed and how are their effects applied to app-db. This "bare re-frame" is
implemented in frame.cljs.

For maximal flexibility bare re-frame must be pure and have no special knowledge
of app-db, reagent/ratom and core.async. It must be possible to create multiple
independent instances of bare re-frame.

For convenience scaffold.cljs is re-implementing original re-frame functionality
of v0.4.1 using those new bare primitives. This is exposed as public api via
core.cljs.

App developer can opt-in using default implementation by requiring re-frame.core.
In this scenario app-db is reagent's ratom and events are dispatched and
processed via core.async channel. Single re-frame instance is created and kept
in re-frame.core/app-frame atom. Plus he/she gets access to the original
imperative api to manipulate them.

The app developer is also free to pick/implement other means of employing bare
re-frame by not requiring re-frame directly and build own scaffold around bare
re-frame primitives.
darwin added a commit to darwin/re-frame that referenced this pull request Aug 16, 2015
This rewrite was motivated by day8#106.

In essence re-frame provides a transducer: state, event -> state.
This transducer is able to apply algorithmic transformations on series of events
and build final state up. This allows complete decoupling of re-frame handlers,
subscriptions and middlewares logic from mechanisms how events are queued,
processed and how are their effects applied to app-db. This "bare re-frame" is
implemented in frame.cljs.

For maximal flexibility bare re-frame must be pure and have no special knowledge
of app-db, reagent/ratom and core.async. It must be possible to create multiple
independent instances of bare re-frame.

For convenience scaffold.cljs is re-implementing original re-frame functionality
of v0.4.1 using those new bare primitives. This is exposed as public api via
core.cljs.

App developer can opt-in using default implementation by requiring re-frame.core.
In this scenario re-frame provides a single app-db which is reagent's ratom and
runs single event loop where events are dispatched and processed via core.async
channel. A single re-frame instance is created and kept in
re-frame.core/app-frame atom. Plus developer gets access to the original
imperative API to manipulate them.

App developer is also free to pick/implement other means of employing bare
re-frame by not requiring re-frame.core directly and to build own scaffold
around bare re-frame primitives.
darwin added a commit to darwin/re-frame that referenced this pull request Aug 16, 2015
This rewrite was motivated by day8#106.

In essence re-frame provides a transducer: state, event -> state.
This transducer is able to apply algorithmic transformations on series of events
and build final state up. This allows complete decoupling of re-frame handlers,
subscriptions and middlewares logic from mechanisms how events are queued,
processed and how are their effects applied to app-db. This "bare re-frame" is
implemented in frame.cljs.

For maximal flexibility bare re-frame must be pure and have no special knowledge
of app-db, reagent/ratom and core.async. It must be possible to create multiple
independent instances of bare re-frame.

For convenience scaffold.cljs is re-implementing original re-frame functionality
of v0.4.1 using those new bare primitives. This is exposed as public api via
core.cljs.

App developer can opt-in using default implementation by requiring re-frame.core.
In this scenario re-frame provides a single app-db which is reagent's ratom and
runs single event loop where events are dispatched and processed via core.async
channel. A single re-frame instance is created and kept in
re-frame.core/app-frame atom. Plus developer gets access to the original
imperative API to manipulate them.

App developer is also free to pick/implement other means of employing bare
re-frame by not requiring re-frame.core directly and to build own scaffold
around bare re-frame primitives.
darwin added a commit to darwin/re-frame that referenced this pull request Aug 16, 2015
This rewrite was motivated by day8#106.

In essence re-frame provides a transducer: state, event -> state.
This transducer is able to apply algorithmic transformations on series of events
and build final state up. This allows complete decoupling of re-frame handlers,
subscriptions and middlewares logic from mechanisms how events are queued,
processed and how are their effects applied to app-db. This "bare re-frame" is
implemented in frame.cljs.

For maximal flexibility bare re-frame must be pure and have no special knowledge
of app-db, reagent/ratom and core.async. It must be possible to create multiple
independent instances of bare re-frame.

For convenience scaffold.cljs is re-implementing original re-frame functionality
of v0.4.1 using those new bare primitives. This is exposed as public api via
core.cljs.

App developer can opt-in using default implementation by requiring re-frame.core.
In this scenario re-frame provides a single app-db which is reagent's ratom and
runs single event loop where events are dispatched and processed via core.async
channel. A single re-frame instance is created and kept in
re-frame.core/app-frame atom. Plus developer gets access to the original
imperative API to manipulate them.

App developer is also free to pick/implement other means of employing bare
re-frame by not requiring re-frame.core directly and to build own scaffold
around bare re-frame primitives.
@darwin
Copy link
Author

darwin commented Aug 16, 2015

closing, let's move to #107

@darwin darwin closed this Aug 16, 2015
darwin added a commit to darwin/plastic that referenced this pull request Aug 17, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants