Open jacobrabjohns opened 5 years ago
@jacobrabjohns
Sorry, I missed this notification.
Something like this:
const selectUserById = makeParameterizedSelector('selectUserById', userId => {
return query => {
return query(state => state.users[id])
}
})
The selector can be used like this:
selectUserById(1)(state)
selectUserById(2)(state)
selectUserById(3)(state)
with the property that identity is preserved:
selectUserById(1) === selectUserById(1) // returns the same function instance
Hey @dtinth, what do you think about including the definition of makeParameterizedSelector
in this library?
makeParameterizedSelector
keys selectors with:
const key = args.join(',')
However, I don't think this is safe because not all arguments can be uniquely stringified:
[{ a: 1 }, { b: 1 }].join(',')
// => "[object Object],[object Object]"
For this reason I wonder whether it would be better for makeSelector
to allow selectors to receive extra parameters, and then check those for equality in addition to the existing dependencies check. Example excerpt and usage:
function select(state: State, ...extraParams: ExtraParams): Result {
if (cachedResult) {
+ // Check if extra params have changed
+ let changed = false
+ for (const [index, value] of cachedResult.extraParams.entries()) {
+ if (extraParams[index] === value) {
+ changed = true
+ }
+ }
// Check if dependencies changed
for (const [selector, value] of cachedResult.dependencies.entries()) {
if (selector(state, ...extraParams) !== value) {
changed = true
reason = selector
break
}
}
}
}
const selectUserById = makeSelector((query, userId) => {
return query(state => state.users[id])
})
selectUserById(state, myUserId);
This also means users could use makeSelector
to create all types of selectors: parameterized and non-parameterized.
WDYT @dtinth?
@OliverJAsh Thanks for your opinion! Sorry, I missed the notification, that’s why it took me so long to reply. Feel free to ping me on Twitter (@dtinth).
Yes I agree having makeParameterizedSelector
in would make the library more convenient to use. However it also means that the API surface area that this library has to support will grow. This will increase the maintenance cost for us. That’s why I try to keep this library as bare-bones as possible, which are:
setWrapper
test, this allows us to augment rereselect
with extra performance instrumentation logic without having to release new versions. At Taskworld, we need to integrate this tightly into our React/Redux stack so that we can trace which selectors are recomputed or executed as part of rerendering caused by Redux action).So for this project, flexibility comes over convenience.
I would be very happy if someone makes a higher-level library that makes use-cases such as parameterized selectors more convenient.
In reply to your proposal,
We at @taskworld use TypeScript, so we typed the selectorFactory
like function selectorFactory<T extends (string | number)[]>(...args: T) {
. This eliminates the join problem.
An issue with allowing extraParams
is that each set of parameter passed to the selector needs its own cache. Otherwise when a selector is called multiple times with different arguments, the cache will never be hit, which defeats the purpose of selector memoization. That’s where cache keys come in, and that’s why I had to serialize them into strings.
Thanks for the reply @dtinth!
- An issue with allowing
extraParams
is that each set of parameter passed to the selector needs its own cache. Otherwise when a selector is called multiple times with different arguments, the cache will never be hit, which defeats the purpose of selector memoization. That’s where cache keys come in, and that’s why I had to serialize them into strings.
Isn't the solution to this to use factories, e.g. so we can have one selector instance per component? E.g. in reselect:
const makeSelectUserById = () => createSelector(
getUsersById,
(state, userId) => userId,
(users, id) => {
return users[id]
}
)
const selectUserById1 = makeSelectUserById(); // this has its own cache
const selectUserById2 = makeSelectUserById(); // this has its own cache
We could use the same idea with my proposal above:
const makeSelectUserById = () => makeSelector((query, userId) => {
return query(state => state.users[id])
})
const selectUserById1 = makeSelectUserById(); // this has its own cache
const selectUserById2 = makeSelectUserById(); // this has its own cache
In that case, if a selector receives a different argument, then we deliberately want to recompute.
- At Taskworld, we need to integrate this tightly into our React/Redux stack so that we can trace which selectors are recomputed or executed as part of rerendering caused by Redux action).
Wow, I'd love to hear more about this, because currently we have little visibility over how well our selectors are memoized, which in turn means we have little visibility over how efficient/inefficient our React re-renders are. Are you able to share any more information about the setup you have?
@OliverJAsh Factories is one solution but the lifetime the selector-cache depends on the lifecycle of the component that uses it.
Consider a fictional case in a project management app where we need a selector, parameterized by project ID, that takes the following input selectors:
and outputs a summary of suggestions to users what they should focus on today.
If this selectors happens to be used by 2 components, then this summary must be computed twice each time the store is updated. That’s why we create a global memoization-cache instead, which is what makeParameterizedSelector
does. Therefore, it looks like this:
const selectSummaryByProjectId = _.memoize((projectId: string) => {
return makeSelector(query => {
// ...
})
})
Now, multiple components that uses the same projectId
for selectSummaryByProjectId(projectId)
will be accessing the same selector-cache.
The drawback is that without a memoization-cache eviction strategy, this global memoization-cache will keep growing as more distinct projectId
values are used, which could lead to memory leaks. To fix this makeParameterizedSelector
function as shown in the README needs two extra features:
To summarize, makeParameterizedSelector
:
So that’s why I did not include it in the library.
The basic flow is this:
react-redux
is notified that the store state is updated.Here’s how we tie 4 back to 2 (and sometimes 1):
React has an Interaction Tracing API that allows binding re-renders to an interaction. This will be visible in React profiler as well.
We integrated that API into Redux. We implemented it directly, but a middleware is also available.
That means while the selector is being executed (as part of a re-render) we can access the current interaction using scheduler
’s unstable_getCurrent
function. This allows us to trace the recomputation of selector back to its originating interaction by wrapping the makeSelector
API:
const makeNamedSelector = (selectorName, selectionLogic) => {
return makeSelector(query => {
const currentInteractions = Tracing.unstable_getCurrent()
PerformanceDebugger.recordSelectorReduxInteraction(
selectorName,
currentInteractions
)
return selectionLogic(query)
})
}
For example, and if we notice that the MessageUpdated
Redux action caused selectOnlineUsers
to be re-executed, then we did not memoize that selector properly. This gave us visibility into what we wouldn’t have seen otherwise.
Can you please provide me with an example of how you are using the function provided? I am struggling to infer usage, and it doesnt appear to be anywhere within your tests etc.
Thanks :)