reduxjs / redux-toolkit

The official, opinionated, batteries-included toolset for efficient Redux development
https://redux-toolkit.js.org
MIT License
10.72k stars 1.17k forks source link

Help - How to memoize RTKQ selector with dynamic argument from state, not selector arg. #4362

Open padge opened 6 months ago

padge commented 6 months ago

Looking for help on this, as I can't wrap my head around how to get a proper memoized selector using RTKQ... The selector needs the gameId from store state, not a selector arg.

This is what I have right now that isn't memoizing:

const createInventorySelector = createSelector(
  (id: string) => id,
  (id) => api.endpoints.listInventoryForGame.select(id),
);
export const selectInventory = createSelector(
  (state: RootState) => state,
  selectGameId,
  (state: RootState, gameId) => createInventorySelector(gameId)(state),
);

const inventory = selectInventory(state);

I know state => state isn't good, but I'm trying to follow the example here https://redux-toolkit.js.org/rtk-query/usage/usage-without-react-hooks#memoization, and don't see how to do it otherwise.

Thanks.

phryneas commented 6 months ago

More like this.

const createInventorySelector = createSelector(
  selectGameId,
  (id) => api.endpoints.listInventoryForGame.select(id),
);
export const selectInventory = createSelector(
  (state: RootState) => state,
  createInventorySelector,
  (state: RootState, inventorySelector) => inventorySelector(state),
);

const inventory = selectInventory(state);
padge commented 6 months ago

Ahh ok, thank you for the correction. Unfortunately it's still not being memoized correctly, as the number of recomputations is the same as before (I'm using checkSelector from "reselect-tools"). It looks like it's recomputing after every dispatched action.

markerikson commented 6 months ago

Yes, a selector that has (state) => state is effectively broken and will never memoize correctly. Please don't do that :(

(In fact, Reselect ought to be warning you about that in dev mode.)

padge commented 6 months ago

Yeah okay, good to know. I was wondering if it was even possible. I suppose it's probably not necessary, given that the selector isn't doing any heavy computation? If that's the case, it might be worth adding a caveat to the selectGetPostError example in the docs.

Do you have any recommendations for a better solution, or do I just leave that selector as is and handle memoization on the selectors that depend on that selector?

padge commented 6 months ago

I tried something like this which was recomputed much less, but it doesn't seem ideal (accessing rtkq implementation details?):

export const selectInventory = createSelector(
  (state: RootState) => state.api.queries,
  selectGameId,
  (queries, gameId) => {
    return queries[`listInventoryForGame("${gameId}")`];
  },
);
phryneas commented 6 months ago

The question is also at which selector you are looking here.

The example I had above contains multiple selectors:

While selectInventory recalculates every time, the other two do not.

Realistically, you can rewrite my example up there to

const createInventorySelector = createSelector(
  selectGameId,
  (id) => api.endpoints.listInventoryForGame.select(id),
);

const selector = createInventorySelector(state)
const inventory = selector (state);

and both createInventorySelector and selector will recalculate significantly fewer times (they also would recalculate the same amount of times in the example with selectInventory, it just depends at which of those you look).

markerikson commented 6 months ago

Query selectors are created internally using createSelector, but strictly speaking don't benefit from being memoized (it's ultimately a straight lookup as you've got there) I was reminded they do derive the status flags.

That said, it feels like this might be easier if you skip the createSelector part:

const selectInventory = (state: RootState) => {
  const id = selectGameId(state);
  const inventorySelector = api.endpoints.listInventoryForGame.select(id)
  return inventorySelector(state);
}

or if you wanted to save on the slight cost of generating the per-ID selectors:

const selectorsForIds = {};

const selectInventory = (state: RootState) => {
  const id = selectGameId(state);

  if (!(id in selectorsForIds)) {
    selectorsForIds[id] = api.endpoints.listInventoryForGame.select(id)
  }
  const inventorySelector = selectorsForIds[id];
  return inventorySelector(state);
}
padge commented 6 months ago

Right okay, that makes sense. I like the example of not using createSelector, as it's more explicit that it won't be memoized since it relies on state => state. Thank you both for the help!