fix(openapi-react-query): add client parameter to queryKey

[jeiea]

Background

• I opened and close feat(react-query): Add client parameter to queryKey and remove options parameter #1979.

How to Review

• I'm unclear on the purpose of the added queryOptions parameter. If it's necessary, last commit can be excluded.
  options parameter is preserved for not being breaking.

Checklist

• Unit tests updated
• docs/ updated (if necessary)
• pnpm run update:examples run (only applicable for openapi-typescript)

[changeset-bot]

🦋 Changeset detected

Latest commit: f056e57

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
openapi-react-query	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[kerwanp]

Thanks for the contribution! It is a really odd behavior that keys conflicts even tho you are using different clients as each one should have its own QueryCache. Do you have any example that shows the conflict in action?

[jeiea]

I believe my test illustrates the issue well. I ran the added test on the revision before this PR, and the test failed. The test uses useQueries with different openapi-typescript clients. Before this PR, the running query count was 1, which I think demonstrates the problem.

... each one should have its own QueryCache.

From my understanding, the TanStack Query Client manages the QueryCache, and it is either provided globally or through a provider.

To be treated as separate queries, at least one of the following must be different:

• Query client (specifically TanStack’s, not from openapi-typescript or openapi-react-query)
• Query key

Typically, the query client is shared, and as shown in this fix, the query key was the same, which led me to suspect that this was the issue.

packages/openapi-react-query test: . test:js$ vitest run
packages/openapi-react-query test: . test:js:  RUN  v2.1.3 /Users/jeiea/Documents/openapi-typescript/packages/openapi-react-query
packages/openapi-react-query test: . test:js: (node:41850) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
packages/openapi-react-query test: . test:js: (Use `node --trace-deprecation ...` to show where the warning was created)
packages/openapi-react-query test: . test:js:  ❯ test/index.test.tsx  (24 tests | 1 failed) 563ms
packages/openapi-react-query test: . test:js:    × client > queryOptions > should treat different fetch clients as separate instances
packages/openapi-react-query test: . test:js:      → expected 1 to be 2 // Object.is equality
packages/openapi-react-query test: . test:js: ⎯⎯⎯⎯⎯⎯⎯ Failed Tests 1 ⎯⎯⎯⎯⎯⎯⎯
packages/openapi-react-query test: . test:js:  FAIL  test/index.test.tsx > client > queryOptions > should treat different fetch clients as separate instances
packages/openapi-react-query test: . test:js: AssertionError: expected 1 to be 2 // Object.is equality
packages/openapi-react-query test: . test:js: - Expected
packages/openapi-react-query test: . test:js: + Received
packages/openapi-react-query test: . test:js: - 2
packages/openapi-react-query test: . test:js: + 1
packages/openapi-react-query test: . test:js:  ❯ test/index.test.tsx:256:40
packages/openapi-react-query test: . test:js:     254| 
packages/openapi-react-query test: . test:js:     255|       // client1 and client11 have the same fetch client, so they shar…
packages/openapi-react-query test: . test:js:     256|       expect(queryClient.isFetching()).toBe(2);
packages/openapi-react-query test: . test:js:        |                                        ^
packages/openapi-react-query test: . test:js:     257|     });
packages/openapi-react-query test: . test:js:     258|   });
packages/openapi-react-query test: . test:js: ⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯⎯[1/1]⎯
packages/openapi-react-query test: . test:js:  Test Files  1 failed (1)
packages/openapi-react-query test: . test:js:       Tests  1 failed | 23 passed (24)
packages/openapi-react-query test: . test:js:    Start at  12:32:53
packages/openapi-react-query test: . test:js:    Duration  1.73s (transform 96ms, setup 0ms, collect 369ms, tests 563ms, environment 451ms, prepare 63ms)
packages/openapi-react-query test: . test:js: Failed
packages/openapi-react-query test:  ELIFECYCLE  Command failed with exit code 1.
packages/openapi-react-query test: Failed

[kerwanp]

Thanks for the detailed explanation. I think we should keep the original behavior as it is how react query should behave: One query client, same cache. You must provide a different query client explicitly or implicitly through a provider.

I don't see any reason why you would have this use case but with the introduced changes in this PR it would not be possible anymore to have two different fetchClient that "share the same cache".

const client1 = createFetchClient<api1>()
const client2 = createFetchClient<api2>()

const query1 = createQueryClient(client1)
const query2 = createQueryClient(client2)

// They can share the same keys
query1.useQuery('get', '/posts')
query2.useQuery('get', '/posts')

query2.useQuery('get', '/posts', {}, queryClient2) // Use a different QueryClient

[jeiea]

query2.useQuery('get', '/posts', {}, queryClient2) // Use a different QueryClient

So, from my understanding, if there are multiple base urls, then queryClient2 must always be provided whenever query2 is used, correct?

I hadn’t initially considered sharing cache key, but I can see how it might be desirable if the only difference lies in the middleware of openapi-typescript.

In a situation where two different baseUrl values are used, I wouldn’t typically expect to use two separate TanStack Query clients if we’re only using TanStack Query without openapi-react-query. So, this seems to be a limitation of openapi-react-query.

One advantage of maintaining this limitation, as mentioned, is the ability to treat different openapi-fetch clients as interchangeable. Although I don’t have direct experience with wanting to unify clients that only differ in middleware, I can imagine scenarios—such as when APIs requiring and not requiring authentication are mixed within the same endpoint. In that case, we might prefer handling authentication tokens in middleware rather than including them in cache keys. However, it’s still unclear under what circumstances "the same cache key" must be shared in this scenario since unauthenticated and authenticated responses are likely to differ significantly.

On the other hand, when openapi-fetch clients are different, they may have distinct baseUrls, and it feels more natural for their cache keys to differ given the possibility of entirely different requests.

Could you provide an example of the highlighted scenario above?

[kerwanp]

Could you provide an example of the highlighted scenario above?

I never had encountered a requirement like this, but we could imagine something with API versioning and a persistent storage for cache where you incrementally move usage from one fetch client (API v1) to second fetch client (API v2). You might still want your user to have the initial cache from API v1 even after moving some parts of your app to API v2.

Anyway we are covering really uncommon cases. And in fact we already provide a way to cover both cases by allowing the user to provide a specific query client but I can understand this might be painful.

A solution to cover both cases would be to give to the user the ability to provide a prefixQueryKey when creating the openapi query client that would replace the clientId. This way the default behavior is the one intended by react-query but still give an easy way to have different keys with the same QueryClient:

const client1 = createQueryClient(fetchClient, { prefixQueryKey: ['cache2'] })
const client2 = createQueryClient(fetchClient, { prefixQueryKey: ['cache1'] })

[jeiea]

The example you provided seems plausible, and the suggested approach looks good.

Below are some detailed considerations I thought about, and I’m curious if you have any other opinions on them. They are listed in order of preference:

• Type of prefixQueryKey: string, unknown, unknown[]
• Default value of prefixQueryKey: '', null, []

The reason I believe a string key is sufficient is that React already uses string as key, and verifying that an unknown type doesn’t introduce serialization issues can be challenging.

[kerwanp]

The reason I believe a string key is sufficient is that React already uses string as key, and verifying that an unknown type doesn’t introduce serialization issues can be challenging.

I don't think we should think about react keys but about react-query keys. The default value should be undefined and the type must match the queryKey option of react-query. When defined, it is unshifted to the queryKey that openapi-react-query define in the queryOptions method.

The type of QueryKey is ReadonlyArray<unknown>.