Support for re-reselect
bdurrer opened this issue · 4 comments
I am a bit suprised to not find anything on google or the issue log here, so maybe I am just doing it wrong :)
We have a lot of re-reselect selectors, but as soon as I add them to registerSelectors
, I get log-spam from re-reselect, because the re-selector's keySelector
is called without the mandatory cache key parameter.
index.js:334 [re-reselect] Invalid cache key "undefined" has been returned by keySelector function.
I guess the tools would need some way to give the keySelector the correct parameters, eg a helper that you could provide or by inspecting the cached selectors. I managed to get a partial view on the selector by fetching one of the cached selectors directly:
const cachedSelector = selector.getMatchingSelector(null /* this normally the state */, 'default' /* cache key param */);
reselectTools.registerSelectors({ cachedSelector });
Hey @bdurrer - I don't use re-reselect in my own apps, so I never thought about this use case. However, if you can come up with a way to get it to work I'd be happy to take a PR
Is this just an issue with re-reselect support, or is it an issue with any multi-arity selector? In src/index.js#checkSelector
every selector is executed with the result of stateGetter
, and off the top of my head I can't think of any easy way to provide a default ...rest
parameter for each selector. I always use a second props
object parameter, which I think is a pretty common pattern (any other parameter type can't be composed), so you could possibly have an optional second parameter for registerSelectors
with a default props
object. However, you would need to provide it with a key and value for every selector in the tree, which would be enormous for any non-trivial application. You could alternatively require an additional defaultProps
value for each multi-parameter function, but this just splits up the same amount of work. I don't see any reasonable way to implement this feature that wouldn't require an extensive integration effort.
So in summary:
- A
defaultProps
parameter toregisterSelectors
would require a key-value pair for every unique key used in the application. This would also result in the same key-value pair being used for every selector that consumes that key, which may not work. - Adding a
defaultProps
value or a default parameter value for each "props" selector (i.e.(__, props = { prop: 1 }) => props.prop
) to every multi-arity selector would allow you to specify the default props individually, but would require even more work than the singledefaultProps
parameter toregisterSelectors
Thanks for the info! You're correct that it's probably more a consequence of the second argument - I've never had to use that myself since I tend to think of selectors more as a mechanism for holding whole-app computed state.
Maybe displaying all cached re-reselectors is not practicable anyway. I guess it could also have a negative impact on performance.
At least in our setup, there are only a handful of possible cach entries (each app tab has it's own selector cache entry) and it would already be nice to view one or two variants of each re-reselectors.
How about having a second parameter on registerSelectors
, which allows to provides a function for each selector. It's job would be to calculate the arguments for one selector, similar to what getStateWith
does for the state.
The function would simply return the required input for one of the cached re-reselectors. One could easily make that function dynamically switch caches based on some extern data, e.g. a global cache-key variable, so you can browse through the selectors based on an external controller.
e.g. something like
function selectorArgs(state, selectorName) {
if (reasons) {
return ['param1', 'param2']
} else {
return ['param3', 'param4']
}
}
registerSelectors({ mySelector }, { mySelector: selectorArgs })
would then call the selector like this
const state = _getState()
const selectorArgs = _allSelectorsArgs[selectorName]
const args = selectorArgs ? selectorArgs(state, selectorName) : []
extra.output = selector(state , ...args)