-
-
Notifications
You must be signed in to change notification settings - Fork 714
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
Feature request: Facilitate using output of subscriptions as input to handlers #255
Comments
The way I see this, the problem is not that you need subscriptions in your side-effects, but that you are not seperating your views from your event handlers. Instead of a Example: (defn score-alert []
(let [show (r/subscribe [:should-show])
score (r/subscribe [:score])]
(fn []
[:div {:class (str "stuff"
(when @show "unhide"))}
@score]))) EDIT: added example |
@IsaacKhor Your solution is what I would use if I had an alert that was a dom element. But say that I want to use the native alert box or fire an XHR, then this does not work. |
I think I understand your problem now: In that case, I think we can somehow implement this in interceptors? Instead of |
@IsaacKhor Yes exactly! That is an option. As long as there is a way to get the value of a subscription without creating one, I'd be happy. |
Since 0.8.0, (r/reg-event-db
:fairies
(fn [db _]
(assoc db :magic @(r/subscribe [:magical-value]))) and not worry about performance. However, that is very very ugly. I'm going to work on somehow putting that into an interceptor sometime later, maybe submit a PR to include it in |
I just got into the same issue: I too have a subscription At the moment I'm also solving this with While I'm very relieved that it doesn't affect performance, I don't like the looks of it and it feels very much of a hackish solution (because it's cutting the re-frame data cycle, as @danielcompton said on slack). |
I agree, it does look like an ugly hack. I have been working on refactoring the data in my app using specter and accessing all data through that, but re-frame's new subscription graph/composition of subs is really challenging. As of right now, I'm working on pulling out all data access to specter on a separate layer with handlers and subs accessing all data through that layer; however, exams are coming up, and it will probably have to wait until after Christmas for me to come up with a new data layer. That being said, I think re-frame needs a new data layer. Just being handed a db is not working well anymore; if anybody wants to work on a generalised data layer for re-frame, it would be extremely helpful. |
@IsaacKhor is was looking as specter lately too, and indeed it would be help in refactoring out a "data accessing layer" common to both subs and handlers. With "generalised data layer" you mean some "data accessing layer" built into re-frame, or some recommended way to handle this complex cases? |
@ff- I agree that it feels very hackish. @IsaacKhor I think the nested subscription model actually decreases the need for a more advanced data layer. I can access elegantly abstract away one data access from the other with different subscriptions. But if you want datascript integration you might want to check this out: mpdairy/posh#4 |
Maybe something like this? (reagent.ratom/reaction
(let [sub (subscribe [:magical-value])]
(dispatch [:foo @sub])))
(r/reg-event-db
:foo
(fn [db [_ v]]
(assoc db :magical-value v))) That is, using Its also a bit of a roundabout way since the handler that needs the data doesn't directly access it, and the EDIT: (defn stream-query-to-db ; better name needed ;-)
[query db-path]
(reagent.ratom/reaction
(dispatch [::stream-query-to-db db-path @(subscribe query)])))
(r/reg-event-db
::stream-query-to-db
(fn [db [_ db-path value]]
(assoc-in db db-path value))) then elsewhere: (stream-query-to-db
[:magical-value] ; The query
[:path :to :magical-value]) ; Where in the app state to put the query's result
(r/reg-event-db
:fairies
(fn [db _]
(let [magical-value (get-in db [:path :to :magical-value]]
... Still very much a nasty hack and it also seems a bit weird to store a materialised view of db inside db, but at least from the users pov its convenient. (Also still untested) |
Great effort but I since the subscriptions are already cached as of 0.8.0 I think this doesn't make much sense to be honest. I would be very happy to hear from someone that understands re-frame under the hood why this is or is not a feasible feature request. |
These are quick notes for myself, for when I get back to this (in a while?). Although someone might beat me to it, and do a library. Hint. Hint. So I'm not directly addressing/answering the above conversations ... just jotting stuff down quickly so I don't forget it ... There's two quite different issues to be addressed:
Point 2 should be done via a effects handler, called say,
Then later, turn it off:
Does anyone want to create an (effects) library for this? Does it need to be in the core of re-frame? Regarding Point 1 (at the top) .... to make a subscription value available in an event handler, it would have to be injected into the coeffects of that handler via an interceptor. We absolutely can't have an event handler reaching out and grabbing global values. So usage would look like this:
Notice Note this Point 2 can be handled in a library as well. It doesn't have to be in the core of re-frame, unless we want that for ease of general use. Okay, so this ended up more than some quick notes. Ended up a quick design session and a call for volunteers. |
One thing to be careful with point 2, is to make sure that you get a consistent, correct view of the data. If you are creating subscriptions that are interleaved with effects being run, I'm not sure if that is safe. May not be an issue, but thought I'd drop it here. |
We have a need for both point 1 & 2 above, and I took a stab at @mike-thompson-day8 mentionned on slack that:
However, my understanding of There also seems to be an issue in the case of a sub that'd never get used outside of (defn dispose-maybe
"Dispose of `ratom-or-reaction` if it has no watches."
[ratom-or-reaction]
;; Behavior notes:
;;
;; 1. calling `reagent/dispose!` takes care of "walking up" the reaction graph
;; to the nodes that the reaction was `watching` and remove itself from
;; that node's `watches`'.
;;
;; 2. In turn removing a watch causes that node to dispose of itself iff it
;; has no other watches.
;;
;; 3. Essentially disposing of a node will dispose of all its dependencies iff
;; they're not needed by another node.
;;
;; 4. Re-frame adds an on-dispose hook to the reactions returned by
;; `re-frame/subscribe` that will dissoc them from the subscription cache.
;;
;; There are some potential issues with this scheme:
;;
;; Imagine a sub that is only used by `inject-sub` and not by components, then
;; it will never have watches. This seems like it would be a problem if that
;; sub now gets injected in multiple handlers. Due to the disposal behavior
;; descibed above, for every handler the cofx would cause:
;;
;; - subscribe to find the cache empty
;; - subscribe to grab the event handler and produce a value
;; - subscribe to wrap the value in a reaction
;; - the cofx to deref that reaction
;; - the cofx to dispose of it (it has no watches)
;; - the reaction to remove itself from the cache (on-dispose hook)
;;
;; We'd basically pay for a cache miss every time we inject that sub?
;;
(when-not (seq (.-watches ratom-or-reaction))
(ratom/dispose! ratom-or-reaction)))
(re-frame/reg-cofx
:sub
(fn [cofx [id :as query-vector]]
(let [sub (re-frame/subscribe query-vector)
val (deref sub)]
(-dispose-maybe sub)
(assoc cofx id val))))
(defn inject-sub
[query-vector]
(re-frame/inject-cofx :sub query-vector))
;;; Example usage:
(re-frame/reg-event-fx
::my-event
[(inject-sub [::foo/bar])]
(fn [{:keys [db] ::foo/keys [bar] :as cofx} _]
;; bar is now bound to a deref'd value of ::foo/bar
)) |
Also took a stab at implementing @mike-thompson-day8 suggestion for point 2. There are a couple of differences here:
Would appreciate some feedback esp on correctness. I was also wondering if the api might be cleaner by defining different fxs for registration and disposal rather than relying on the (defonce tracks-register (atom {}))
(defn new-track
"Create a new reagent track that will execute every time `subscription`
updates.
If `event` is provided will always dispatch that event.
If event is nil, `val->event` is required and will be invoked with the latest
value from the subscription. It should return an event to dispatch or nil for
a no-op.
"
[{:keys [subscription event val->event]}]
{:pre [(vector? subscription) (or (vector? event) (ifn? val->event))]}
#?(:cljs
(ratom/track!
(fn []
(let [val @(re-frame/subscribe subscription)]
(when-some [event (or event (val->event val))]
(re-frame/dispatch event)))))))
(defmulti track-fx :action)
(defmethod track-fx :register
[{:keys [id] :as track}]
(if-some [track (get @tracks-register id)]
(throw (ex-info "Track already exists" {:track track}))
(let [track (new-track track)]
(swap! tracks-register assoc id track))))
(defmethod track-fx :dispose
[{:keys [id] :as track}]
(if-some [track (get @tracks-register id)]
#?(:cljs (do (ratom/dispose! track) (swap! tracks-register dissoc id)) :clj nil)
(throw (ex-info "Track isn't registered" {:track track}))))
(re-frame/reg-fx :track track-fx)
;;
;; Example
;;
(re-frame/reg-event-fx
::start-trigger-track
(fn [cofx event]
{:track
{:action :register
:id 42
:subscription [::sub "blah"]
:val->event (fn [val] [::trigger val])}}))
(re-frame/reg-event-fx
::stop-trigger-track
(fn [cofx event]
{:track
{:action :dispose
:id 42}}))
(comment
(do
;; Define a sub and the event we want to trigger
(defonce foo (ratom/atom 0))
(re-frame/reg-sub-raw ::sub (fn [_ _] (ratom/make-reaction #(deref foo))))
(re-frame/reg-event-db ::trigger (fn [db val] (println "Trigger" val) db))
;; Start the track
(re-frame/dispatch [::start-trigger-track])
;; Update the ::sub, will cause ::trigger to run
(swap! foo inc)
(swap! foo inc)
(swap! foo inc)
;; Stop the track, updates to ::sub aren't tracked anymore
(re-frame/dispatch [::stop-trigger-track])
(swap! foo inc))) |
This way (untested) of writing the
Repeat, this is untested. And perhaps not even a good idea. |
@mike-thompson-day8 interesting, that does look simpler too. I also had some comments about a potential issue if the sub that's injected is never used in a component. If that assumption was correct, it looks like the problem would persist with this solution. Could you confirm that it's an issue and whether this could be worked around? |
Your "Behavior notes" 1 to 4 are correct. So too are your notes about the expense of a "cache miss". As you speculate, there will be the overhead of creating a subscription, adding to cache etc, and then pulling that structure down again. I don't know any easy way to avoid this without significant surgery to the way My version is slightly less efficient again because it pays the price of creating a But ... I'm guessing for most cases the subscription will already exist and be cached, making the value readily/efficiently available. |
Thanks for the review @mike-thompson-day8 It seems a good compromise would be to have the co-fx log a warning if it disposes of an injected sub. If the cache miss were to happen inside a handler that's fired often like Also curious if you think |
We shipped the inject cofx in a pre-release of our re-frame-utils. People following this issue might want to check it out! |
@julienfantin Sweet! I'll probably check it out. Wouldn't mind some docs on how to use it though 👍 |
@vikeri cool! Docs are still on the todo list, but the fx handler for inject has a decent docstring showing usage example as well as some behavior notes that you should be aware of. Also there are tests that may provide some more context. |
I've recently updated the FAQ entry on this: |
Discussion continues on #680 |
Pasting from slack since this is probably a better forum to discuss these things.
First I want to apologize for not knowing enough of re-frame's internals to know if this can be solved. Would gladly help out in implementing a possible solution though.
Problem
I want to use the value of a subscription as input to a handler. Say I am developing a game and I want to display an alert box with the current score. I have an fx handler for displaying alerts
:alert
. The current score has a subscription:current-score
that is calculated from other subscriptions that in turn are calculated from other subscriptions per the new0.8.0
way of using subscriptions as input to other subscriptions.Current solution
Today I have two ways of solving this:
:current-score
in the component that triggers the:alert
dispatch and dereference the sub when dispatching. This is not very elegant since the component does not actually depend on:current-score
but will still be updated whenever that value changes.Suggested solution
One way of solving it would be to have a function that took a subscription vector but did not return a subscription but just a dereferenced value directly. Not exactly pure but could be quite useful if used carefully? And then this could be sugared into the reg-event-db/fx as an optional parameter.
If it's unclear I'll happily provide some examples.
The text was updated successfully, but these errors were encountered: