CODEWITHSUNDEEP
reactjsreduxreselect
Augustin Riedinger
Augustin Riedinger

Reputation: 22150

Reselect multiple instance memoization

I using React/Redux/Reselect.

From the reselect doc, if I have multiple component that will use the selector with different parameters, I need to create one per instance.

const makeGetVisibleTodos = () => {
  return createSelector(
    [ 
       (state, props) => props.visibilityFilter, 
       state => state.todos,
    ],
    (visibilityFilter, todos) => {
       return todos.filter(td => td.visibility === visibilityFilter)
      }
    }
  )
}

But in my case, the listId can come from several origins (Eg. props.listId, props.location.match.listId, props.location.search.listId etc.)

So I feel more like writing the following:

const makeGetVisibleTodos = listId => {
  return createSelector(
    [
      state => state.todos,
    ],
    (todos) => {
       todos.filter(td => td.listId === listId)
      }
    }
  )
}

and connect it the following way:

  connect(
    (state, ownProps) => ({
      offerer: makeGetVisibleTodos(ownProps.location.match.listId)(state, ownProps),
    }),
  )

I'm pretty sure it works, but I'm not 100% sure this will memoize correctly:

If two components call makeGetVisibleTodos with the same listId, they will have 2 different cache values, right? Which is not what I want ...

How about this?

const makeGetVisibleTodos = listId => {
  return createSelector(
    state => state.todos,
    state => listId,
    (todos, listId) => {
       todos.filter(td => td.listId === listId)
      }
    }
  )
}

In this case, do makeGetVisibleTodos(ownProps.listId) and makeGetVisibleTodos(ownProps.match.params.listId) share the same cache value, when ownProps.listId === ownProps.match.params.listId?

Or another way to formulate the question is: how to pass extra-parameters that wouldn't (directly) depend on state and ownProps but are checked on equality during memoization?

I could also extend ownProps:

  connect(
    (state, ownProps) => ({
      offerer: makeGetVisibleTodos()(state, Object.assign({}, ownProps, {listId: ownProps.location.match.listId}),
    }),
  )

But find it super ugly and it's loosing the point...

Upvotes: 1

Views: 2561

Answers (3)

Andrea Carraro
Andrea Carraro

Reputation: 10419

As the author of re-reselect (a small wrapper around reselect) I'd like to point out how the library would solve your use case.

re-reselect provides selectors which retains memoization across different components out of the box.

import createCachedSelector from 're-reselect';

const getVisibleTodos = createCachedSelector(
  state => state.todos,
  (state, listId) => listId,
  (todos, listId) => {
     todos.filter(td => td.listId === listId)
    }
  }
)(
  (todos, listId) => listId, // Create/use a different selector for each different listId
);

Use it like a normal selector (in any different container component) and forget about selectors factories:

const makeMapStateToProps = () => {
  const mapStateToProps = (state, props) => {
    return {
      todos: getVisibleTodos(state, props.params.match.listId),
      todos2: getVisibleTodos(state, props.listId),
      todos3: getVisibleTodos(state, 'hardcodedListId')
    }
  }
  return mapStateToProps
}

This use case is also described in re-reselect docs.

Upvotes: 2

Niekert
Niekert

Reputation: 947

In the example you posted above, memoization will not work correctly because you create a new instance of the reselect selector everytime the mapStateToProps function is called.

Notice how in the docs the mapStateToProps function also becomes a factory function (makeMapStateToProps) and the makeGetVisibleTodos is called before returning the actual mapStateToProps function, like so.

const makeMapStateToProps = () => {
  const getVisibleTodos = makeGetVisibleTodos()
  const mapStateToProps = (state, props) => {
    return {
      todos: getVisibleTodos(state, props)
    }
  }
  return mapStateToProps
}

Also, you do not pass the props that you use in your selector in the makeGetVisibleTodos() call, but when actually calling the selector itself.

This would result in something like

const makeGetVisibleTodos = () => {
  return createSelector(
    state => state.todos,
    state => (_, listId), // Here you have access to all the arguments passed
    (todos, listId) => {
       todos.filter(td => td.listId === listId)
      }
    }
  )
}

Then you can write the selectors like this:

const makeMapStateToProps = () => {
  const getVisibleTodos = makeGetVisibleTodos()
  const mapStateToProps = (state, props) => {

    return {
      todos: getVisibleTodos(state, props.params.match.listId),
      todos2: getVisibleTodos(state, props.listId),
      todos3: getVisibleTodos(state, 'hardcodedListId')
    }
  }
  return mapStateToProps
}

Upvotes: 2

Shubham Khatri
Shubham Khatri

Reputation: 281646

Your method will not do memoization correctly since when you call makeGetVisibleTodos is always creating a new selector. You can improve on it by writing

const listIdQuerySelector = (state, props) => {
    return props.match.params && props.match.params.listId;
};
const todoSelector = createSelector(
    [
      listIdQuerySelector,
      state => state.todos,
    ],
    (listId, todos) => {
       todos.filter(td => td.listId === listId)
      }
    }
  )

and use it like

connect(
    (state, ownProps) => ({
      offerer: makeGetVisibleTodos(state, ownProps),
    }),
  )

Upvotes: 0

Related Questions