Skip to content

Commit 3539f82

Browse files
authored
fix(query): namespace non-GET query keys with the HTTP verb (orval-labs#3312)
* fix(query): namespace non-GET query keys with the HTTP verb When a non-GET operation is routed to a Query hook (today only via operation-level `query.useQuery: true`, and the upcoming fix for orval-labs#2376), its generated cache key was path-only — e.g. both `GET /pets` and `POST /pets` produced keys starting with `'/pets'`. TanStack Query then mixed their cached data and `invalidateQueries({ queryKey: ['/pets'] })` matched both, which is a silent runtime bug (no type error, no snapshot diff). Insert the uppercased HTTP verb as a leading segment when: - `verb !== GET`, AND - `useOperationIdAsQueryKey` is not enabled (operation IDs are already unique across verb + path, so the prefix would be redundant). Existing GET keys and operation-id-based keys are unchanged. This unblocks orval-labs#2376 by making it safe for the eventual fix to globally route non-GET operations to Query hooks without cache key collisions. BREAKING CHANGE: Users that already opt non-GET operations into Query hooks via `override.operations.<id>.query.useQuery: true` will see their cache keys gain a leading verb segment (e.g. `['/pets', body]` → `['POST', '/pets', body]`). Update any `invalidateQueries` / `getQueryData` calls that filter by these keys. * fix(query): keep broad invalidation working for verb-prefixed cache keys Address Copilot review on orval-labs#3312: when a non-GET operation is routed to a Query hook and a `mutationInvalidates` rule targets it without providing the required path params, the broad-invalidation fallback emitted a predicate / partial key that no longer matched the cache key (because the cache key now starts with the verb segment). Mirror `getQueryKeyVerbPrefix`'s behavior in `mutation-generator.ts`: - Default mode: predicate becomes `query.queryKey[0] === 'DELETE' && query.queryKey[1].startsWith('/pets/')`. - Split-key mode: partial key becomes `['DELETE', 'pets']`. Verb prefix is suppressed for GET targets (existing behavior preserved) and when `useOperationIdAsQueryKey` is enabled (operation IDs are already verb+path unique, so `getQueryKeyVerbPrefix` does not insert a prefix and neither should the invalidation logic). Also extends the `getQueryKeyVerbPrefix` unit tests to cover HEAD per Copilot feedback, so all `Verbs` enum members are exercised. Two new snapshot configs document the fixed behavior: - `invalidatesNonGetQueryTarget` — default mode. - `invalidatesNonGetQueryTargetSplitKey` — split-key mode. * refactor(query): reuse getQueryKeyVerbPrefix in mutation-generator Address CodeRabbit nitpick on orval-labs#3312: the verb prefix logic in `createGenerateInvalidateCall` duplicated the rules in `getQueryKeyVerbPrefix`. Importing the helper directly removes that duplication so the two sites stay in sync if the prefix rules ever change. `info.method` is widened by the spec walker to HTTP_METHODS (a superset of `Verbs` that also includes `options`/`trace`), but the helper only branches on `Verbs.GET`, so casting is safe — any non-GET method is treated identically.
1 parent f62674c commit 3539f82

47 files changed

Lines changed: 3052 additions & 8 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

packages/query/src/mutation-generator.ts

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import {
1111
isString,
1212
type OutputHttpClient,
1313
pascal,
14+
type Verbs,
1415
} from '@orval/core';
1516

1617
import {
@@ -19,6 +20,7 @@ import {
1920
getQueryErrorType,
2021
} from './client';
2122
import type { FrameworkAdapter } from './framework-adapter';
23+
import { getQueryKeyVerbPrefix } from './query-generator';
2224
import { getQueryOptionsDefinition } from './query-options';
2325

2426
interface NormalizedTarget {
@@ -62,6 +64,9 @@ const HTTP_METHODS = [
6264

6365
interface OperationRouteInfo {
6466
route: string;
67+
/** HTTP method (lowercase) — needed to mirror the verb prefix that
68+
* `getQueryKeyVerbPrefix` adds to non-GET cache keys. */
69+
method: string;
6570
/** true when the route has path params that lack a default value */
6671
hasRequiredPathParams: boolean;
6772
}
@@ -108,7 +113,7 @@ const findOperationInfo = (
108113
if (opId !== operationName && camel(opId) !== operationName) continue;
109114

110115
if (!routePath.includes('{')) {
111-
return { route: routePath, hasRequiredPathParams: false };
116+
return { route: routePath, method, hasRequiredPathParams: false };
112117
}
113118

114119
// Collect path parameters from both path-level and operation-level
@@ -121,7 +126,7 @@ const findOperationInfo = (
121126
(p) => p.schema?.default === undefined && p.default === undefined,
122127
);
123128

124-
return { route: routePath, hasRequiredPathParams };
129+
return { route: routePath, method, hasRequiredPathParams };
125130
}
126131
}
127132
return undefined;
@@ -204,6 +209,7 @@ const generateParamArgs = (
204209
const createGenerateInvalidateCall = (
205210
spec: Record<string, unknown> | undefined,
206211
shouldSplitQueryKey: boolean,
212+
useOperationIdAsQueryKey: boolean,
207213
) => {
208214
return (target: NormalizedTarget): string => {
209215
const method =
@@ -228,19 +234,42 @@ const createGenerateInvalidateCall = (
228234
// starts with a path param like /{tenantId}/...), fall through to
229235
// the zero-arg call rather than generating an overly-broad match.
230236
if (prefix !== undefined) {
237+
// Mirror the verb prefix that `getQueryKeyVerbPrefix` injects into
238+
// non-GET Query keys; without this, the predicate / partial key
239+
// would never match a verb-prefixed cache key and the broad
240+
// invalidation would silently no-op. We share the helper from
241+
// `query-generator.ts` so both sites stay in sync.
242+
// `info.method` is narrowed by the spec walker to one of HTTP_METHODS
243+
// (a superset of `Verbs` that also includes `options`/`trace`); the
244+
// helper only branches on `Verbs.GET`, so the cast is safe for any
245+
// non-GET method.
246+
const verbPrefix = getQueryKeyVerbPrefix({
247+
verb: info.method as Verbs,
248+
useOperationIdAsQueryKey,
249+
});
250+
231251
if (shouldSplitQueryKey) {
232-
// Split-key mode: query keys are arrays like ['pets', petId].
252+
// Split-key mode: query keys are arrays like ['pets', petId]
253+
// (or ['DELETE', 'pets', petId] for non-GET Query keys).
233254
// Use partial key matching with static route segments.
234255
const segments = prefix
235256
.split('/')
236257
.filter((s) => s !== '')
237258
.map((s) => `'${s}'`)
238259
.join(', ');
239-
return ` queryClient.${method}({ queryKey: [${segments}] });`;
260+
const keyArr = verbPrefix
261+
? `['${verbPrefix}', ${segments}]`
262+
: `[${segments}]`;
263+
return ` queryClient.${method}({ queryKey: ${keyArr} });`;
240264
}
241265

242-
// Default mode: query keys are template strings like ['/pets/${petId}'].
243-
// Use predicate with startsWith for broad matching.
266+
// Default mode: query keys are template strings like
267+
// ['/pets/${petId}'] (or ['DELETE', '/pets/${petId}'] for non-GET
268+
// Query keys). Use a predicate that knows where the route segment
269+
// lives in the tuple.
270+
if (verbPrefix) {
271+
return ` queryClient.${method}({ predicate: (query) => query.queryKey[0] === '${verbPrefix}' && typeof query.queryKey[1] === 'string' && query.queryKey[1].startsWith('${prefix}') });`;
272+
}
244273
return ` queryClient.${method}({ predicate: (query) => typeof query.queryKey[0] === 'string' && query.queryKey[0].startsWith('${prefix}') });`;
245274
}
246275
}
@@ -422,6 +451,7 @@ ${
422451
generateInvalidateCall: createGenerateInvalidateCall(
423452
context.spec,
424453
!!query.shouldSplitQueryKey,
454+
!!query.useOperationIdAsQueryKey,
425455
),
426456
uniqueInvalidates,
427457
})

packages/query/src/query-generator.test.ts

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
1+
import { Verbs } from '@orval/core';
12
import { describe, expect, it } from 'vitest';
23

3-
import { getMutationInvalidatesConflictWarning } from './query-generator';
4+
import {
5+
getMutationInvalidatesConflictWarning,
6+
getQueryKeyVerbPrefix,
7+
} from './query-generator';
48

59
const ruleFor = (op: string) => ({
610
onMutations: [op],
@@ -103,3 +107,65 @@ describe('getMutationInvalidatesConflictWarning', () => {
103107
expect(message).toContain("'createPets'");
104108
});
105109
});
110+
111+
describe('getQueryKeyVerbPrefix', () => {
112+
it('returns undefined for GET so existing GET keys remain unchanged', () => {
113+
expect(
114+
getQueryKeyVerbPrefix({
115+
verb: Verbs.GET,
116+
useOperationIdAsQueryKey: false,
117+
}),
118+
).toBeUndefined();
119+
});
120+
121+
it('returns the uppercased verb for non-GET verbs to disambiguate cache keys', () => {
122+
expect(
123+
getQueryKeyVerbPrefix({
124+
verb: Verbs.POST,
125+
useOperationIdAsQueryKey: false,
126+
}),
127+
).toBe('POST');
128+
expect(
129+
getQueryKeyVerbPrefix({
130+
verb: Verbs.PUT,
131+
useOperationIdAsQueryKey: false,
132+
}),
133+
).toBe('PUT');
134+
expect(
135+
getQueryKeyVerbPrefix({
136+
verb: Verbs.PATCH,
137+
useOperationIdAsQueryKey: false,
138+
}),
139+
).toBe('PATCH');
140+
expect(
141+
getQueryKeyVerbPrefix({
142+
verb: Verbs.DELETE,
143+
useOperationIdAsQueryKey: false,
144+
}),
145+
).toBe('DELETE');
146+
expect(
147+
getQueryKeyVerbPrefix({
148+
verb: Verbs.HEAD,
149+
useOperationIdAsQueryKey: false,
150+
}),
151+
).toBe('HEAD');
152+
});
153+
154+
it('returns undefined when useOperationIdAsQueryKey is true (operation IDs are already verb+path unique)', () => {
155+
expect(
156+
getQueryKeyVerbPrefix({
157+
verb: Verbs.POST,
158+
useOperationIdAsQueryKey: true,
159+
}),
160+
).toBeUndefined();
161+
});
162+
163+
it('treats undefined useOperationIdAsQueryKey as falsy', () => {
164+
expect(
165+
getQueryKeyVerbPrefix({
166+
verb: Verbs.POST,
167+
useOperationIdAsQueryKey: undefined,
168+
}),
169+
).toBe('POST');
170+
});
171+
});

packages/query/src/query-generator.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,32 @@ export const getMutationInvalidatesConflictWarning = ({
7474
);
7575
};
7676

77+
/**
78+
* Computes a verb prefix segment for query keys when a non-GET operation is
79+
* routed to a Query hook. Without this prefix, two operations sharing a path
80+
* (e.g. `GET /pets` and `POST /pets`) would generate cache keys that both
81+
* begin with `'/pets'`, so TanStack Query would mix their cached data and
82+
* `invalidateQueries({ queryKey: ['/pets'] })` would match both.
83+
*
84+
* Skipped for GET (preserves existing keys) and when
85+
* `useOperationIdAsQueryKey` is enabled (operation IDs are already unique
86+
* across verb + path, so the prefix would be redundant).
87+
*
88+
* Returns the uppercased verb when a prefix should be inserted, or
89+
* `undefined` when no prefix is needed.
90+
*/
91+
export const getQueryKeyVerbPrefix = ({
92+
verb,
93+
useOperationIdAsQueryKey,
94+
}: {
95+
verb: Verbs;
96+
useOperationIdAsQueryKey: boolean | undefined;
97+
}): string | undefined => {
98+
if (useOperationIdAsQueryKey) return undefined;
99+
if (verb === Verbs.GET) return undefined;
100+
return verb.toUpperCase();
101+
};
102+
77103
const getQueryFnArguments = ({
78104
hasQueryParam,
79105
hasSignal,
@@ -887,6 +913,11 @@ export const generateQueryHook = async (
887913
.map((p) => `...(${p.name} ? [${p.name}] : [])`)
888914
.join(', ');
889915

916+
const verbPrefix = getQueryKeyVerbPrefix({
917+
verb,
918+
useOperationIdAsQueryKey: override.query.useOperationIdAsQueryKey,
919+
});
920+
890921
// Note: do not unref() params in Vue - this will make key lose reactivity
891922
queryKeyFns += `
892923
${override.query.shouldExportQueryKey ? 'export ' : ''}const ${queryOption.queryKeyFnName} = (${queryKeyProps}) => {
@@ -896,6 +927,7 @@ ${override.query.shouldExportQueryKey ? 'export ' : ''}const ${queryOption.query
896927
queryOption.type === QueryType.SUSPENSE_INFINITE
897928
? `'infinite'`
898929
: '',
930+
verbPrefix ? `'${verbPrefix}'` : '',
899931
queryKeyIdentifier,
900932
queryKeyParams,
901933
body.implementation,

0 commit comments

Comments
 (0)