A new approach to sharing selectors: createIndexedSelector

[1000hz]

I think there’s a better approach to sharing selectors with props between components than the one outlined in the docs. That approach suggests each component should create its own private instance of the selector, but this leads to unnecessary recomputations. Two different selector instances given the same arguments will individually recompute and store the same result.

Instead of creating a private instance of the selector for each component, we could instead create a higher-order selector that keeps a cache of selectors and instantiates a new instance only when necessary. We could call this concept an Indexed Selector.

const indexResolver = (state, index) => index

export function createIndexedSelector(selectorFactory, resolver = indexResolver) {
  const cache = {}
  return (...args) => {
    const key = resolver(...args)
    const selector = cache[key] || (cache[key] = selectorFactory())
    return selector(...args)
  }
}

Note that for convenience and generalizability, I’ve opted to pass the index to selectors instead of a props object by default, but you can use your own resolver if you'd rather pass in your whole props object to your selectors. Rewriting the docs example to use this pattern, we get:

import { createSelector, createIndexedSelector } from 'reselect'

const getVisibilityFilter = (state, index) =>
  state.todoLists[index].visibilityFilter

const getTodos = (state, index) =>
  state.todoLists[index].todos

const getVisibleTodos = createIndexedSelector(() => {
  return createSelector(
    [ getVisibilityFilter, getTodos ],
    (visibilityFilter, todos) => {
      switch (visibilityFilter) {
        case 'SHOW_COMPLETED':
          return todos.filter(todo => todo.completed)
        case 'SHOW_ACTIVE':
          return todos.filter(todo => !todo.completed)
        default:
          return todos
      }
    }
  )
})

export default getVisibleTodos

const mapStateToProps = (state, props) => {
  return {
    todos: getVisibleTodos(state, props.listId)
  }
}

This has the added bonus of simplifying our connector since we no longer need to use makeMapStateToProps!

Let me know of anything I might be overlooking or further changes you’d like to discuss.

[coveralls]

Coverage remained the same at 100.0% when pulling 73ffb42 on 1000hz:indexed-selectors into 1143dcb on reactjs:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 73ffb42 on 1000hz:indexed-selectors into 1143dcb on reactjs:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 73ffb42 on 1000hz:indexed-selectors into 1143dcb on reactjs:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 73ffb42 on 1000hz:indexed-selectors into 1143dcb on reactjs:master.

[codecov-io]

Current coverage is 100% (diff: 100%)

Merging #213 into master will not change coverage

@@           master   #213   diff @@
====================================
  Files           1      1          
  Lines          12     13     +1   
  Methods         0      0          
  Messages        0      0          
  Branches        0      0          
====================================
+ Hits           12     13     +1   
  Misses          0      0          
  Partials        0      0          

Powered by Codecov. Last update 1143dcb...73ffb42

[walerun]

It looks interesting. But what if components have unique key/id each time when we create them. If we will delete some component, it means cache will have an unused selector, wich will never invoke. It may cause cache growth. Though maybe it's not a big problem.
I have one more question. Maybe it is not related to current PR, but how can we combine components selectors to create a new selector?

[toomuchdesign]

Hi all! I've just published a tiny plugin to memoize a collection of selectors based on a resolver function for a project of mines.

It works more or less as Lodash's .memoize() does, but with almost zero configuration. The name is re-reselect and it's just a few line of code without dependencies.

If externalizing to another micro-package is ok, It should fit this multi-component scenario, too.

Ping me for any info.
Thank's for the great work you're doing on Reselect. Cheers!

[drcmda]

@ellbee @1000hz @toomuchdesign

Could something like create[Indexed/Cached/Mapped]Selector be officially taken in?

Our team is kind of scared committing to reselect because in essence it would force us to use the factory pattern pretty much all the time.

We feel like the least use-case, one that rarely applies to a real world scenario, has been made default while in the majority of cases mapStateToProps gets padded with boilerplate which offloads the cognitive load to the component designer that now has to think through intricacies that shouldn't be of concern.

[toomuchdesign]

@drcmda

Your point is the reason why I published re-reselect. I just moved a memoized reselect factory into a separate package declaring reselect as peerDependency.

[ellbee]

I think what I'll probably do here is advertise re-reselect in the documentation, and if it turns out to be very popular then we will pull it into reselect itself.

[bilby91]

I'm using re-reselect now and was a smooth transition with minor adjustments to my code base. Works really good! Thanks @toomuchdesign !