feat: Add queryKey to QueryObserverResult

[patrick-mcdougle]

When using useQueries it personally makes me nervous that you have
to rely on the array order to remap the queryKeys. By passing the
queryKey back through into the QueryObserverResult, this remapping
can more easily be done by the code that does the result
processing.

This feature was suggested here:
#2624 (reply in thread)

Disclaimer: This is my first contribution to react-query so please let me know if I didn't follow any required conventions.

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/tannerlinsley/react-query/FNECHwG59E6UycWuRwNN97pWRSKu
✅ Preview: https://react-query-git-fork-patrick-mcdougle-key-0f5bc7-tannerlinsley.vercel.app

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 6a10e0f:

Sandbox	Source
tannerlinsley/react-query: basic	Configuration
tannerlinsley/react-query: basic-typescript	Configuration

[TkDodo]

not sure I understand that. Why would we be interested in the key of the previous query?

[patrick-mcdougle]

Without this we lost some memoization because the queryKey array was new each time the hook was called.

Said another way: prevQueryResult.queryKey !== query.queryKey (but they are shallow equal).

The only reason I caught that was because a test failed.

[TkDodo]

Is it important to keep referential stability between queryKeys? If yes, we should likely triple equal compare the query hashes.

What I don’t understand is how this can work with changing keys. For example: having ['todos', id] as key and id transitions from 1 to 2. The previous key would be ['todos', 1] and the new one is ['todos', 2] - but with the logic above we would get the old, wrong key returned, wouldn’t we..?

[patrick-mcdougle]

I need to read the code a little more closely, but it's my understanding that each time createResult on a QueryObserver is called, it's always the same "query" (that is, a unique queryKey [hash]) but the result is being updated with new information (retry, success, error, the hook being called again with no update, etc). Under this understanding, changed keys would result in two instances of a QueryObserver (one for id #1 and another for id #2 in your example above). Are QueryObservers pooled in the way you're suggesting, or is there one QueryObserver per unique queryHash?

I'm going to go read the code again and I'll check back.

[patrick-mcdougle]

So I think this needs to change to:

const queryKey = queryChange ? query.queryKey : prevQuery.queryKey

but this is causing a test failure that I'm going to need to figure out. Help would be great. I think it has to do with the shallow result checking in updateResult ~L623 since the queryKey is an array not a primitive like everything else.

[patrick-mcdougle]

Moving this to a draft to prevent accidental merges while we work through the issue in the comments.

[TkDodo]

not going to merge this for now. What you can always do is relay everything back via the queryFn, like:

const results = useQueries(array.map(x => ({
  queryKey: ['foo', x],
  queryFn: async ({ queryKey }) => {
    const { data } = await api.goFetchMyData(...)
    return { ...data, queryKey }
  }
})))

that way, queryKey or whatever else you want will be available in results.map(x => x.data.queryKey))

hope that helps!