fix(query,core): fix mutationInvalidates codegen with path params and tags-split (#3090) - #3198
Merged
melloware merged 3 commits intoApr 6, 2026
Conversation
… tags-split (orval-labs#3090) Bug 1: When mutationInvalidates targets a query with required path parameters but no params mapping, the generated code called the query key function without arguments, causing TS2554 errors and silent runtime mismatches. Now generates predicate-based broad invalidation using the route prefix instead. Bug 2: In tags-split mode, the ile option in mutationInvalidates resolved import paths relative to the output root instead of the consuming file's subdirectory, producing broken imports like './items' instead of '../items/items'. Now correctly resolves relative paths from each tag's subdirectory. Signed-off-by: jaery <jaeryong95@gmail.com>
Contributor
📝 WalkthroughWalkthroughImport paths for tags-split mode are now rewritten to resolve relative paths against the output root and match the split directory layout. Additionally, mutation invalidation becomes OpenAPI-spec–aware, detecting required path parameters and adjusting invalidation strategies accordingly. Test snapshots are generated for new configurations. Changes
Sequence Diagram(s)sequenceDiagram
participant MutGen as Mutation Generator
participant Spec as OpenAPI Spec
participant InvGen as Invalidate Call Generator
participant QK as Query Key Builder
participant QC as QueryClient
MutGen->>MutGen: Check if target.params is empty
alt target.params is empty
MutGen->>Spec: findOperationInfo(spec, target.query)
Spec-->>MutGen: Operation with required path params?
alt Required path params detected
MutGen->>InvGen: Generate predicate-based or partial-key invalidation
InvGen->>InvGen: Use static route prefix for matching
InvGen-->>MutGen: Invalidation strategy (predicate/partial-key)
else No required params
MutGen->>QK: queryKeyFn() with zero args
QK-->>MutGen: Query key
end
else target.params is present
MutGen->>QK: queryKeyFn(...args)
QK-->>MutGen: Query key
end
MutGen->>QC: invalidateQueries with computed strategy
QC-->>MutGen: Invalidation complete
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
tests/configs/react-query.config.ts (1)
18-80: Add a fixture for theshouldSplitQueryKeyinvalidation path.The new generator logic now has a separate branch when
query.shouldSplitQueryKeyis enabled, but the fixtures added here only exercise default-key invalidation and tags-split import rewriting. A small snapshot config with an unmapped required path param undershouldSplitQueryKey: truewould keep that new path from regressing silently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/configs/react-query.config.ts` around lines 18 - 80, Add a new test fixture that exercises the generator branch where query.shouldSplitQueryKey === true: create a config block similar to the existing invalidates entry but set override.query.shouldSplitQueryKey = true and include a mutationInvalidates entry that invalidates the unmapped required-path query 'showPetById' (i.e., list an invalidates entry of 'showPetById' with no params mapping) so the snapshot tests cover predicate-based invalidation for split-key mode; reference the existing override.query.mutationInvalidates array and the 'showPetById' invalidation to place the new case next to the current invalidates test.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/core/src/writers/split-tags-mode.ts`:
- Around line 100-107: The code incorrectly assumes any resolvedPath whose
basename is a known tag is always a tag directory and blindly rewrites it to
dirname/tag/tag+extension, which duplicates explicit file imports like
"./items/items" and diverges from the Angular split-tag naming used later;
update the branch that uses tagNames.has(targetBasename) (the logic producing
targetFile from resolvedPath, targetBasename, tagNames, extension) to compute
the actual split-tag filename using the same filename rule used elsewhere (e.g.,
the Angular branch at lines ~218-221) instead of always joining dirname/tag/tag;
specifically, detect when the import already points to a file (avoid adding an
extra segment) and apply the correct per-platform/generated-name (e.g.,
"<tag>.service" for Angular or "<tag>/<tag>" for default) so targetFile uses the
real split-tag filename.
In `@packages/query/src/mutation-generator.ts`:
- Around line 131-135: getStaticRoutePrefix currently slices up to the first `{`
and returns `'/'` for templates that start with a path param, which causes
overly-broad invalidation; update getStaticRoutePrefix to detect when the
extracted prefix contains no stable literal segment (e.g., only slashes or
empty) and in that case do not return `'/'` but instead fallback to using the
full route template (or return an explicit sentinel like an empty string) so the
caller can perform exact/template-aware matching; apply the same guard/fallback
logic to the similar logic referenced around lines 193-212 so both places avoid
producing a global prefix for routes that start with a path parameter.
- Around line 143-149: The code in needsQueryKeyFnCall treats any truthy
target.params (e.g., [] or {}) as a mapping and skips spec-based fallback,
causing get...QueryKey() to be emitted with no args; change the check to treat
only non-empty mappings as present by replacing "if (target.params) return true"
with a test that verifies target.params is not null/undefined and (if
Array.isArray(target.params) then length>0 else
Object.keys(target.params).length>0). Apply the same non-empty-mapping check in
the other analogous spot (the similar params existence check around the later
block referenced in the comment) so both code paths consistently treat empty
params as “no mapping.”
- Around line 90-105: findOperationInfo currently only matches routes by raw
operationId, which misses generated/renamed operation-name forms like
target.query (used elsewhere with camel(`get-${target.query}-query-key`));
update findOperationInfo to compare both the raw operation.operationId and the
generated operation-name form used by the generator (i.e., the same
transformation used for camel(`get-${target.query}-query-key`) / generated
operation names) so that lookups using target.query succeed; specifically,
inside findOperationInfo (and the lookup logic that calls it) compute the
canonical/generated name for each operation (using the same
camel/get-...-query-key logic) and treat a match if either operation.operationId
=== operationName or generatedName === operationName.
---
Nitpick comments:
In `@tests/configs/react-query.config.ts`:
- Around line 18-80: Add a new test fixture that exercises the generator branch
where query.shouldSplitQueryKey === true: create a config block similar to the
existing invalidates entry but set override.query.shouldSplitQueryKey = true and
include a mutationInvalidates entry that invalidates the unmapped required-path
query 'showPetById' (i.e., list an invalidates entry of 'showPetById' with no
params mapping) so the snapshot tests cover predicate-based invalidation for
split-key mode; reference the existing override.query.mutationInvalidates array
and the 'showPetById' invalidation to place the new case next to the current
invalidates test.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ef2f26b0-c915-4a03-a60a-48e4f62e9d0b
📒 Files selected for processing (32)
packages/core/src/writers/split-tags-mode.tspackages/query/src/mutation-generator.tstests/__snapshots__/angular/multi-content-query-params/endpoints.tstests/__snapshots__/angular/multi-content-query-params/model/error.tstests/__snapshots__/angular/multi-content-query-params/model/index.tstests/__snapshots__/angular/multi-content-query-params/model/item.tstests/__snapshots__/angular/multi-content-query-params/model/items.tstests/__snapshots__/angular/multi-content-query-params/model/listItemsParams.tstests/__snapshots__/react-query/invalidates-tags-split/health/health.tstests/__snapshots__/react-query/invalidates-tags-split/model/cat.tstests/__snapshots__/react-query/invalidates-tags-split/model/catType.tstests/__snapshots__/react-query/invalidates-tags-split/model/createPetsBody.tstests/__snapshots__/react-query/invalidates-tags-split/model/createPetsParams.tstests/__snapshots__/react-query/invalidates-tags-split/model/createPetsSort.tstests/__snapshots__/react-query/invalidates-tags-split/model/dachshund.tstests/__snapshots__/react-query/invalidates-tags-split/model/dachshundBreed.tstests/__snapshots__/react-query/invalidates-tags-split/model/dog.tstests/__snapshots__/react-query/invalidates-tags-split/model/dogType.tstests/__snapshots__/react-query/invalidates-tags-split/model/error.tstests/__snapshots__/react-query/invalidates-tags-split/model/index.tstests/__snapshots__/react-query/invalidates-tags-split/model/labradoodle.tstests/__snapshots__/react-query/invalidates-tags-split/model/labradoodleBreed.tstests/__snapshots__/react-query/invalidates-tags-split/model/listPetsParams.tstests/__snapshots__/react-query/invalidates-tags-split/model/listPetsSort.tstests/__snapshots__/react-query/invalidates-tags-split/model/pet.tstests/__snapshots__/react-query/invalidates-tags-split/model/petCallingCode.tstests/__snapshots__/react-query/invalidates-tags-split/model/petCountry.tstests/__snapshots__/react-query/invalidates-tags-split/model/petWithTag.tstests/__snapshots__/react-query/invalidates-tags-split/model/pets.tstests/__snapshots__/react-query/invalidates-tags-split/pets/pets.tstests/__snapshots__/react-query/invalidates/endpoints.tstests/configs/react-query.config.ts
…d snapshots (orval-labs#3090) - Match operationId by both raw and camelCase forms for renamed operations - Treat empty params ([] / {}) as 'no mapping' to avoid zero-arg calls - Guard route prefix when no stable literal segment exists (e.g. /{tenantId}/...) - Use Angular .service suffix when resolving tag file paths in tags-split mode - Add shouldSplitQueryKey invalidation test fixture - Remove orphaned angular/multi-content-query-params snapshots from unrelated branch Signed-off-by: jaery <jaeryong95@gmail.com>
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (2)
packages/core/src/writers/split-tags-mode.ts (1)
103-115:⚠️ Potential issue | 🟠 MajorHandle explicit file targets before the tag-directory rewrite.
Lines 107-114 still treat any
path.basename(resolvedPath)that matches a tag name as a tag directory. An explicit target like./items/itemstherefore becomes.../items/items/items.ts(or.service.ts) instead of the requested file, and the fallback branch doubles paths that already end with the configured extension. Restrict the special case todirname/<tag>and otherwise preserve the explicit file stem.💡 Suggested change
const resolvedPath = path.resolve(dirname, imp.importPath); const targetBasename = path.basename(resolvedPath); + const isTagDirectoryTarget = + tagNames.has(targetBasename) && + path.dirname(resolvedPath) === dirname; let targetFile: string; - if (tagNames.has(targetBasename)) { + if (isTagDirectoryTarget) { // Target is a known tag directory. Use the real generated // filename which includes the Angular `.service` suffix when // applicable (e.g. dirname/health/health.service.ts). const tagFilename = targetBasename + serviceSuffix + extension; targetFile = path.join(resolvedPath, tagFilename); } else { - targetFile = resolvedPath + extension; + targetFile = resolvedPath.endsWith(extension) + ? resolvedPath + : resolvedPath + extension; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/core/src/writers/split-tags-mode.ts` around lines 103 - 115, The code currently treats any resolvedPath whose basename matches a tag name as a tag-directory and rewrites it, which mangles explicit file targets; update the logic in the block that sets targetFile (using resolvedPath, targetBasename, tagNames, dirname, serviceSuffix, extension) so the special-case rewrite only runs when the resolved path is an immediate child directory of dirname (i.e., path.dirname(resolvedPath) === dirname) and not when the import explicitly targets a file stem; otherwise preserve the explicit stem and append extension only if the resolvedPath does not already end with the configured extension or serviceSuffix+extension.packages/query/src/mutation-generator.ts (1)
136-146:⚠️ Potential issue | 🟠 MajorAvoid the zero-arg fallback for routes that start with a path param.
For a template like
/{tenantId}/pets/{petId},needsQueryKeyFnCall()returns false, butgetStaticRoutePrefix()also returnsundefined, so Lines 239-240 emitget...QueryKey()again. That recreates the original TS2554/runtime bug, and whenfileis set Lines 499-502 may already have dropped the import for that symbol. Keep this branch on a template-aware broad invalidation path instead of falling back to the zero-arg call.Also applies to: 162-169, 212-240, 499-502
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/query/src/mutation-generator.ts` around lines 136 - 146, getStaticRoutePrefix currently returns undefined for prefixes like "/" (routes that start with a path param), which causes callers (e.g., needsQueryKeyFnCall and the code that emits get...QueryKey()) to fall back to a zero-arg call and reintroduce the TS2554/runtime bug; change getStaticRoutePrefix to return the raw prefix (even "/" or other single-segment prefixes) instead of undefined so template-starting routes stay on the template-aware broad invalidation path; update the hasLiteralSegment logic in getStaticRoutePrefix so it does not treat "/" as a guard to return undefined, and verify callers (needsQueryKeyFnCall and the emission sites that previously fell back to zero-arg get...QueryKey()) will now use the returned prefix rather than emitting the zero-arg call.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@packages/core/src/writers/split-tags-mode.ts`:
- Around line 103-115: The code currently treats any resolvedPath whose basename
matches a tag name as a tag-directory and rewrites it, which mangles explicit
file targets; update the logic in the block that sets targetFile (using
resolvedPath, targetBasename, tagNames, dirname, serviceSuffix, extension) so
the special-case rewrite only runs when the resolved path is an immediate child
directory of dirname (i.e., path.dirname(resolvedPath) === dirname) and not when
the import explicitly targets a file stem; otherwise preserve the explicit stem
and append extension only if the resolvedPath does not already end with the
configured extension or serviceSuffix+extension.
In `@packages/query/src/mutation-generator.ts`:
- Around line 136-146: getStaticRoutePrefix currently returns undefined for
prefixes like "/" (routes that start with a path param), which causes callers
(e.g., needsQueryKeyFnCall and the code that emits get...QueryKey()) to fall
back to a zero-arg call and reintroduce the TS2554/runtime bug; change
getStaticRoutePrefix to return the raw prefix (even "/" or other single-segment
prefixes) instead of undefined so template-starting routes stay on the
template-aware broad invalidation path; update the hasLiteralSegment logic in
getStaticRoutePrefix so it does not treat "/" as a guard to return undefined,
and verify callers (needsQueryKeyFnCall and the emission sites that previously
fell back to zero-arg get...QueryKey()) will now use the returned prefix rather
than emitting the zero-arg call.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3d56f354-0ca3-46a7-924b-87bfbc5ec716
📒 Files selected for processing (24)
packages/core/src/writers/split-tags-mode.tspackages/query/src/mutation-generator.tstests/__snapshots__/react-query/invalidates-split-query-key/endpoints.tstests/__snapshots__/react-query/invalidates-split-query-key/model/cat.tstests/__snapshots__/react-query/invalidates-split-query-key/model/catType.tstests/__snapshots__/react-query/invalidates-split-query-key/model/createPetsBody.tstests/__snapshots__/react-query/invalidates-split-query-key/model/createPetsParams.tstests/__snapshots__/react-query/invalidates-split-query-key/model/createPetsSort.tstests/__snapshots__/react-query/invalidates-split-query-key/model/dachshund.tstests/__snapshots__/react-query/invalidates-split-query-key/model/dachshundBreed.tstests/__snapshots__/react-query/invalidates-split-query-key/model/dog.tstests/__snapshots__/react-query/invalidates-split-query-key/model/dogType.tstests/__snapshots__/react-query/invalidates-split-query-key/model/error.tstests/__snapshots__/react-query/invalidates-split-query-key/model/index.tstests/__snapshots__/react-query/invalidates-split-query-key/model/labradoodle.tstests/__snapshots__/react-query/invalidates-split-query-key/model/labradoodleBreed.tstests/__snapshots__/react-query/invalidates-split-query-key/model/listPetsParams.tstests/__snapshots__/react-query/invalidates-split-query-key/model/listPetsSort.tstests/__snapshots__/react-query/invalidates-split-query-key/model/pet.tstests/__snapshots__/react-query/invalidates-split-query-key/model/petCallingCode.tstests/__snapshots__/react-query/invalidates-split-query-key/model/petCountry.tstests/__snapshots__/react-query/invalidates-split-query-key/model/petWithTag.tstests/__snapshots__/react-query/invalidates-split-query-key/model/pets.tstests/configs/react-query.config.ts
✅ Files skipped from review due to trivial changes (21)
- tests/snapshots/react-query/invalidates-split-query-key/model/createPetsBody.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/dachshundBreed.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/catType.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/dogType.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/cat.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/petCallingCode.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/pets.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/petWithTag.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/createPetsParams.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/error.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/dachshund.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/listPetsSort.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/dog.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/listPetsParams.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/labradoodle.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/petCountry.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/createPetsSort.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/pet.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/index.ts
- tests/snapshots/react-query/invalidates-split-query-key/model/labradoodleBreed.ts
- tests/snapshots/react-query/invalidates-split-query-key/endpoints.ts
melloware
approved these changes
Apr 6, 2026
This was referenced Apr 9, 2026
This was referenced May 3, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #3090
mutationInvalidatestargets a query with required path parameters but noparamsmapping, the generated code called the query key function without arguments (e.g.getShowPetByIdQueryKey()), causingTS2554errors and silent runtime mismatches. Now generates predicate-based broad invalidation using the route prefix instead.tags-splitmode, thefileoption inmutationInvalidatesresolved import paths relative to the output root instead of the consuming file's subdirectory, producing broken imports like'./items'instead of'../items/items'. Now correctly resolves relative paths from each tag's subdirectory.Changed files
packages/query/src/mutation-generator.tspackages/core/src/writers/split-tags-mode.tsfileoption import paths relative to tag subdirectory in tags-split modetests/configs/react-query.config.tstests/__snapshots__/react-query/invalidates/endpoints.tstests/__snapshots__/react-query/invalidates-tags-split/**Test plan
bun run format:check— passbun run build— 12/12 passbun run typecheck— 12/12 passbun run lint— 24/24 passbun run test— pass (existingresolve-version.test.tsfailure is pre-existing on master)bun run test:snapshots— 67/67 tasks, 3509/3509 tests passinvalidates/endpoints.tsusespredicate+startsWithfor Bug 1invalidates-tags-split/pets/pets.tsimports'../health/health'for Bug 2Summary by CodeRabbit
New Features
Tests