Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
padge opened this issue Apr 22, 2024 · 8 comments

Comments

@padge
Copy link

padge commented Apr 22, 2024

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
Copy link
Member

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
Copy link
Author

padge commented Apr 22, 2024

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
Copy link
Collaborator

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
Copy link
Author

padge commented Apr 22, 2024

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
Copy link
Author

padge commented Apr 22, 2024

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
Copy link
Member

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

The example I had above contains multiple selectors:

  • createInventorySelector
  • the selector created by .select(id)
  • selectInventory

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
Copy link
Collaborator

markerikson commented Apr 22, 2024

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
Copy link
Author

padge commented Apr 22, 2024

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants