-
Notifications
You must be signed in to change notification settings - Fork 0
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
proof of concept for reselect without connect #9
base: memoize-functions
Are you sure you want to change the base?
Conversation
// visibleTodos = createMemoizedFunction(() => [this.props.todos, this.props.visibilityFilter], selectTodos); | ||
// matchingTodos = createMemoizedFunction(() => [this.visibleTodos(), this.state.search], selectMatchingTodos); | ||
|
||
visibleTodos = visibleTodosSelector.bind(this, this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This strikes me as undesirable because visibleTodos
is required to be here for for this class to work, but it's not used directly anywhere inside this
. If visibleTodosSelector
was defined in a different file and/or if multiple connected components used that selector, it would become very hard code to follow (and probably maintain):
To generalize that- with this approach, looking up the chain of dependent selectors, you go up the chain and eventually the dependencies of a "top level" selector are the props of [every component that uses the selector]. With a selector that's not a top level selector, like matchingTodosSelector
, you have the dependency that [every component that uses the selector] defines visibleTodos
as a property, even though it's not used directly, or passed directly to anything in class App
.
Or:
// Reselect without connect:
# of places to look/check for selector dependency like visibleTodos() === # of components that use any selectors that depend on visibleTodos()
This is in contrast to regular connected reselect, where each selector's dependencies are clearly defined in one place (even if they're not in the same file, they're still in one place, and where they come from if in a different file is explicated at the top of the file in the import
statement). For the "top-level" selectors, they only have one dependency, the state, and that dependency is super easy to watch change, and we always know where it is.
Or:
// Regular connected reselect
# of places to look/check for a selector dependency === always 1 (either state, or other explicitly defined and imported selector)
Basically I think a lot of complexity is added in going from:
// Regular connected reselect
state --> denormalization --> component consumption
to:
// Reselect without connect:
state --> component --> denormalization --> component consumption
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think your points are helping to distinguish a difference between global selectors for denormalizing state that components would want to receive as denormalized vs. computations that a component may want to memoize.
This PR proved you could use reselect
as the memoizing engine for component-specific calculations, but it's really serving a different purpose within the app than a global selector.
Let me explain:
Suppose the redux store has a list of specialties, and we want to transform those into a different data structure for easier consumption by a <Select> component
Also suppose two different components need the specialties options, say <NewReferralPage>
and <NewServiceRequestPage>
. I think it could make sense to use a reselect
selector to denormalize the data globally, and then use connect
(once for each page component) to pass the denormalized data into each of those two pages as a property like specialtyOptions
.
On the other hand:
Now suppose we add <Pane>
components to the todo app, with each pane having its own searchTerm
and visibilityFilter
. Each pane should take three properties: todos
, filter
, and searchTerm
. Each pane needs to combine those properties into the derived data that the pane wants to display. Computing this data in a memoized fashion is important, and this PR shows how that could be done by leveraging the reselect
library, even though it's serving a different purpose than a global selector.
In many cases, we may have a component that needs to make calculations, and we may want to memoize those calculations. If there is only one kind of component that needs such computations (for example, a todo pane is the only kind of component that needs to know about visibleTodos
and matchingTodos
, there isn't much value in defining a reusable selector that knows how to reach into the component to find the inputs.
However, if there are multiple kinds of components that need to perform the same kind of computations on the same kind of inputs, then we could extract the logic for "finding" the inputs within the component into a reusuable module ---- in such cases, we're building an implicit contract for what methods those components need to expose. This is completely analogous to our Personable
module in Ruby, where the implicit contract is that the class must expose a first_name
and a last_name
method:
module Personable
extend ActiveSupport::Concern
def name
"#{first_name} #{last_name}"
end
def comma_delimited_name
"#{last_name}, #{first_name}"
end
end
Now each class that wants to use the Personable module has complete flexibility to define first_name
and last_name
however it wishes, but the Personable module is completely agnostic about that. This is analogous to matchingTodosSelector
being agnostic about how a component gets or provides a visibleTodos()
method.
As for counting the number of places to look for a dependency --- I'm not sure I follow what you're trying to say.
It's true that a global selector intending to transform state
data can "live" anywhere and be "connected" anywhere, without worrying about whether the component that wants to use it fulfils a certain implicit contract. This is because the actual "implicit" contract of such selectors is that they be called by something (connect
) that provides a particular object (state
).
It's also true that a selector like matchingTodosSelector()
is a different animal because it is expected to be passed a component that provides a certain API. Therefore, not just any component can use it (in the same way that not just any model in our Ruby app would use Personable
).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Basically when I was describing "number of places to look for a dependency" I was talking about the number of spots we need to be sure conform to that implicit contract (exposing this.props.todos
and this.visibleTodos()
). I think relying on those implicit contracts is undesirable in this case, because it makes testing the functions that compute visibleTodos
and matchingTodos
more difficult to test (they would arguably need to be tested in the context of every class/component that include them), and because it makes the code/thing/function that computes visibleTodos
tightly coupled to the components that want visibleTodos
computed (the computing function needs knowledge of how the objects passed to it expose the data it needs, not to mention it expands the component's public API).
I think it would be better to have "computations that a component may want to memoize" implemented as memoizing functions that don't need to know anything about the classes/components that use them (no implicit contracts, instead a more explicit contract in the form of args). The memoizing function would just take the pieces of data that are necessary for the computation as arguments, instead of binding/passing the function this
, and expanding this
's public API.
So instead of:
visibleTodos = visibleTodosSelector.bind(this, this);
it could be something more like:
visibleTodos = visibleTodosFactory(todos, visibilityFilter);
// Note: for this to be a memoizing function it would actually end up being something more like: visibleTodosFactory([todos, visibilityFilter]), but I don't want to de-rail
The benefits of trading the passing-of-this for passing-in-args approach here are:
- Looser coupling
- Easier to test (both the component, and the code that computes the visibleTodos)
- More explicit contract (code easier to follow and reason about)
- No public API expansion of component class
- More idiomatic
Here is a blog post related to include
ing utility functions and creating higher order components as means of achieving composition.
https://medium.com/@dan_abramov/mixins-are-dead-long-live-higher-order-components-94a0d2f9e750
NOTE: it turns out lodash
has a nice function for creating memoizing functions: const square = _.memoize((a) => { return a^2; });
. For each value of a
, square(a)
will only be computed once, and I think is roughly the same API we briefly thought might be possible with reselect.
No description provided.