RTK Query: selectFromResult must return an object (docs could be more clear on this)

[gillycheesesteak]

Edited based on discussion below

The docs could be more clear about the expected/required return value from selectFromResult (and maybe reasons for those expectations)

Original issue:

As the title states, if the selectFromResult function returns a value directly, it results in the following error:

rtk-query-react.esm.js:318 Uncaught TypeError: Cannot read properties of undefined (reading 'data')

The issue appears to stem from useQuery relying on useQueryState which returns undefined after a few renders, causing the above error on this line: https://github.com/reduxjs/redux-toolkit/blob/e45425128d3c3168c7daa71e5f38f5151234cb8d/packages/toolkit/src/query/react/buildHooks.ts#L963-L972

This behavior may be intentional - if so, the docs should be updated to reflect that.

[phryneas]

The problem is not data (that would destructive fine if undefined), but the object it is on. Does your selectFromResult maybe return undefined in some conditions?

[gillycheesesteak]

If it does, shouldn't that just get passed along similar to when the return value is { data: undefined }?

[phryneas]

No, selectFromResult replaces the whole result object including isLoading and everything - it allows you to optimize renders, but also means that you are responsible about the whole return value now.

[gillycheesesteak]

In that case, I believe the documentation here is misleading: https://redux-toolkit.js.org/rtk-query/usage/queries#selecting-data-from-a-query-result

[gillycheesesteak]

This line should call out that the return value should mimic a response (an object with the relevant data as a prop)

https://github.com/reduxjs/redux-toolkit/blob/e45425128d3c3168c7daa71e5f38f5151234cb8d/docs/rtk-query/usage/queries.mdx#L277

And this paragraph should be more specific about what constitutes a "new object on every render" given that the code example returns a new object with a post property:

https://github.com/reduxjs/redux-toolkit/blob/e45425128d3c3168c7daa71e5f38f5151234cb8d/docs/rtk-query/usage/queries.mdx#L306

[phryneas]

You somehow have the misconception that it should return { data }. That is not the case. The example returns a post property and exactly that is the return value of the hook: { post }. There is no data and it is not required. You get the error because your selectFromResult returns undefined, you would not get that error message if you returned an existing object without data.

[gillycheesesteak]

No, I understand what you are saying - I still think the documentation is misleading about what format is expected (an object, at minimum) and this paragraph does not line up with the example given.

If a new array/object is created and used as a return value within the callback, it will hinder the performance benefits due to being identified as a new item each time the callback is run.

[gillycheesesteak]

Consider the way a normal selector in redux would be used

const value = useSelector((state) => state.some.value)

The impression I got from the docs as written is that this should work, since there is no mention of needing to return an object or something "response-like"

[phryneas]

It does not need to be "response like". You can return any object. { foo: "bar"} is perfectly fine.

There is no requirement to the shape of the return value apart from it being an object, however that might look. I honestly do not understand what you mean with"response like" at this point.

[phryneas]

I'm on mobile here, so I try my best to answer you and I might be missing something here, so please bear with me.

[phryneas]

Yeah rereading this we should probably add some runtime warning in dev mode as well as a warning in the docs - I had assumed that the docs were already pretty clear showing a return object and destructuring even though it's only one value.

[markerikson]

The biggest caveat with returning a primitive here is that we still try to attach methods to the object like refetch, so if it isn't an object that won't work at all.

[gillycheesesteak]

It does not need to be "response like". You can return any object. { foo: "bar"} is perfectly fine.

Understood. I understand what I was doing wrong now, but I think the docs could be more clear.

The fact that some lifecycle methods will be added to whatever you return is why I was calling it "response-like". It means that your actual selected value should be a property of the object you return, not returned directly from selectFromResult

[markerikson]

We should probably add a dev check for this.

[KasimAhmic]

Just to make sure I'm understanding all this correctly, returning a primitive from selectFromResult should be avoided and we should always be returning some sort of object? If so, will the reference to the returned object be stable to prevent needless re-renders?

The reason I ask is because we've been doing something to this effect in production for quite some time now with seemingly no issues:

interface InventoryItem {
  id: string;
  price: number;
}

const inventoryAdapter = createEntityAdapter<InventoryItem, string>();
const inventorySelectors = inventoryAdapter.selectors;

export function useInventorySelector<T extends (items: EntityState<InventoryItem, string>) => ReturnType<T>>(
  saleId: string,
  selector?: T,
) {
  return api.getInventory.useQueryState(saleId, {
    selectFromResult: (state) => selector?.(state.data),
  });
}

export const InventoryItem: FC = ({ id }) => {
  const saleId = useAppSelector(selectSaleId);

  const price = useInventorySelector((items) => inventorySelectors.selectById(items, id).price);
  // price is type number
}

The types here are a little idealistic and in reality, we have to do a bit more to get this past the type checker but we've noticed no issues with this approach. Though in hindsight, I suppose how much we had to fight with the type checker should've been our first indication that this should've been avoided...

[phryneas]

@KasimAhmic We do add a refetch property, so your result here will probably be a very weird Number object instance with a refetch property, not a primitive number.

[EskiMojo14]

if you're just using useQueryState then you probably won't have any issues, but useQuery spreads the results of both useQueryState and useQuerySubscription into a new object, so that will likely not work correctly.

[KasimAhmic]

@KasimAhmic We do add a refetch property, so your result here will probably be a very weird Number object instance with a refetch property, not a primitive number.

@phryneas Unless I'm missing something here, that doesn't appear to be the case. Made a small reproduction repo showing this here. It appears that useQueryState doesn't attempt to add anything to the returned values.

---

if you're just using useQueryState then you probably won't have any issues, but useQuery spreads the results of both useQueryState and useQuerySubscription into a new object, so that will likely not work correctly.

@EskiMojo14 Ah I'm just now realizing this discussion was focusing on useQuery, my bad. Should I open a separate issue to discuss this further then?

EDIT: Opened a new issue to move the discussion there: #3989