decouple re-frame and reagent+core.async #107
Conversation
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.
|
This PR will need more work to achieve 100% compatibility. I wrote a few tests. I plan to write more. Also made sure simpleexample works. Open issues are:
|
project.clj
Outdated
| [org.clojure/clojurescript "0.0-3211"] | ||
| (defproject re-frame "0.4.2-TRANSDUCERS" | ||
| :description "A Clojurescript MVC-like Framework For Writing SPAs Using Reagent." | ||
| :url "https://github.com/Day8/re_frame.git" |
There was a problem hiding this comment.
ah, that was unintentional, I was renaming a folder under tests directory and intellij was too smart, I guess. Will change it back.
thanks @nberger compiler was complaining and didn't overwrite existing protocol: WARNING: Protocol IPrintWithWriter implemented multiple times at line 35 src/re_frame/frame.cljs
| @@ -189,13 +164,13 @@ | |||
| (fn after-handler | |||
| [db v] | |||
| (let [new-db (handler db v)] | |||
| (f new-db v) ;; call f for side effects | |||
| (f new-db v) ;; call f for side effects | |||
There was a problem hiding this comment.
Why so much whitespace?
There was a problem hiding this comment.
Sorry, I have reformatted it with Intellij/Cursive, it is indenting comment like that.
Feel free to edit it. I won't be touching it that much.
lessons learned from plastic
|
I'm happy to announce a success story with Plastic. I was able to switch to this fork and use re-frame as a library: My custom scaffolds for main/worker thread routers are less than 100 LOC: |
and give it a better name
src/re_frame/frame.cljs
Outdated
| (let [old-db @db-atom | ||
| new-db (process-event frame old-db event)] | ||
| (if-not (identical? old-db new-db) | ||
| (reset! db-atom new-db)))) |
There was a problem hiding this comment.
This pattern:
(if-not (identitcal? old-db new-db)
(reset! db-atom new-db))is appearing in quite a few places, it might be better refactor it out to a separate fn:
(defn reset-if-changed! [db-atom old-db new-db] (...))|
LGTM |
|
@danielcompton, @mike-thompson-day8 this PR has been sitting idle for a while, but would make great improvements to the code base. Can we get an estimate when this will be reviewed/merged? Having multiple re-frame instances on a single page would be really beneficial to my use case. |
|
@thenonameguy honestly these changes are massive (but really interesting), and I'm going to need some time to chew over them. Mike and I are both in a pretty busy phase of work at the moment and speaking for myself I don't have the time or mental capacity to review it right now. Ballpark estimate, I doubt I'd be able to look at it for a few weeks at least, and I suspect Mike is in a similar boat to me. If you're keen to work with this in the meantime, you could always take this patch and run a forked version for a while. Your feedback on how it feels from an API perspective would also be helpful here. I know that's not perhaps not the answer you were looking for, but I don't have a lot of time or energy to give in the very short term to help get this over the line. |
|
Also, Day8 have what I presume to be the largest Re-Frame codebase in existence, and we don't want to break our own apps or get stuck on an old version without having good mitigations for the points that @darwin has mentioned above. None of this is to say that this will (or won't) get merged, but we do have some other considerations to take into account as well. |
|
Sure, thanks for the response. Then I'll just start using a fork as you mentioned. |
|
For people interested in this version. I'm going maintain the fork here: Just released v0.1.0 as |
|
Any updates or should it be closed? |
This rewrite was motivated by #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 middleware 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 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 opts-in using default implementation by requiring re-frame.core.
In this scenario re-frame creates a single app-db (reagent's ratom) and
runs single event loop where events are dispatched and processed asynchronously
using 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
API to manipulate state of this single re-frame instance.
App developer is also free to pick/implement other means of employing bare
re-frame by not requiring re-frame.core and building her own scaffold
around bare re-frame primitives.