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
Better re-frame subscriptions - opinions/ideas requested #170
Comments
About the (defsub
:subscribe [:anything]
:query (comp fn-1 fn-2 fn-3)) Which is better than (defsub
:subscribe [:anything]
:query [x] ((comp fn-1 fn-2 fn-3) x)) Removing |
Personally I like that we follow a classic macro approach, keeping ...another thing I noticed from the above description (thanks Mike, as usual awesome writing) is that a data-only pattern is emerging, where you don't need macros and you define params in a map. I am talking about this in particular: (defsub :sorted-items
:ret-spec :info-model/items
:fn sorted-items
:args items :from :subscribe [:items]
sort-attr :from :subscribe [:sort-by]
[_ a b c] :from :query-v
:body (sort-by sort-attr items)) This is an accepted trade in the Clojure community as well (see Onyx), greatly improve composability and allows user to define their own dsl. |
I would prefer something like this (defsub :sorted-items
:ret-spec :info-model/items
[db query-v]
[items [:items]
sort-by [:sort-by]]
(sort-by sort-attr items)) where [items [:items]
sort-by [:sort-by]] is like "extracted" from |
Hey @mike-thompson-day8 ! Thanks again for your work. My 2c in the order that you listed items.
|
In my humble opinon, I am not on-board with defsub. As an alternative I propose having pure functions with one arg per subscription and a function that adds metadata to the functions var (and possibly a macro to add sugar to the function). Then subscribe takes a var and looks up metadata. On the plus side this is very unmagical, but it may not be backwards compatible. Thus: (def sorted-items sort-by)
(re-frame.core/new-sub #'sorted-items [:sort-by :items])
(re-frame.core/subscribe #'sorted-items) |
Simplifying subscriptions is a good idea. In my opinion, defsub should use a pure function (defined inline or otherwise) to do the extraction logic. One reason is that the extraction functions can be defined in cljc, and introspected, tested, and extended via clojure. This also allows mocking http endpoints and testing the behavior of the frontend system from the jvm. I'd vote: (defsub :items
:ret-spec :info-model/items
(fn [db _]
(:items db))) |
hmm... defsub is looking a bit like (defsub :sorted-items
:ret-spec :info-model/items
[db [_ a b c]]
(alet [items (subscribe [:items])
sort-attr (subscribe [:sort-by])]
(sort-by sort-attr items))) i need to think on this some more |
Good point @mccraigmccraig, it also reminds me in some ways of manifold's let-flow macro. |
right @danielcompton |
one difficulty with |
Using the current Compare to current: Trying to accommodate both the simple:
And the more complex:
|
Suggestion from @escherize via slack:
|
@mike-thompson-day8, |
After some reflection, I think that this version is my favorite fwiw. (regsub :footer-stats
:<- {:todos (subscribe [:todos])
:completed (subscribe [:completed-count])}
(fn [{:keys [todos completed]} _]
[(- (count todos) completed) completed]))
(regsub :footer-stats
:<- {:todos (subscribe [:todos "apple"])
:completed (subscribe [:completed-count @(subscribe [:order-by])])}
(fn [{:keys [todos completed]} _]
[(- (count todos) completed) completed])) I'm not sure if I've addressed your objections though, @rasom. |
@escherize, for example (register-sub :something
(fn [db [_ param]]
(let [something-other (subscribe [:something-other param])]
(reaction (some-magic (get-in @db [:something param]) @something-other))))) I mean... Where we will get that |
Okay, after a week of on-again off-again struggle, a good solution turns out to be embarrassingly easy. How did I not see this before? So, I'm still not 100% sure about the whole |
Using a function to setup the input signals to the subscription looks good, @mike-thompson-day8. I was chatting with @gadfly361, and he had an interesting idea for |
@mike-thompson-day8 Thank you for bringing this up for discussion! The proposed changes (regsub :footer-stats
:<- [:todos]
:<- [:completed-count]
(fn [[todos completed] _]
[(- (count todos) completed) completed])) I think the macro syntax does in fact facilitate seeing subscriptions more as derivations of other nodes/signals in a graph, rather than mostly (reactive) queries on the db. I think this concept is clear in the ‘first function’ approach a well: (regsub :footer-stats
(fn [query-v _]
[(subscribe [:todos])
(subscribe [:completed-count])])
(fn [[todos completed] _]
[(- (count todos) completed) completed])) But it’s more obscured in the syntax here. Specifically the relation of the returned vector of signals in the first function, to the destructured first arg in the second fn is more difficult to recognize and easier to get wrong. The user also needs to remember that As we are already employing a macro to make subscriptions more user friendly, I was thinking if we should consider to even go a few steps further: Say I want to subscribe to (subscribe [:footer-stats 1.2])
(regsub :footer-stats
:<- [:todos]
:<- [:completed-count]
(fn [[todos completed] [_ param]]
[(- (count todos) completed) (* completed param)])) Problems:
Here is a slightly modified syntax that attempts to address these issues: (regsub :footer-stats [param]
:< :todos
:< :completed-count
(fn [todos completed param]
[(- (count todos) completed) (* completed param)]))
This would also allow to 'pass on' subscription arguments to the 'sub subscriptions' like this: (regsub :visible-todos [limit]
:< [:todos limit]
:< :showing
(fn [todos showing _]
(let [filter-fn (case showing
:done :done
:all identity)]
(filter filter-fn todos)))) Instead of writing (regsub :visible-todos [limit]
:< :todos
:< :showing
(fn [todos showing limit]
(let [filter-fn (case showing
:done :done
:all identity)]
(->> (filter filter-fn todos)
(take limit))))) which would (theoretically) be less efficient. By omitting the vector around the sub-subscription ids, omitting the additional dash and omitting the destructuring, I believe the syntax gets easier to scan/read and comprehend: (regsub :visible-todos [limit]
:< :todos
:< :showing
(fn [todos showing limit]
..)) vs. the original proposal: (regsub :visible-todos
:<- [:todos]
:<- [:showing]
(fn [[todos showing] [_ limit]]
..)) Interestingly, mixing the vector form (when params are passed) with the non vector forms appears intuitive to me (interested in what others think of cause!): :< [:todos limit]
:< :showing Small thing: I did not recognize this Lastly, I'd suggest to go all-in with the new regsub syntax and make the input signal declaration mandatory: (regsub :showing []
:< :db
(fn [db]
(:showing db))) I think especially beginners will appreciate the consistency in how In the current abbreviated (one function) form, the user has to know that in this case db is provided in the first arg: (regsub :showing []
(fn [db]
(:showing db))) It's visually quite different/inconsistent from the explicit signal listing form and may therefore be a slightly confusing. |
@andreasthoelke thanks for that very detailed write up. I've read through it a few times fairly carefully. I have already wrestled with virtually all the proposals you make. For example, I've been backwards and forwards a few times over how to present signals to the computation function. You suggest positional arguments, rather than a data structure (vector?) as the first parameter. But I've gone with a non positional approach because some may prefer to do this:
I've also agonized over the whole destructing of query params in a way that allows their access in subscriptions I must admit, I'm pretty happy now with the current approach. In fact, I'm in half a mind to not even allow for the sugar. There's something very consistent and right about the I do like stuff minimal, so I'm thinking seriously about switching to |
Looking through our codebase, I'd estimate that 99.9%* of our subscriptions don't use subscription parameters, so the syntactic sugar would be able to be used almost everywhere (I think?). * not really an estimate |
I wonder about the need for multiple (regsub :visible-todos
:<- [[:todos] [:showing]]
(fn [[todos showing] [_ limit]]
..)) |
This code is now in the |
@mike-thompson-day8 I am thinking of putting the new I saw the todo app and it looks very neat (there is a typo, the sub fn is called |
Sorry, yes, I've been progressing slowly on this. Can you give me two more days please and I'll cut an alpha? In the meantime, you can of course, grab |
Here is a newbie prospective who has been with clojurescript and re-frame for just a couple of days. I am continuously tempted to abandon re-frame patterns for two simple reasons. First is the annoying repetition that (register-sub :user-ids
(fn [db]
(reaction (get @db :user-ids [])))) The simple alternative of using So here is the proposal. Why not make (defsub sorted-items []
(sort-by @(sort-attr) @(items)))) The above would register the subscription function like this (re-frame.core/register-sub
:sorted-items
(fn [db [_]]
(let [items (items) ;; defsub-ed before
sort-attr (sort-attr))] ;; defsub-ed before
(reaction (sort-by @sort-attr @items))))) It also would define the actual subscription invocation function (defn sorted-items []
(subscribe [:sorted-items])) So no more awkward The above code assumes that In any case, it looks to me that, explicit declaration of subscriptions in |
@vspinu Both Remember also, when you "invoke functions" via dispatch and subscribe you are down the path to "programming with data". There are many nice implications. If your application is simple, then sure abandon re-frame - almost anything will work. But, if your application is a bit more complex, you'll be glad you didn't. |
Sure, I was not suggesting that. Both are not pure functions but I would still prefer to pass |
@arichiardi I've cut a version |
That's awesome, I think I can handle the changes no problem, the new features are super cool ;) |
Not related to |
@arichiardi CLJS-1701 shouldn't be an issue for re-frame. I've held off involving spec. . |
How sure are we that query de-dup is going to be a thing in re-frame's future? I know API breakage is on the cards, as are bugs (given the alpha), but is it confirmed that the de-duplication concept is here to stay? I ask because right now I'm struggling with incorporating de-duplication as mentioned in Subscribing-To-A-Database . Lots of using counter atoms to prevent firing off multiple remote queries just because a query reaction is de-refd multiple times, etc. I love the de-dup that's in 0.8.0-alpha1 so far, as it's a great way to minimize net traffic in the face of laziness, but I don't want to start depending on it and then having it go away. (Totally within your rights, given the whole alpha thing!) (BTW, thanks for such an awesome |
@rtomsick, it's very unlikely query dedup is going away, barring major issues. I wasn't clear from your post, are you having issues with re-frames query dedup in the alpha, or implementing your own dedup? |
@vspinu not necessarily a permanent solution, but for repetitious simple db gets, I created a single :raw-query subscription that takes a vector param which is passed to (get-in) on the app-db. This has been great for quick development. Searching the codebase for the paths allows you to identify any re-used :raw-query's that deserve being converted into explicit subscriptions. You could make it more 'secure' by applying a filter to the paths in the :raw-query subscriptions itself to restrict access to parts of the db. |
Yes. I did exactly that. It might seem natural, but it wasn't so obvious to me when I started with re-frame. The following lines essentially removed 80% of my sub and dispatch querying code: (re-frame.core/register-sub :db-get-in
(fn [db [_ & ks]]
(reagent.ratom/reaction (get-in @db ks))))
(defn db-sub [ks]
(re-frame.core/subscribe (into [:db-get-in] ks)))
(re-frame.core/register-handler :db-assoc-in
(fn [db [_ ks val]]
(assoc-in db ks val)))
(defn db-assoc-in [ks val]
(re-frame.core/dispatch [:db-assoc-in ks val])) |
Those handlers and subscriptions will reduce the number of subscriptions and handlers you have to write, at the cost of tightly coupling your application to the structure of app-db. The idea of subscriptions and handlers is to separate the what from the where. If you need to change the structure of app-db for some reason, then you will need to go through all of your view code, subscriptions, and handlers to makes sure you've updated those places that depend on it. If you make more specific handlers and subscriptions, then you can refactor app-db without a lot of code churn. A middle ground we've sometime used is to create handlers to update parts of an app-db, e.g. |
What does the subscription in https://github.com/Day8/re-frame/wiki/Subscribing-To-A-Database look like with reg-sub? I've been playing with that for an hour or so now and haven't worked out the right magics. |
@newhook I haven't had time to think that through. So I'd just continue to use the same pattern described in that Wiki page, via |
@danielcompton I'm not having any issues with the alpha, but I'm trying to figure out how long-lived my own de-dup code will be. 😄 If it looks like de-dup is here to stay then I won't worry about implementation cleanliness (since I'll probably see a new release of re-frame before I ship). Thanks! |
STATUS: We're getting close to a release of v0.8.0. But I won't because good docs are still pending. Currently the best explanation is: |
Released as part of v0.8.0. Although docs are still lacking. At this point, todomvc tutotial continues to be the best soruce of information: |
Better re-frame subscriptions
I'd like to pour some macro sugar on
re-frame.core/register-sub
. I'd like your feedback and ideas.Naming
First question: what should we call this macro?
defsub
,def-sub
,defs
? Other? Below, I assumedefsub
.The Goals
Solve these current problems:
reaction
and, in more complex cases, chaining reactions, and how one subscription can depend on the output of another subscription, etc. All doable, and knowable, of course, but I'd love to make it simpler.app-db
as a parameter, not a value. They also return areaction
rather than a value. There's a technique to create pure subscription functions but it involves a bit too much boilerplate. I'd like pure functions without the boilerplate (defsub
should look after the boilerplate for me)Improve by adding:
cljs.spec
for its output, and this should be checked. Maybe, this checking will be be disabled in production.Other goals:
re-frame.core/register-sub
should still just work.And, what's going on is:
A. There's a pure function
B. it performs computation using the values from:
- one or more "input signals"
- data from a query vector
- zero or more dynamic query parameters
Question: are there other considerations/goals I've missed?
Backgrounder
This ticket is about subscription definition, but let's remind ourselves about use:
The 1st parameter is the
query vector
, which starts with thequery-id
(generally a namespaced keyword). The 2nd parameter is a vector of input signals (ratoms/reactions). When they change, the subscription should be rerun.Most subscriptions only involve one parameter. Many re-framers don';t even know about the 2nd.
What
defsub
use might look likeA super simple example:
Notice:
:items
), just as it would be withre-frame.core/register-sub
. So no change there.fn
of the kind normally given tore-frame.core/register-sub
except:db
is a value, not areagent.core/atom
reagent.ratom/reaction
Point 2 is 50% of the battle.
Question: does this variation work better for you?
Or maybe pare it right back to this:
This approach is lovely for simple cases. But, it could be problematic as we get into more complex cases. Read on.
Return Spec Checking
defsub
can also take a registeredcljs.spec
which describes the structure of the value returned by the handlerfn
.Signal Graphs
re-frame automatically de-duplicates subscriptions. As a result, an architecture in which subscriptions are built using inputs from other subscriptions will result in an efficient signal graph.
So we want to support that process.
Here's a complex registration from the existing re-frame README:
This formulation has problems. Firstly, it requires a bit of knowledge arranging all those reactions correctly. Hard for a newbie. And easy enough for someone experienced to get wrong. Secondly, this formulation offers no chance for de-duplication of the input reactions. If the same
reaction
s are used elsewhere they will run as duplicates.So, current best practice is to rewrite it like this:
Better!! But still a bit hard for a newbie to get right. For example, someone can easily wrap the entire
let
in areaction
which is wrong.And, even if that wasn't a problem, we don't want
reaction
used explicitly any more.So, here's how it might look with
defsub
:Note:
:subscribe
clauses. In the absence of a:subscribe
a default will be provided:subscribe [:/]
which is a pre-defined subscription which gives you the root ofapp-db
fn
positionally as the first N parameters.fn
is still pure. Values in, values out. No side effects.Or perhaps this variation is better:
OR maybe we do awayaway with the
fn
formulation (args etc):but this latest one makes the simple case a bit hard:
maybe I have to do this:
Hmm. I want my DSL to be immediately familiar to a clojure programmer. I'm not sure that loosing the
fn
part is good. We are writing a function. It is just that we want it to be reactively re-run.OR maybe we prefer
So which of these appeals? Other alternatives?
The Generated code
Let's switch gears and, for the moment, forget about DSL aesthetics.
What code will this macro generate? It will still need to call
re-frame.core/register-sub
.Further Thoughts
Provide a spec for the query-v ?
Perhaps just fspec the handler?
Remember a subscription can use a value in the query-v
Remember a subscription can use the value from a dynamic parameter
:subscription [:some-q 42] [r]
The text was updated successfully, but these errors were encountered: