feat(core): add tags-operations and tags-operations-split output modes - #3754
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds ChangesTag operations generation
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant writeSpecs
participant TagsOperationsWriter
participant TargetGenerator
participant GeneratedFiles
writeSpecs->>TagsOperationsWriter: dispatch output mode
TagsOperationsWriter->>TargetGenerator: generate tag and operation targets
TargetGenerator-->>TagsOperationsWriter: helpers, schemas, mocks, and operation targets
TagsOperationsWriter->>GeneratedFiles: write operation files and barrels
Possibly related PRs
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/core/src/writers/tags-operations-mode.ts`:
- Around line 397-425: The mock index is written once per tag to a shared
tag-independent path, so concurrent writes overwrite each other. In
packages/core/src/writers/tags-operations-mode.ts lines 397-425, hoist
mockIndexEntries outside the tagEntries.map/Promise.all callback, accumulate all
tagged operations, and emit each mock index after Promise.all resolves; apply
the same change in packages/core/src/writers/tags-operations-split-mode.ts lines
424-452, preserving the existing sorted exports and path generation.
🪄 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: 5abc145f-39ac-4ed2-a59b-9bf43ba36763
📒 Files selected for processing (9)
packages/core/src/types.tspackages/core/src/writers/index.tspackages/core/src/writers/tags-operations-mode.test.tspackages/core/src/writers/tags-operations-mode.tspackages/core/src/writers/tags-operations-split-mode.test.tspackages/core/src/writers/tags-operations-split-mode.tspackages/core/src/writers/target-tags-operations.tspackages/mock/src/faker/getters/array-item-factory.tspackages/orval/src/write-specs.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: |
|
This feature seems great but the PR checks are failing? |
Adds two new output modes that emit one file per operation nested under a per-tag directory, plus a per-tag barrel so existing tag-level imports keep working: - tags-operations: one file per operation with types + runtime combined - tags-operations-split: two files per operation (schemas + runtime) Shared per-tag boilerplate (query-key helpers, AwaitedInput, HTTP status types, etc.) is written once per tag to a helper file and imported by name rather than duplicated into every operation file. Scoped to standalone-function clients (react-query, svelte-query, vue-query, swr, fetch) since axios, zod, Angular, and Hono group operations into a shared factory/class/handler structure that per- operation files can't represent without changing their output shape.
- Use kebab-case for operation filenames (consistent with mock filenames) - Pass oneMore: true to all generateMutatorImports calls so relative mutator paths resolve correctly from the tag subdirectory - Filter helper imports per-operation to only names used in that operation's implementation, eliminating TS6133 unused-import errors - Write a root barrel (dirname/index.ts) re-exporting per-tag barrels, and restrict workspace index re-exports to that root barrel + global schemas to eliminate TS2308 ambiguous-re-export errors
The aggregate mock index lives at a tag-independent `<mockDir>/index.<ext>` path whose entries reference `./<tag>/<op>`, but `mockIndexEntries` was scoped inside the per-tag `Promise.all` callback and written per tag. Concurrent tags therefore overwrote the shared file, leaving only the last-completing tag's operations when `indexMockFiles` was enabled with multiple tagged mocks. Hoist `mockIndexEntries` to function scope (carrying each operation's tag), accumulate across all tags, and write each mock index once after `Promise.all` resolves — sorted by tag then operation for idempotent output. Mirrors the existing pattern in `split-tags-mode.ts`. Applies to both `tags-operations` and `tags-operations-split`. Also updates the writer tests to the kebab-case operation filenames and restores the trailing newline on package.json (fixes the failing `vp fmt --check` CI step).
8a73866 to
7bc7dfd
Compare
|
@melloware I think this is ready to go. I understand its a bit more code than the usual PR but considering its for 2 entirely new writers, I think it's reasonable. I will be doing some further audits to make sure nothing is exorbitant, but this seems ready to go. We've tested it on our spec file at Turo and were able to fix a few bugs with the agents initial round of code changes, primarily around pathing and import cleanups in resulting files. |
|
Excellent i am waiting for the build to pass |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
packages/core/src/writers/tags-operations-mode.test.ts (1)
92-102: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover divergent OpenAPI and TypeScript schema names.
This only tests
name: 'Error', so it cannot catch a resolver incorrectly matchingschemaName. Use, for example, schemaErrorDtowith import{ name: 'ErrorDto', schemaName: 'error' }, then retain the shared-schema import assertion.Based on learnings, schema identity must be matched on
GeneratorImport.name, not the OpenAPIschemaName.🤖 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/core/src/writers/tags-operations-mode.test.ts` around lines 92 - 102, Update the test fixture around createSplitModeOperation to use a divergent schema identity: define the schema as ErrorDto and import it with name ErrorDto plus schemaName error. Preserve the shared-schema import assertion so the test verifies resolution matches GeneratorImport.name rather than the OpenAPI schemaName.Source: Learnings
packages/core/src/writers/tags-operations-split-mode.ts (1)
309-405: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftBoth writers carry a byte-identical copy of the mock-emission and path-resolution logic. The split-mode writer already imports
assertClientSupportsTagsOperationsfrom the non-split one, but the mock loop,schemasPathRelative/schemasTargetderivation, mock-index accumulation and emission, root barrel, and per-tag error wrapping are duplicated verbatim — so every future fix (including the mock-index race already fixed once in both) has to be applied twice.
packages/core/src/writers/tags-operations-split-mode.ts#L309-L405: extract the per-operation mock emission + index accumulation into a shared helper (e.g.writeOperationMocks({ operation, tag, output, builder, ... })in a newtags-operations-shared.ts) and call it here.packages/core/src/writers/tags-operations-mode.ts#L295-L391: replace this copy with the same shared helper, and move the sharedschemasPathRelative/schemasTarget, mock-index emission, and root-barrel blocks alongside it.🤖 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/core/src/writers/tags-operations-split-mode.ts` around lines 309 - 405, The mock generation and path/index emission logic is duplicated between packages/core/src/writers/tags-operations-split-mode.ts:309-405 and packages/core/src/writers/tags-operations-mode.ts:295-391. Extract the per-operation mock emission and index accumulation into a shared helper such as writeOperationMocks in a new tags-operations-shared.ts, then call it from both writers; move the shared schemasPathRelative/schemasTarget derivation, mock-index emission, root-barrel handling, and per-tag error wrapping into the shared flow as requested, preserving existing behavior and eliminating both copies.packages/core/src/writers/tags-operations-mode.ts (1)
183-191: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueEscape import names before building the matcher.
imp.name/imp.aliasare interpolated raw into the pattern. Generated identifiers are normally safe, but any name containing a regex metacharacter (e.g. a leading$, which JS treats as an anchor mid-pattern) silently drops a required import instead of throwing. A small escape keeps this robust and also quiets the static-analysis warning.♻️ Suggested hardening
const implementationImports = operation.imports.filter((imp) => { const searchWords = [imp.alias, imp.name] .filter((part): part is string => Boolean(part?.length)) + .map((part) => part.replaceAll(/[$()*+.?[\\\]^{|}]/g, String.raw`\$&`)) .join('|'); if (!searchWords) return false; - return new RegExp(String.raw`\b(${searchWords})\b`, 'g').test( + return new RegExp(String.raw`\b(${searchWords})\b`).test( operation.implementation, ); });🤖 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/core/src/writers/tags-operations-mode.ts` around lines 183 - 191, Update the import-name matcher in the implementationImports filter to escape regex metacharacters in each imp.alias and imp.name before joining them into the pattern. Preserve the existing word-boundary matching and filtering behavior while ensuring names such as those beginning with $ are matched literally.Source: Linters/SAST tools
🤖 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/core/src/writers/tags-operations-mode.ts`:
- Around line 440-457: Update the MSW branch in the index-generation map to
export the handler name from the corresponding mock output, using
mockOutput.implementation.handlerName or its established derived value instead
of constructing it with pascal(opName). Preserve the existing filename-based
path generation and non-MSW export behavior, while ensuring kebab-cased
operations such as getV2Pet and get2Pet retain their actual MSW export names.
In `@packages/core/src/writers/tags-operations-split-mode.test.ts`:
- Around line 206-209: Update the filename assertion in the split-mode writer
test around the existing paths check to use the writer’s kebab-cased operation
filename, get-health.schemas.ts, instead of getHealth.schemas.ts. Keep the
assertion verifying that this generated schema file does not exist, while
leaving the paths.some check unchanged.
In `@packages/core/src/writers/target-tags-operations.ts`:
- Around line 64-93: Update extractDeclaredNames and its declaration regexes to
also collect and force exports for top-level interface, enum, class, let, and
var declarations, while preserving existing export handling. Keep interface
names in typeNames, and include enum/class/let/var names in valueNames so
generated operation imports retain runtime bindings.
In `@packages/orval/src/write-specs.ts`:
- Around line 441-465: Update getImplementationPathsForIndex so the
tags-operations ambiguity filtering applies only to the workspace barrel import
specifiers or computed imports list, not the returned implementationPaths.
Preserve all per-tag/operation, helper, operation-schema, and mock paths for
afterAllFilesWrite and runFormatter while retaining only root-barrel and
global-schema entries where resolving barrel imports requires it.
---
Nitpick comments:
In `@packages/core/src/writers/tags-operations-mode.test.ts`:
- Around line 92-102: Update the test fixture around createSplitModeOperation to
use a divergent schema identity: define the schema as ErrorDto and import it
with name ErrorDto plus schemaName error. Preserve the shared-schema import
assertion so the test verifies resolution matches GeneratorImport.name rather
than the OpenAPI schemaName.
In `@packages/core/src/writers/tags-operations-mode.ts`:
- Around line 183-191: Update the import-name matcher in the
implementationImports filter to escape regex metacharacters in each imp.alias
and imp.name before joining them into the pattern. Preserve the existing
word-boundary matching and filtering behavior while ensuring names such as those
beginning with $ are matched literally.
In `@packages/core/src/writers/tags-operations-split-mode.ts`:
- Around line 309-405: The mock generation and path/index emission logic is
duplicated between
packages/core/src/writers/tags-operations-split-mode.ts:309-405 and
packages/core/src/writers/tags-operations-mode.ts:295-391. Extract the
per-operation mock emission and index accumulation into a shared helper such as
writeOperationMocks in a new tags-operations-shared.ts, then call it from both
writers; move the shared schemasPathRelative/schemasTarget derivation,
mock-index emission, root-barrel handling, and per-tag error wrapping into the
shared flow as requested, preserving existing behavior and eliminating both
copies.
🪄 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 Plus
Run ID: a504d89d-e490-4036-bfd8-53c40058408a
📒 Files selected for processing (9)
packages/core/src/types.tspackages/core/src/writers/index.tspackages/core/src/writers/tags-operations-mode.test.tspackages/core/src/writers/tags-operations-mode.tspackages/core/src/writers/tags-operations-split-mode.test.tspackages/core/src/writers/tags-operations-split-mode.tspackages/core/src/writers/target-tags-operations.tspackages/mock/src/faker/getters/array-item-factory.tspackages/orval/src/write-specs.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/src/writers/index.ts
- packages/core/src/types.ts
- packages/mock/src/faker/getters/array-item-factory.ts
|
@jakiestfu can tyou review the latest Code Rabbit comments? |
Three fixes from CodeRabbit review:
- The aggregate mock index re-derived MSW export names via
`get${pascal(opName)}Mock`, but the actual exported symbol is
`mockOutput.implementation.handlerName` (e.g.
`getListPetsMockHandler`), so every generated mock-index import was
broken (wrong name entirely, not just an edge case). Carry
`handlerName` through the accumulated mock-index entry instead of
re-deriving it. Applies to both `tags-operations` and
`tags-operations-split`.
- `write-specs.ts`: `getImplementationPathsForIndex` narrows the
workspace-barrel path list to just the root barrel + global schemas
for tags-operations modes (to avoid ambiguous re-exports), but that
narrowed list was also being assigned back to `implementationPaths`,
which flows into `afterAllFilesWrite` and the formatter. That
silently dropped every per-tag/operation/helper/schema/mock file from
formatting and lifecycle hooks whenever `output.workspace` was set.
Scope the narrowing to the barrel's import-specifier computation only;
keep the full path list for downstream hooks/formatting.
- Fixed a dead test assertion asserting a camelCase path
(`getHealth.schemas.ts`) the writer can never emit (it writes
`get-health.schemas.ts`), left over from the kebab-case filename
change.
Tightened the mock-index regression test to assert the exact `export {
handlerName }` line rather than just the file path, so a re-introduced
name mismatch would be caught.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/core/src/writers/tags-operations-mode.test.ts`:
- Around line 410-415: Update mockIndexEntries to re-export each operation’s
mockHandler under a unique operation-derived alias, avoiding duplicate names in
the aggregate mock barrel. Preserve the existing source paths and revise the
assertions in the tags-operations-mode test to expect the aliased exports.
🪄 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 Plus
Run ID: dea24386-40d0-4694-8301-1efe5ffd2e63
📒 Files selected for processing (5)
packages/core/src/writers/tags-operations-mode.test.tspackages/core/src/writers/tags-operations-mode.tspackages/core/src/writers/tags-operations-split-mode.test.tspackages/core/src/writers/tags-operations-split-mode.tspackages/orval/src/write-specs.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/core/src/writers/tags-operations-mode.ts
- packages/core/src/writers/tags-operations-split-mode.test.ts
- packages/core/src/writers/tags-operations-split-mode.ts
The aggregate mock index re-exported each MSW mock's real
`handlerName` verbatim. `handlerName` derives from `operationName`,
which a user-supplied `override.operationName` function can map to
the same value for multiple operations — producing two identically
named exports in the same barrel file.
Alias each re-export by `${pascal(tag)}${pascal(opName)}MockHandler`,
derived from the entry's `{tag, opName}` dedup key, which is always
unique. Applies to both `tags-operations` and `tags-operations-split`.
Also includes an unrelated snapshot update (trailing blank line in
tests/__snapshots__/axios/issue-3675-index-target/index.schemas.ts)
picked up by `test:snapshots:update`; unconnected to tags-operations.
Summary
Adds two new output modes that emit one file per operation nested under a per-tag directory, for use cases that want finer-grained generated files than
tags/tags-split(one file per tag) provide:tags-operations— one file per operation containing both types and runtime hook/client code, e.g.<build>/pets/get-pet.ts.tags-operations-split— two files per operation:<build>/pets/get-pet.ts(runtime) +<build>/pets/get-pet.schemas.ts(the component schemas that operation references, resolved transitively).Both modes also write:
<build>/pets/index.ts) that re-exports every operation's runtime file, so existingimport from '<build>/pets'-style imports keep working.<build>/pets/pets.helpers.ts) for boilerplate that today gets inlined once per tag (query-key helpers,AwaitedInput/SecondParametertype aliases, HTTP status code unions, etc.) — imported by name into each operation file instead of duplicated into every one.Client scope
These modes are scoped to clients that already compile each operation to a fully standalone function/hook:
react-query,svelte-query,vue-query,swr, andfetch. Axios (and zod) wrap every operation in a single per-tag factory function (export const getPets = (axiosInstance) => { ...; return { getPet, listPets } }), and Angular/Hono group operations into a class/service/handler — none of these have an existing per-operation code path, so attempting to support them would mean changing their generated output shape. Configuring an unsupported client with either new mode throws a clear error pointing attags/tags-splitinstead.Test plan
packages/core/src/writers/tags-operations-mode.test.ts,tags-operations-split-mode.test.ts(17 tests) covering per-operation file layout (dash-case filenames), the per-tag barrel, the shared helper file (both type-only and runtime-value imports, filtered to names each operation actually uses), transitive schema resolution, the global schemas file, per-operation mock files, the aggregate mock index across multiple tags, and the client guard.packages/core/src/writers/**) passes with no regressions — 185 tests.@orval/core,@orval/mock, andorvalpackage test suites pass (aside from 3 pre-existing, environment-dependent failures inresolve-version.test.tsunrelated to this change, reproducible onmaster).@tanstack/react-query.Summary by CodeRabbit
tags-operationsandtags-operations-splitoutput modes.