fix(core): extract named enums in nullable object compositions - #3564
Conversation
The explicit OAS 3.1 nullability form `anyOf|oneOf: [{ object }, { type:
null }]` routed through combineSchemas, which resolves the object member
with `combined: true` and an undefined propName. That dropped the schema
name, so nested enum properties were inlined as string-literal unions
instead of being extracted into named `as const` consts.
Divert the single inline object member to the property-iteration path
with its `name` preserved (and a synthesized ` | null`), mirroring the
orval-labs#3340 fix one level up. The guard is narrow: allOf, `$ref` object
members, real multi-member unions, primitive members, and empty objects
keep the existing combineSchemas behavior.
Closes orval-labs#3563
|
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)
📝 WalkthroughWalkthroughThis PR fixes issue ChangesNullable object composition and enum extraction
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related issues
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Fixes a regression where nullable objects expressed as OAS 3.1 anyOf/oneOf compositions with a { type: 'null' } member lose their name, causing nested enums to be inlined instead of emitted as named as const definitions.
Changes:
- Adds a guard in
getObject()to divert nullable{ object } | nullcompositions away fromcombineSchemas()to preserve naming for nested enum extraction. - Adds a regression test covering both
anyOfandoneOfnullable-object composition forms.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/core/src/getters/object.ts | Detects { object-with-properties } | null composition and routes through the object property-iteration path to preserve name. |
| packages/core/src/generators/schema-definition.test.ts | Adds regression coverage to ensure nested enums become named as const schemas for anyOf/oneOf nullable-object compositions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @@ -211,6 +211,73 @@ export function getObject({ | |||
| if (itemAllOf || itemOneOf || itemAnyOf) { | |||
| const separator = itemAllOf ? 'allOf' : itemOneOf ? 'oneOf' : 'anyOf'; | |||
There was a problem hiding this comment.
Fixed in fe46cb2: members is now derived from separator (anyOf/oneOf only, undefined otherwise) so both paths inspect the same combiner and allOf never diverts.
| // `allOf` is intersection, not a nullable union, so it is excluded; real | ||
| // unions, `$ref` object members, primitive members, and empty objects keep | ||
| // the combineSchemas behavior via the guard below. | ||
| const members = itemAnyOf ?? itemOneOf; |
| const objectMembers = members.filter((member) => !isNullMember(member)); | ||
| const objectMember = objectMembers[0]; |
There was a problem hiding this comment.
Renamed to nonNullMembers/nonNullMember in fe46cb2 — agreed, the array holds all non-null members (incl. non-object and $ref schemas), and the object/$ref/properties checks narrow it afterwards.
|
|
||
| const isNullableObjectComposition = | ||
| members.some(isNullMember) && | ||
| objectMembers.length === 1 && |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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/getters/object.ts`:
- Around line 225-226: The nullable-object shortcut is being applied by
selecting members via "const members = itemAnyOf ?? itemOneOf" without checking
the active combiner, which can run when separator === "allOf" (or pick the wrong
branch) and bypass combineSchemas; update the logic in the getter where members
is computed (the block using itemAnyOf, itemOneOf and separator around lines 225
and again at 257-279) to respect the active combiner: only use itemAnyOf when
separator === "anyOf" and only use itemOneOf when separator === "oneOf" (fall
back to null/undefined otherwise), and ensure the path then calls combineSchemas
when separator is a sibling-combiner (e.g., "allOf") so sibling constraints are
preserved.
🪄 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: d2fcb0ad-187d-4253-bc03-fede01d8cac6
📒 Files selected for processing (2)
packages/core/src/generators/schema-definition.test.tspackages/core/src/getters/object.ts
Select the composition `members` from the active `separator` instead of `itemAnyOf ?? itemOneOf`, so the nullable-object shortcut and the combineSchemas fallback always operate on the same combiner. This stops the shortcut from firing on an `allOf`-primary schema (which would drop sibling-combiner constraints) and from inspecting `anyOf` members when `oneOf` is the active separator. Also rename `objectMembers`/`objectMember` to `nonNullMembers`/ `nonNullMember`: the array holds every non-null member (including non- object and `$ref` schemas), not only object members.
What
Follow-up to #3340 (PR #3560). Enum properties inside a nested object are now extracted into named
as constconsts when the object's nullability is spelled as the explicit OAS 3.1 compositionanyOf: [{ object }, { type: "null" }](and the analogousoneOf), matching how top-level, non-nullable nested, andtype: ["object", "null"]objects already behave.Before
After
Why
#3340fixed thetype: ["object", "null"]shape, but the explicitanyOf/oneOf+null-member form takes a different path: it enters the combiner branch ofgetObject(packages/core/src/getters/object.ts) and is handed tocombineSchemas, which resolves each member withcombined: trueand — under the v8 defaultaliasCombinedTypes: false— an undefinedpropName. The object member'sgetObjecttherefore receivesname = undefined, and the enum-extraction guard inresolvers/object.tsis blocked (bothpropNameempty andcombinedtrue), so the nested enums inline.How
In the combiner branch, before delegating to
combineSchemas, detect a "nullable object composition" — ananyOf/oneOfwhose members are exactly one inline object with properties plus one or morenull-type members — and divert that single object member to the normal property-iteration path with itsnamepreserved and a synthesized| null. This mirrors the #3340 fix one level up.The guard is deliberately narrow; these keep the existing
combineSchemasbehavior:allOf(intersection semantics, not a nullable union)$refobject members (e.g.anyOf: [{ $ref }, { null }]→Ref | null; the referenced schema extracts at its own definition)[{ object }, { string }, { null }])[{ string }, { null }]){ [key: string]: unknown } | null)The divert is unconditional w.r.t.
aliasCombinedTypes, consistent with #3340 (which also drops the…AnyOfwrapper for the equivalent type-array shape).Related precedent: #2710 / PR #3424 (
isNullableEnumComposition) made nullable enum compositions transparent insidecombineSchemas; this is the object-level analogue.Tests
packages/core/src/generators/schema-definition.test.ts: parametrized regression test overanyOfandoneOfasserting the nested enums extract as namedas constschemas and the object keeps its| null.typecheck, andlint.anyOfandoneOf, includingaliasCombinedTypes: true; the generated schema files compile undertsc --strict.Closes #3563
Summary by CodeRabbit