Reselect: Selector with dynamic argument not memoized

Created on 13 Jan 2019  路  5Comments  路  Source: reduxjs/reselect

Hello,

Sorry if this has already been discussed but I can't find a way to make it work for my use case.

My selector:

import { createSelector, defaultMemoize } from 'reselect'

const allStatesSelector = store => store.state.states

export const getAllStates = createSelector(
  allStatesSelector,
  states => defaultMemoize(night => {
    const availableStates = night ? nightStates : dayStates
    return states.filter(state => availableStates.includes(state.id))
  }),
)

I'm calling it that way:

getAllStates(store)(props.night)

The selector is executed 1800 times, instead of 2 times (because there's only 2 possibilities, night is either true or false).

What did I miss?

Most helpful comment

Okay.

First, it's important to understand that Reselect takes the "input selectors" you provide, and calls them _every_ time. It then inspects the results of those "input selectors". If any of the return values are different than last time, then it calls the "output selector" with all of those values as arguments. If _all_ of the return values are the same as before, then it skips calling the output selector and returns the cached result.

So, your allStatesSelector is being used as an "input selector", and thus will be called every time.

The way to think about this is, "I only want my output selector to run whenever one of these values changes".

Similarly, you don't ever want to have an "output selector" that just returns the argument, like state => state. That's not doing anything useful.

Also, you can pass multiple arguments to a Reselect selector, but you then need to write "input selectors" that extract those arguments and pass them on to the "output selector".

So, as a basic example:

const selectAllStates = state => state.states;
const selectIsNight = (state, isNight) => isNight;

const selectMatchingStates = createSelector(
    [selectAllStates, selectIsNight],
    (allStates, isNight) => {
        const availableStates = isNight? nightStates : dayStates
        return states.filter(state => availableStates.includes(state.id))
    }
);

You might want to read my post Idiomatic Redux: Using Reselect Selectors for Encapsulation and Performance for some further guidance.

All 5 comments

Two questions:

  • Why are you calling defaultMemoize() yourself?
  • Why are you having your "output selector" return a separate function?

Also:

  • What does your data look like?
  • Where are you counting "1800 times"?
  • Where and when are you calling the selector?

Hello!

I've tried many things hence why I ended up with this code xD

This version seemed more legit but still was being excecuted ~1800 times (which is the number of call I'm making to this selector during one render).

import { createSelector } from 'reselect'

const allStatesSelector = (store, night) => {
  const { states } = store.state

  console.log('allStatesSelector executed with night = ', night)

  const availableStates = night ? nightStates : dayStates
  return states.filter(state => availableStates.includes(state.id))
}

export const getAllStates = createSelector(
  allStatesSelector,
  states => states,
)

Called like so:

getAllStates(state, props.night)

My data looks like :

nightStates:

const nightStates = [5, 6, 7, 10, 11, 12, 13]

dayStates:

const nightStates = [1, 2, 3, 4, 6, 7, 10, 11, 12, 13]

store.state.states:

{
  states: [
    {
      isWorking: true,
      isRegularWorking: true,
      id: 1,
      display: '0',
      hours: 8
    },
    {
      isWorking: true,
      isRegularWorking: true,
      id: 2,
      display: '0',
      hours: 8
    },
    {

      isWorking: true,
      isRegularWorking: true,
      id: 3,
      display: 'C',
      hours: 10
    },
    {

      isWorking: true,
      isRegularWorking: true,
      id: 4,
      display: 'C',
      hours: 10
    },
    {

      isWorking: true,
      isRegularWorking: true,
      id: 5,
      display: 'C',
      hours: 12
    },
    {

      isWorking: true,
      isRegularWorking: false,
      id: 6,
      display: 'C',
      hours: 12
    },
    {

      isWorking: false,
      isRegularWorking: false,
      id: 7,
      display: 'A',
      hours: 8
    },
    {

      isWorking: false,
      isRegularWorking: false,
      id: 10,
      display: 'R',
      hours: 0
    },
    {

      isWorking: false,
      isRegularWorking: false,
      id: 11,
      display: 'V',
      hours: 0
    },
    {

      isWorking: false,
      isRegularWorking: false,
      id: 12,
      display: 'S',
      hours: 0
    },
    {

      isWorking: false,
      isRegularWorking: false,
      id: 13,
      display: 'M',
      hours: 0
    }
  ]
}

The selector is called in a component displayed 1800 times in the page (basically that's a table cell).
I know that I could retrieve the selector on the parent component and pass it to the cell, but it'll still be called multiple times and I want to get this memoization to work anyway :D

Okay.

First, it's important to understand that Reselect takes the "input selectors" you provide, and calls them _every_ time. It then inspects the results of those "input selectors". If any of the return values are different than last time, then it calls the "output selector" with all of those values as arguments. If _all_ of the return values are the same as before, then it skips calling the output selector and returns the cached result.

So, your allStatesSelector is being used as an "input selector", and thus will be called every time.

The way to think about this is, "I only want my output selector to run whenever one of these values changes".

Similarly, you don't ever want to have an "output selector" that just returns the argument, like state => state. That's not doing anything useful.

Also, you can pass multiple arguments to a Reselect selector, but you then need to write "input selectors" that extract those arguments and pass them on to the "output selector".

So, as a basic example:

const selectAllStates = state => state.states;
const selectIsNight = (state, isNight) => isNight;

const selectMatchingStates = createSelector(
    [selectAllStates, selectIsNight],
    (allStates, isNight) => {
        const availableStates = isNight? nightStates : dayStates
        return states.filter(state => availableStates.includes(state.id))
    }
);

You might want to read my post Idiomatic Redux: Using Reselect Selectors for Encapsulation and Performance for some further guidance.

Great thank you!

It's still not working but I know why now... It's because my selectors are called in an alternance of night = true and night = false.

I thought the memoization would take place for every argument, like if you called it with a certain set of arguments, it will memoize it until the store change and not only if it was called with the same arguments last time :/

Yup. If you need to call the same selector multiple times in a row with different arguments, you'll need to create distinct instances of that selector so they each memoize properly. See the "factory functions" section of the post I linked for details on that.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

clementdevos picture clementdevos  路  4Comments

nickzarate picture nickzarate  路  8Comments

0x80 picture 0x80  路  5Comments

jEnbuska picture jEnbuska  路  6Comments

maxim1006 picture maxim1006  路  3Comments