fix(mock): emit bare factory call for primitive-union $ref in additionalProperties (#3200) - #3504
Merged
melloware merged 3 commits intoJun 1, 2026
Conversation
…nalProperties (orval-labs#3200) When `schemas: true` emits per-schema faker factories, an `additionalProperties` dictionary whose value is a $ref to a primitive `oneOf`/`anyOf` (e.g. `number | string`) delegated to `get<X>Mock()` but wrapped the call in `{ ...get<X>Mock() }`. The factory returns a primitive union, which is not spreadable: the output failed to compile (TS2698) and would discard the value as `{}` at runtime. The delegation now treats a `oneOf`/`anyOf` as object-like only when every branch resolves to an object, so primitive unions emit the bare `get<X>Mock()` call while object compositions keep the spread form. Closes orval-labs#3200
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughConservatively detect when a referenced schema actually resolves to an object shape before emitting ChangesDictionary Mock Generation Fix
Sequence Diagram(s)sequenceDiagram
participant Client
participant resolveMockValue
participant resolvesToObjectLike
participant getRefInfo
Client->>resolveMockValue: request mock generation for additionalProperties $ref
resolveMockValue->>resolvesToObjectLike: is schema object-like?
resolvesToObjectLike->>getRefInfo: resolve $ref -> schema
resolvesToObjectLike->>resolvesToObjectLike: recursively validate oneOf/anyOf branches
resolvesToObjectLike-->>resolveMockValue: boolean result
resolveMockValue-->>Client: emit bare factory call or object spread accordingly
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a regression test and generator logic to fix invalid spreading of primitive-union faker mock factories when used as additionalProperties dictionary values under schemas: true (Issue #3200).
Changes:
- Added an OpenAPI reproduction spec and test config entry for Issue 3200.
- Added a regression test asserting dictionary values use a bare
get<X>Mock()call (no object spread). - Updated faker mock value resolver to only use spread delegation when the referenced schema resolves to an object-like shape; added corresponding snapshots.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/specifications/issue-3200.yaml | New OpenAPI spec reproducing the primitive-union $ref dictionary case. |
| tests/configs/mock.config.ts | Adds generation config for Issue 3200 outputs (schemas + operation response mocks). |
| tests/api-generation.spec.ts | Adds regression assertion to prevent { ...get<X>Mock() } for primitive unions. |
| tests/snapshots/mock/issue-3200/model/stringToNumberMap.ts | New snapshot for generated dictionary interface. |
| tests/snapshots/mock/issue-3200/model/stringToIntegerMap.ts | New snapshot for generated dictionary interface. |
| tests/snapshots/mock/issue-3200/model/numberLike.ts | New snapshot for generated primitive union type. |
| tests/snapshots/mock/issue-3200/model/integerLike.ts | New snapshot for generated primitive union type. |
| tests/snapshots/mock/issue-3200/model/index.ts | New snapshot barrel export for Issue 3200 models. |
| tests/snapshots/mock/issue-3200/model/index.faker.ts | New snapshot for faker factories ensuring bare call delegation. |
| tests/snapshots/mock/issue-3200/endpoints.ts | New snapshot for generated endpoints and response mocks. |
| packages/mock/src/faker/resolvers/value.ts | Fixes delegation logic by checking whether compositions resolve to object before spreading; adds helper. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+464
to
+468
| function compositionResolvesToObject( | ||
| schema: MockSchema, | ||
| context: ContextSpec, | ||
| seen = new Set<string>(), | ||
| ): boolean { |
Comment on lines
+469
to
+490
| let resolved: Partial<OpenApiSchemaObject> | undefined = | ||
| schema as Partial<OpenApiSchemaObject>; | ||
|
|
||
| if (isReference(schema)) { | ||
| const refPath = typeof schema.$ref === 'string' ? schema.$ref : ''; | ||
| if (seen.has(refPath)) { | ||
| return false; | ||
| } | ||
| seen.add(refPath); | ||
| const { refPaths } = getRefInfo(refPath, context); | ||
| resolved = Array.isArray(refPaths) | ||
| ? (prop( | ||
| context.spec, | ||
| // @ts-expect-error: refPaths are not guaranteed to be valid keys of the spec | ||
| ...refPaths, | ||
| ) as Partial<OpenApiSchemaObject>) | ||
| : undefined; | ||
| } | ||
|
|
||
| if (!resolved) { | ||
| return false; | ||
| } |
Comment on lines
+472
to
+477
| if (isReference(schema)) { | ||
| const refPath = typeof schema.$ref === 'string' ? schema.$ref : ''; | ||
| if (seen.has(refPath)) { | ||
| return false; | ||
| } | ||
| seen.add(refPath); |
Comment on lines
+813
to
+821
| expect(content).toContain( | ||
| '[faker.string.alphanumeric(5)]: getIntegerLikeMock(),', | ||
| ); | ||
| expect(content).toContain( | ||
| '[faker.string.alphanumeric(5)]: getNumberLikeMock(),', | ||
| ); | ||
| // The primitive-union factory call must never be spread into the object. | ||
| expect(content).not.toContain('{ ...getIntegerLikeMock() }'); | ||
| expect(content).not.toContain('{ ...getNumberLikeMock() }'); |
![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
🤖 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/mock/src/faker/resolvers/value.ts`:
- Around line 467-477: The bug is that the shared Set parameter seen is being
mutated and reused across sibling branches, causing false cycles; fix by
treating seen as a recursion stack: do not mutate the original Set for sibling
branches—when you add a reference (refPath) or recurse into a oneOf/anyOf
branch, create a new Set (e.g., clone seen and add refPath) or push/pop so each
recursive path gets its own stack; update all usages around isReference handling
and the oneOf/anyOf branch recursion sites so each branch receives its own
copied/isolated seen instead of sharing the same Set.
🪄 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: 4f90c64b-da66-4828-b104-95e93724e3f6
📒 Files selected for processing (11)
packages/mock/src/faker/resolvers/value.tstests/__snapshots__/mock/issue-3200/endpoints.tstests/__snapshots__/mock/issue-3200/model/index.faker.tstests/__snapshots__/mock/issue-3200/model/index.tstests/__snapshots__/mock/issue-3200/model/integerLike.tstests/__snapshots__/mock/issue-3200/model/numberLike.tstests/__snapshots__/mock/issue-3200/model/stringToIntegerMap.tstests/__snapshots__/mock/issue-3200/model/stringToNumberMap.tstests/api-generation.spec.tstests/configs/mock.config.tstests/specifications/issue-3200.yaml
…ck (orval-labs#3200) Addresses review feedback on the additionalProperties dictionary fix: - The cycle-guard `Set` was shared and mutated across all `oneOf`/`anyOf` branches, so the first branch could poison its siblings: a composition like `oneOf: [{$ref: Foo}, {$ref: Foo}]` made the second branch look cyclic and return `false`, misclassifying an object-only union as non-object-like. The guard now takes a fresh copy at each `$ref` hop, so siblings sharing a `$ref` no longer trip it. - Rename `compositionResolvesToObject` -> `resolvesToObjectLike`; it also recognizes plain object schemas (`properties`/`additionalProperties`/`allOf`), not just compositions. - Return early when a `$ref` is not a string instead of using an `''` fallback key, and initialize `resolved` via an explicit if/else for clarity. - Make the regression assertions whitespace-tolerant and detect a spread regardless of brace formatting.
melloware
approved these changes
Jun 1, 2026
3 tasks
3 tasks
This was referenced Jun 29, 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
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.
What
Fixes invalid TypeScript generated for an
additionalPropertiesdictionary whose value is a$refto a primitiveoneOf/anyOf(e.g.IntegerLike = number | string) when per-schema faker factories are enabled (schemas: true).Given:
orval emitted:
getIntegerLikeMock()returns a primitive union (number | string), which is not spreadable, so the output fails to compile withTS2698: Spread types may only be created from object typesand would discard the value as{}at runtime. After the fix the dictionary value is the bare call:Why
When delegating a
$refto itsget<X>Mock()factory, the code wrapped the call in{ ...get<X>Mock() }whenever the schema hadoneOf/anyOf— without checking what the branches actually resolve to. A composition of primitives is not object-like, so it must not be spread.The delegation now treats a
oneOf/anyOfas object-like only when every branch resolves to an object (compositionResolvesToObject). Object compositions (e.g.Dog | Cat) keep the spread form; primitive unions emit the bare call. Thetype === 'object'andallOfarms are unchanged, so existing fixtures (e.g. petstore'sPet/Dog) are byte-for-byte identical — no snapshot changes.Note on the original report
The issue was filed on 8.6.2 describing an
IntegerLike | undefinedwidening. That exact symptom no longer reproduces onmaster(faker v10'sarrayElement<const T>(): Tno longer returnsundefined, and thePartial<>/overrideResponsespread is no longer emitted for dictionary factories). The remaining type error is theTS2698above, which surfaces through theschemas: truefaker factory path added later in #3426. It hits the same schema shape, so this PR addresses it under #3200.Test plan
tests/specifications/issue-3200.yaml(covers bothoneOfandanyOfprimitive-union dictionary values) andissue3200config inmock.config.ts(faker,schemas: true,operationResponses: true).api-generation.spec.tsasserting the bareget<X>Mock()call and the absence of{ ...get<X>Mock() }.scripts/typecheck-generated.mjs(failed with TS2698 before, passes after).bun run test,bun run test:snapshots,bun run typecheck,bun run lint,bun run format:checkall pass; existing snapshots unchanged.Closes #3200
Summary by CodeRabbit
Bug Fixes
Tests