Open mike-thompson-day8 opened 7 years ago
Instead of a deep reaching change like this it might also make sense to consider advocating higher order components. E.g. users could define components that use subscribe
but on their own don't render much themselves and instead pass down data to sub components.
This way most components that actually render DOM nodes would remain pure.
At the very least it should be considered what an alternative approach adds over using higher order "data supply" components.
This is just a quick thought, so I'm hope I didn't miss anything critical.
@martinklepsch so essentially a similar idea to wrapping stateful components: https://github.com/Day8/re-frame/blob/master/docs/Using-Stateful-JS-Components.md
That is, there's an inner and outer component. The outer component handles the subscriptions and passes them, as data, to the pure inner component. Seems like a reasonable convention to me and also means that the inner components are less re-frame specific (well, they still are if they call dispatch in event handlers, unless the react/dom event handlers also get passed down from the outer component).
@danielytics Exactly! The combination of sending both state and actions as props from the wrapper is the same pattern that redux advocates and it makes a lot of sense.
It also makes it much easier to reason about the components when testing. You have the wrapper where the test is literally a spec or a description of expectations:
And then the inner view is just presentation, plus maybe some local state ratoms like expanded/collapsed etc.
I've done something similar using the "outer" container components to inject the subscriptions into the "inner" presentational pure function components.
Perhaps something like a higher-order function/component that merges in some subscription as a prop to a function/component, so you can string together a bunch of subscriptions in one thread-first macro?
We also have more "pure" components in our projects built on top of re-frame. Looks like many people went through the same route, but it's maybe a library that should be separate from re-frame imho.
View functions with a subscription are not pure. They are using a data input from a non-argument source.
From a practical point view, I haven't been motivated to solve this problem previously - given the way re-frame structures your app, non-local reasoning is not a problem in this case, and neither is testing because subscribe is trivial to stub out, so it feels like you are dealing with a pure function - however from a purist's point of view, this is certainly a "pebble in the shoe". In due course, I'd like it fixed, but I'm relaxed about when.
Here's a quick sketch of approximately how (not all issues resolved):
create a new
reg-view
function thru which all views are associated with anid
. Make this registration much likereg-sub
in the way that you can nominate N signal inputs via a signals function.change Reagent so that the first element of vector can be an
id
previously registered byreg-view
. So, instead of[:div ...]
, you could use[:something/panelX ...]
Would be used like this (notice how similar this is to reg-sub):
The computation
fn
becomes pure. The "subscribed to" values are put in the first argument (same pattern as is currently used withreg-sub
).Later, when you wanted to use this view:
[:a-view-id 12 "hello"]
Reagent would look up the view ida-view-id
and use the associated render function.Significant Open Questions: