fix(axios): eliminate module-level returnTypesToWrite map - #3696
Merged
melloware merged 1 commit intoJul 12, 2026
Merged
Conversation
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (7)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughWalkthroughThis PR removes axios' shared ChangesPer-operation return type propagation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant target as target.ts / target-tags.ts
participant client as generateOperations
participant axios as generateAxiosImplementation
participant footer as generateAxiosFooter
target->>client: build operations and footer inputs
client->>axios: generate implementation + returnType
axios-->>client: { implementation, returnType }
client-->>target: operations with types.result
target->>footer: footer({ operations, operationNames, ... })
footer->>footer: emit per-operation Result types
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/orval/src/client.ts (1)
179-202: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winKeep
operationsin the footer fallback path.When an
OutputClientFuncreturns an object-style footer, the deprecated array call can fall back here, but Lines 197-202 dropoperations. Axios emits*Resultaliases only fromoperations, so custom client-function wrappers can lose those aliases.Proposed fix
} catch { implementation = footer({ operationNames, + operations, title: titles.implementation, hasMutator, hasAwaitedType, }); }Based on learnings, footer generation must read result types from per-operation data to avoid cross-tag/name collisions.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/orval/src/client.ts` around lines 179 - 202, The footer fallback in client.ts is dropping operations when `OutputClientFunc` falls back from the deprecated array call, which can cause missing `*Result` aliases. Update the footer generation path in the try/catch around implementation so the object-style footer call in the catch includes operations, matching the main footer invocation and preserving per-operation result type data. Use the implementation/footer logic in client.ts as the place to keep the same operations payload in both paths.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/orval/src/generate-spec.test.ts`:
- Around line 1265-1316: The regression test currently uses different operation
names, so it does not exercise the shared operationName collision in
returnTypesToWrite. Update generateSpec - returnTypesToWrite isolation across
tags to force both tag groups to use the same operationName via
override.operations (for example in the SPEC setup used by generateSpec), then
assert each generated tag footer still emits the correct *Result type for its
own response schema. Use the existing generateSpec, normalizeOptions, and
operationName-related setup in this test file so the coverage matches the
module-level map collision bug.
---
Outside diff comments:
In `@packages/orval/src/client.ts`:
- Around line 179-202: The footer fallback in client.ts is dropping operations
when `OutputClientFunc` falls back from the deprecated array call, which can
cause missing `*Result` aliases. Update the footer generation path in the
try/catch around implementation so the object-style footer call in the catch
includes operations, matching the main footer invocation and preserving
per-operation result type data. Use the implementation/footer logic in client.ts
as the place to keep the same operations payload in both paths.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 54964359-4de3-458e-9cfc-72c01b3bb203
📒 Files selected for processing (6)
packages/axios/src/index.tspackages/core/src/types.tspackages/core/src/writers/target-tags.tspackages/core/src/writers/target.tspackages/orval/src/client.tspackages/orval/src/generate-spec.test.ts
@orval/angular
@orval/axios
@orval/core
@orval/effect
@orval/fetch
@orval/hono
@orval/mcp
@orval/mock
orval
@orval/query
@orval/solid-start
@orval/swr
@orval/zod
commit: |
aqeelat
force-pushed
the
fix/3685-axios-returntypes-module-level-map
branch
from
July 9, 2026 20:25
218d317 to
92ed48b
Compare
…#3685) The axios generator used a module-level Map (returnTypesToWrite) to collect *Result type declarations during implementation generation, then read them during footer generation. In tags-split mode, all implementations are generated first (populating the map), then footers are generated per-tag. When two operations from different tags shared the same operationName (via override), the second overwrote the first's entry, causing the wrong *Result type to be emitted in the first tag's footer. Fix: eliminate the module-level map entirely. The return type generator is now returned from generateAxiosImplementation as part of GeneratorClient.returnType, stored on GeneratorOperation.types.result, and passed to the footer via the new optional operations param on ClientFooterBuilder/GeneratorClientFooter. The footer reads from operations[].types.result instead of the side-effect map, ensuring each tag's footer only emits types for its own operations.
aqeelat
force-pushed
the
fix/3685-axios-returntypes-module-level-map
branch
from
July 9, 2026 21:01
92ed48b to
f1f65ec
Compare
melloware
approved these changes
Jul 12, 2026
Merged
2 tasks
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
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.
Problem
The axios generator used a module-level
Map(returnTypesToWrite) to collect*Resulttype declarations during implementation generation, then read them during footer generation. Intags-splitmode, all implementations are generated first (populating the map), then footers are generated per-tag. When two operations from different tags shared the sameoperationName(viaoverride.operationName), the second overwrote the first's entry, causing the wrong*Resulttype to be emitted in the first tag's footer.Closes #3685.
Solution
Eliminate the module-level map entirely. The return type generator is now:
generateAxiosImplementationasGeneratorClient.returnTypeGeneratorOperation.types.resultingenerateOperationsoperationsparam onClientFooterBuilder/GeneratorClientFooteroperations[].types.resultingenerateAxiosFooterThis removes all module-level state and ordering concerns. Each tag's footer only emits types for its own operations.
Changes
GeneratorClientgains optionalreturnType?: (title?) => stringClientFooterBuilderandGeneratorClientFootergain optionaloperations?: GeneratorOperation[]generateAxiosImplementationreturns{ implementation, returnType }instead of side-effecting on a mapgenerateAxiosFooterreads fromoperations[].types.resultinstead of the module-level mapreturnTypesToWriteMap deletedtarget.tsandtarget-tags.tspass filteredoperationsto the footergenerateClientFooterinclient.tspassesoperationsthrough to per-client footersTests
*ResulttypeSummary by CodeRabbit