Coverage remained the same at 100.0% when pullingdd5e5d8 on sgrishchenko:key-selectors-composition into263c998 on toomuchdesign:master.

Hi@sgrishchenko, I see the point of supporting selectors' composability.

I'd like to consider the possibility of providing this feature as an external utility function. Something to directly provide askeySelector or maybe a sort of selectors composer likerecompose does.

If I try provide external utility function, I receive something like this:

constkeySelector=combineKeySelectors(dependency1,dependency2,(state,props)=>props.someProp)constcachedSelector=createCachedSelector(dependency1,dependency2,(state,props)=>props.someProp,(first,second,someProp)=>({    first,    second,    someProp}))(keySelector);

As you can see, I need again copy-paste dependencies and again so easy when adding a dependency, forget to update key selector. What if I create separate functioncreateKeyComposedSelector(...) and encapsulate all additional logic in it?

It seems that the arguments of the hypotheticalcombineKeySelectors function are the same selectors provided asinputSelectors, right?

Not that I like this syntax, but as a proof of concept, might it be expressed like this?

constinputSelectors=[dependency1,dependency2,(state,props)=>props.someProp,];constcachedSelector=createCachedSelector(inputSelectors,(first,second,someProp)=>({    first,    second,    someProp}))(combineKeySelectors(...inputSelectors));

At first, I can't pass all dependencies incombineKeySelectors function. There is three kinds of dependencies:

• Re-reselect selectors (or other selectors, that haskeySelector property)
• Simple state selectors (likestate => state.foo)
• Property selectors, which probably should be used in key selector (like(state, props) => props.someProp)

For this reason, I cannot distinguish state selectors from property selectors and I need do something like this:

constkeySelectors=[dependency1,dependency2,(state,props)=>props.someProp,];constcachedSelector=createCachedSelector([state=>state.foo,state=>state.bar,    ...keySelectors,],(foo,bar,first,second,someProp)=>({    foo,    bar,    first,    second,    someProp}))(combineKeySelectors(...keySelectors));

Secondly, imagine that you future user ofre-reselect library. You have two helpers:createCachedSelector andcombineKeySelectors. As a good developer, the first thing you do is create a helper that combines these two helpers, because you don't want to copy-paste it every time. So why can't we do this out-of-box for library users?

Additionally, I understand your concern about granularity of this approach. I suggest to expose two new helper:

• createKeyCombinedSelector (high level for common usage)
• combineKeySelectors (low level for custom helpers)

What do you think about it?

For this reason, I cannot distinguish state selectors from property selectors and I need do something like this:

I've personally never dealt with such a scenario, therefore take what I say with a grain of salt: what ifcombineKeySelectors called each selector'skeySelector (when existing) or the selector itself (when nokeySelector is exposed) and chain onlystring | number results? It might have performance implications, of course.

It might get cumbersome (wrappers wrapping wrappers... mhh..), but I'd definitely go for thehelper approach.createKeyCombinedSelector exposes slightly different API's (eg. nokeySelector) and I considerpolymorphic API's a no go in this context.

I'd also suggest to find a way ofintroducing these non-core featuregradually: maybe aseparatere-reselect-util repo or exposing someexperimental non stable API's. What would you go for@sgrishchenko?

Same goes for source code and tests: I'd try to keepcore logic and the rest well separated. Rollup will think about shipping all together.

If there are still blurry areas about what these utility are supposed to do, we might consider writing down thedocumentation first.

Thanks for caring@sgrishchenko!
Greetings!

I just sawhttps://github.com/sgrishchenko/reselect-utils! Amazing job, there.

I'd keep this issue open as a remainder to update the docs to extensively mentionreselect-utils.

I'm investigating the feasibility of such an API for the use case@sgrishchenko mentioned:

importcreateCachedSelector,{combineKeySelectors}from're-reselect';constcachedSelector=createCachedSelector(dependency1,dependency2,(state,props)=>props.someProp(first,second,someProp)=>({    first,    second,    someProp}))(combineKeySelectors(options));

If we somehow extended the data provided to thekeySelector with theinputSelectors array,combineKeySelectors should have everything it needs to automatically create the combinedkeySelector.

Closing in favour of#78. Thanks@sgrishchenko for the inspiring contribution!

Hi@dkaledin@veksa@sv1809@eaglus,
with v3.3.0 introducingkeySelectorCreator option, dynamic keySelector composition is now a thing.

With#78 I also tried to provide akeySelectorCreator function to ship withre-reselect, but I realised I don't have enough hands-on experience about dynamic selector composition to come up with a valuable solution.

So I'm inviting you to provide some real-world examples and possible API's to reason about the possibility of shipping such akeySelectorCreator function in a way which is generic enough to live in this library.

Thanks to you all. Cheers!

@toomuchdesign@sgrishchenko Interesting following the back-and-forth here. I actually stumbled upon this thread through having misinterpreted the library similarly tothe users in this other thread, and coming to realize that cache key selection didn't follow the mechanism I originally believed.

I think some of the frustrations expressed here would be mitigated through a design or option which just provided dependency function results to the cache key selector automatically. This would eliminate redundancies between the 'actual' selector and the cache key selector. Although, maybe this is precisely what@sgrishchenko was alluding to with hiscreateKeyCombinedSelector idea. (I'm only just seeing/processing this discussion for the first time, so forgive me if I'm repeating notions already entertained.)

It is unfortunate that reselect does not expose itsinternal mechanism for building dependency results as an abstraction, meaning this library would have to replicate that behavior. However, allowing a key selector option to facilitate a predefined, "default"keySelectorCreator which behaved like this:

functioncreateDependencyResultsKeySelector(keySelector,dependencies){returnfunction(){constdependencyResults=dependencies.map(dependency=>dependency.apply(null,arguments));returnkeySelector.apply(null,dependencyResults);};};

would allow usage like this:

constselectABC=createCachedSelector(selectA,selectB,selectC,(a,b,c)=>compute(a,b,c))((a,b,c)=>`${a}:${b}:${c}`,{useDependencyResults:true});...// BoomselectABC(state);

There would of course be (potentially) redundant work betweenre-reselect andreselect if a cached result was not found, andreselect proceeded on its merry way determining the dependency function results again. Ideally, the dependency functions would have themselves been created with appropriate memoization or caching, so the results wouldn't actually be recomputed with proper usage, but the process of running through the dependencies and building the list again would be.

I do want to emphasize the utility of allowing this library to use dependency results, rather than just initial selector arguments, to derive cache keys. It can be unwieldy to have to independently determine and pass in those values in cases where the selectors aren't normally written to take them, just to ensure that I can take advantage ofre-reselect's superior caching ability. It also limits the ability of cached selectors to be used withcreateStructuredSelector in this fashion:

constmapStateToProps=createStructuredSelector({stuff:selectStuff,otherStuff:selectOtherStuff});

...unless I want to write my key selectors to re-select (hah!) the "dependency results" manually, fromstate, to derive the right cache keys. Which I can. It's just a little leaner when done automatically. : - )

I can't see getting around a certain amount of awkwardness either way. For example, this is a little awkward:

/** * Using the 'extra argument' form: selectPersonByName(state, 'John') */constselectPersonByName=createCachedSelector(selectPeople,// This 'dependency' just exists to forward the// selector's second argument to the result function(state,name)=>name,(people,name)=>people[name])((_,name)=>name);

and this is a little awkward:

/** * Using the 'single state argument' form: selectCurrentStudentTestAverage(state) */constselectCurrentStudentTestAverage=createCachedSelector(selectCurrentStudent,// This 'dependency' just exists to forward something// to the key selector (also, let's pretend currentStudent// does not already have the studentId property)selectCurrentStudentId,currentStudent=>{const{ testResults}=currentStudent;returntestResults.reduce((acc,result)=>acc+result,0)/testResults.length;})((_,studentId)=>studentId,{useDependencyResults:true});

Anyway, that's my 2 cents!

Hi@mbellman,
I read your comment just now. Sorry, it was a quite busy time and I overlooked your message. Thanks for writing, though!

I 100% agree that the confusion about what argument are actually received by thekeySelector is an issue hitting al lot of users. Including me: I realised that I made the same mistake in the last docs update. I just opened a PR.

As you mentioned, there are a few reasons whykeySelector are provided with the initial selector arguments:

• Duplicated logic betweenreselect andre-reselect
• inputSelectors would be evaluated twice byreselect andre-reselect
• inputSelectors results are not known at declaration time (they're evaluated on runtime): using them askeySelectors arguments would have implications that would need further clarifications

I will openan issue to reason about the opportunity of adding the possibility of providingkeySelectors withinputSelectors results instead of their same arguments.

I'm a bit reluctant to proceed lighthearted for 2 reasons:

• providing both option could make the library still harder to grasp
• TS typings complexity would increase exponentially

But I'm totally curious to better understand whether such a feature could provide real value.
Cheers! ✌️