fix(core): preserve distinct object alternatives in oneOf/anyOf unions#3042
Open
schani wants to merge 10 commits into
Open
fix(core): preserve distinct object alternatives in oneOf/anyOf unions#3042schani wants to merge 10 commits into
schani wants to merge 10 commits into
Conversation
#1266) Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
The new one-of-objects.schema fixture uses a top-level array, which the PHP driver does not support (generated TopLevel::from expects stdClass, not array) - the same pre-existing limitation for which union.schema is already skipped. The oneOf-of-objects behavior this fixture exercises is only enabled for TS/JS/Flow, so PHP output is unaffected. Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…T> (#1266) emitTopLevelArray unconditionally emitted `typealias X = JsonArray<T>`, but kotlinx.serialization.json.JsonArray is a concrete, non-generic class, not a generic container. This only compiled by accident for element types that stringify to JsonObject/JsonElement (the one case where a bare `JsonArray` is correct). The new one-of-objects.schema fixture (top-level array of a oneOf union of distinct objects) exposed this for the first time, breaking the kotlinx/schema-kotlinx CI job. Mirror the same JsonObject/JsonElement special case already used in arrayType(): emit bare `JsonArray` for raw JSON element arrays, and a real generic `List<T>` otherwise. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
…st (#1266) test/unit/cjson-enum-default.test.ts (landed on master in 89d9f2a) hardcoded its expected enum body in alphabetical order, but master's ConvenienceRenderer.forEachEnumCase (f58485e, "preserve JSON Schema enum case order") already stopped alphabetizing enum cases in favor of source declaration order. The test was never updated to match, so it fails identically on master alone (verified independent of this PR's changes) and blocks PR #1266's CI once merged with base. Correct the expected case order to the declaration order (state, config, heartbeat) that quicktype now intentionally produces; the test's actual purpose (asserting cJSON enums start at 1) is unaffected. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
Generated-output differences57 files differ — 22 modified, 35 new, 0 deleted |
# Conflicts: # packages/quicktype-core/src/language/TypeScriptFlow/TypeScriptRenderer.ts
Generated-output differences58 files differ — 22 modified, 36 new, 0 deleted |
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.
The bug
Given a JSON Schema
oneOf(oranyOf) of two or more disjoint objectschemas, quicktype merged them into a single object type with all properties
made optional (plus an added index signature) instead of generating a
TypeScript union of the distinct object shapes.
Repro (from the issue):
{ "oneOf": [ { "type": "object", "required": ["id"], "properties": { "id": { "type": "integer" } } }, { "type": "object", "required": ["externalId"], "properties": { "externalId": { "type": "string" } } } ] }Before:
After:
Root cause
Schema parsing built the correct union, but
flattenUnions/UnifyUnionBuildergrouped all
object-kind union members together and unconditionally mergedtheir properties into a single object (
UnionType.canBeInlinedInTypeScriptdisallowed more than one object-typed member, and
UnifyUnionBuilder.makeObjectunioned all their properties as optional). There was no mechanism for marking
some object union members as distinct alternatives that must survive
unification.
The fix
packages/quicktype-core/src/attributes/UnionMembers.ts)to mark object types that originated as standalone
oneOf/anyOfalternatives (not
allOf/intersection members) as "distinct" union members,and to mark
oneOfunions as mutually exclusive.JSONSchemaInput.tsnow tags standaloneoneOf/anyOfobject cases withthis attribute instead of letting them unify like ordinary compatible
objects.
UnionType.canBeInlinedInTypeScript,UnifyUnionBuilder, andUnionBuilderwere updated to keep multiple distinct-object union membersseparate through flattening/unification instead of collapsing them.
TargetLanguage.supportsUnionsWithMultipleObjectTypes(defaultfalse) and enabled it for TypeScript, Flow, JavaScript, and JSON Schemaoutput, so other languages are unaffected.
oneOf), sibling properties are excluded per-member with"prop"?: neverguards so the generated union stays exclusive under TypeScript's excess
property checks.
Test coverage
test/inputs/schema/one-of-objects.schema(+.1.jsonvalid sample and.2.fail.one-of.jsonexpected-failure sample exercising the newone-offixture feature) — enabled for the
typescript,javascript, andflowfixtures via
test/languages.ts.test/unit/schema-object-unions.test.ts— a focused unit test reproducingthe exact issue Union type not supported for objects #1266 schema for both
oneOfandanyOf, asserting theoutput is a union of separate interfaces rather than one merged interface.
Coverage was added and confirmed failing against the unmodified code before
implementing the fix, per repo convention.
Verification
npm run build— passes.npm run lint(Biome) — zero errors.npx vitest run test/unit— 172/172 tests pass.QUICKTEST=true FIXTURE=typescript,schema-typescript,javascript script/test— 208/208 tests pass (run twice to rule out flakiness; an initial run that
showed a couple of unrelated transient failures under heavy concurrent
machine load did not reproduce on repeat, or in isolation, or against a
clean baseline worktree).
corrected union output shown above.
only ones opting into this behavior) are left to CI, since
supportsUnionsWithMultipleObjectTypesdefaults tofalseand this changeshould not affect their generated output.
Fixes #1266
🤖 Generated with Claude Code