Skip to content

fix(downgrader): merge a oneOf with null, as typia and Litestar write it, in 3.1 to 3.0 - #105

Merged
dinwwwh merged 2 commits into
mainfrom
claude/project-thread-xecbgf
Oct 25, 2026
Merged

dinwwwh merged 2 commits into
mainfrom
claude/project-thread-xecbgf

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Oct 25, 2026

Copy link
Copy Markdown
Member

Requested by dinwwwh · project thread

Since #87, 3.1 to 3.0 merges anyOf: [X, { type: "null" }] into X with nullable: true. typia and Litestar write the same nullable value as a oneOf (so do the Figma, Phoenix and Lago docs):

{ "oneOf": [{ "type": "object", "properties": { "a": { "type": "string" } } }, { "type": "null" }] }

That pair stayed a oneOf in 3.0, and oapi-codegen turns each one into a union wrapper (17 unions vs 9 merged on a Litestar app) instead of a nullable field.

Now a oneOf of two branches is queued beside the anyOfs and merges under the same conditions:

  • Only a pair: a oneOf with { type: "null" } twice rejects null, so it stays.
  • A branch with $ref, enum, an applicator or nullable, or one that shares a keyword with the schema around it, stays, as for anyOf. The anyOf to oneOf rewrite for those does not apply, since it is already a oneOf.
  • A $ref kept as written that points into /oneOf/ keeps the oneOf, as fix(downgrader): keep a nullable anyOf that a $ref points into in 3.1 to 3.0 #92 does for /anyOf/.

One README row and one test. pnpm run check and the tests pass with 100% coverage. A gc'd, shuffled wall-time bench with twin builds shows no change (api 100 resources 19.8 ms, corpus 1.69 ms, path item chain 200 1.07 ms, reference diamond 64 0.55 ms on both main and this branch).


Generated by Claude Code

… it, in 3.1 to 3.0

`oneOf: [X, { type: "null" }]` now becomes X with `nullable: true` under
the same conditions as the `anyOf` pair (#87), so oapi-codegen and other
generators read a nullable value instead of a union. Only a pair merges,
since a `oneOf` with null twice rejects null, and a `$ref` into the
`oneOf` keeps it.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017DRHCq36GvkQQjq7BJHMsZ
@dinwwwh dinwwwh self-assigned this Oct 25, 2026
@codecov

codecov Bot commented Oct 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Oct 25, 2026 •

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 21 untouched benchmarks


Comparing claude/project-thread-xecbgf (65348f7) with main (b450552)

Open in CodSpeed

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ℹ️ No critical issues — one minor documentation follow-up.

Reviewed changes

  • Merge a null oneOf pair — finishSchema now queues a two-branch out.oneOf alongside the existing anyOf cases, and mergeNullable(out, root, key) collapses oneOf: [X, { type: "null" }] into X with nullable: true under the same guards as anyOf (no $ref/enum/applicator/nullable on the branch, no key shared with the schema around it).
  • $ref into a oneOf keeps it — convertSchemaRef now scans /oneOf/ as well as /anyOf/ segments, so unionTargets protects a oneOf a kept $ref points into.
  • Rename — nullableAnyOfs/anyOfTargets become nullableUnions/unionTargets carrying a key: 'anyOf' | 'oneOf' tag; the anyOf → oneOf rewrite branch is gated to key === 'anyOf', since a oneOf input is already the desired shape.
  • Test + README row — a new it covers the merge, the null-twice case, type: [...]/enum/$ref branches staying, and a $ref into /oneOf/ keeping it; one behavior-table row documents the pair.

Verified locally: vitest run 269/269 pass and uncheck on the three touched files passes (oxlint, oxfmt, tsc). I traced the merge equivalence (type with a non-null string rejects null, so the pair equals X + nullable) and found no correctness gap.

ℹ️ README limitations still name only anyOf for the $ref-pointed-into union

The Limitations list explains why a null union that a $ref points into is left as written, but it names anyOf alone. This PR extends exactly that behavior to oneOf (with a test asserting it), so a reader no longer learns the whole rule.

Technical details
# README limitation omits the new `oneOf` case

## Affected sites
- `packages/downgrader/README.md:249` — "An `anyOf` of `{ type: "null" }` and a branch that a `$ref` points into stays as written, so the `$ref` still resolves, and code generators read it as a union." Only names `anyOf`.

## Required outcome
- The sentence reflects that the same preservation now applies to a `oneOf` of `{ type: "null" }` and one other branch (e.g. widen to "an `anyOf` or `oneOf`" or add a oneOf example).

## Notes
- Purely a docs completeness nit, non-blocking. The behavior itself is correct and covered by the new test (`packages/downgrader/tests/v3.1-to-v3.0.test.ts:596`).

Pullfrog  | Fix it ➔ | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_017DRHCq36GvkQQjq7BJHMsZ

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • Widened the null-union $ref limitation — commit 65348f7 updates the README Limitations line to name oneOf alongside anyOf for a null union that a kept $ref points into, matching the behavior convertSchemaRef/unionTargets already implement. This addresses the sole finding from the prior review; no source or test changes since 6afbd7b.

Pullfrog  | View workflow run | Using deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

@dinwwwh
dinwwwh merged commit 8cdcda1 into main Oct 25, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants