fix(typescript): preserve anyOf unions of object types instead of merging#3030
Open
schani wants to merge 7 commits into
Open
fix(typescript): preserve anyOf unions of object types instead of merging#3030schani wants to merge 7 commits into
schani wants to merge 7 commits into
Conversation
…ging (#1338) JSON Schema `anyOf` of two plain object types was flattened by `flattenUnions` into a single merged interface with all properties from both objects, instead of producing a proper TypeScript union of the two named types. `secondProperty`-style unions (array | object) already worked correctly; only object/object unions were affected. Root cause: `flattenUnions` always collapsed non-canonical unions of class/object types via `UnifyUnionBuilder`, regardless of whether the target language could represent a union of named object types. Fix: add `TargetLanguage.supportsUnionsWithMultipleObjectTypes` (default `false`, enabled for TypeScript) and skip flattening non-canonical unions whose members are all class/object types when the target language supports it, except where the union still participates in an intersection (IntersectionAccumulator can only hold one member per kind). Test coverage: new JSON Schema fixture `test/inputs/schema/anyof-object-union.schema` with a passing sample and a `.fail.class-union` sample (mixed properties from both object types, which should fail since additionalProperties is false), wired into the "class-union" feature enabled for the `typescript` fixture. Verified locally: - `npm run build` passes - `QUICKTEST=true FIXTURE=schema-typescript script/test`: 71/71 passed (new fixture included) - `QUICKTEST=true FIXTURE=schema-python script/test`: 71/71 passed (regression check for languages without the new capability) - `QUICKTEST=true FIXTURE=schema-golang script/test`: 71/71 passed - `npm run test:unit`: 170/170 passed - Direct repro now emits `firstProperty?: MyObjectA | MyObjectB;` instead of a merged `MyObject` interface Fixes #1338 Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
The new anyof-object-union.schema has a secondProperty that is an
array|object union. The kotlinx renderer emits such unions as a sealed
class without serializer wiring, so it fails to deserialize at runtime
("Class discriminator was missing ... polymorphic scope of
'SecondProperty'") — the same documented limitation already covered by
kotlinx's other union skips. Add the schema to kotlinx's skipSchema list.
This is a pre-existing kotlinx limitation, not caused by the PR's
TypeScript-only flattenUnions change (kotlinx still merges the object
union in firstProperty as before). Verified the fixture round-trips for
rust locally, and cjson/golang/dart/flow/php/javascript/ruby passed in CI.
Co-Authored-By: Claude <noreply@anthropic.com>
…g constructor-matching limitation) (#undefined) Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
test/unit/cjson-enum-default.test.ts (merged in from master via #3052) asserted alphabetically-sorted enum case order, but the codebase's documented and tested convention (see enum-order.test.ts) is to preserve JSON Schema declaration order. The cjson renderer already does this correctly; only the test's expectation was wrong, causing a deterministic failure on master and on this branch after merging master in. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
Generated-output differences45 files differ — 3 modified, 42 new, 0 deleted |
Generated-output differences46 files differ — 3 modified, 43 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.
Bug
With JSON Schema and the TypeScript target,
anyOfof two plain objecttypes did not turn into a TypeScript union. Instead the two object types
were merged/flattened into a single interface containing all properties
from both, and the referencing property was typed as that merged
interface instead of a union.
Repro schema:
{ "$ref": "#/definitions/TestObject", "definitions": { "TestObject": { "type": "object", "properties": { "firstProperty": { "anyOf": [{ "$ref": "#/definitions/MyObjectA" }, { "$ref": "#/definitions/MyObjectB" }] } } }, "MyObjectA": { "type": "object", "properties": { "a": {"type":"string"}, "b": {"type":"number"}, "c": {"type":"boolean"} } }, "MyObjectB": { "type": "object", "properties": { "d": {"type":"string"}, "e": {"type":"number"}, "f": {"type":"boolean"} } } } }node dist/index.js --lang typescript --src-lang schema test.schemaproduced:instead of the expected
Root cause
flattenUnions(packages/quicktype-core/src/rewrites/FlattenUnions.ts)always collapsed non-canonical unions whose members were class/object
types via
UnifyUnionBuilder/UnifyClasses, regardless of whether thetarget language could actually represent a union of named object types.
TypeScript can (and does, for e.g.
array | objectanyOf combinations),but the class/object-union case was always flattened.
Fix
TargetLanguage.supportsUnionsWithMultipleObjectTypes(
packages/quicktype-core/src/TargetLanguage.ts), defaulting tofalse, and enabled it for TypeScript(
packages/quicktype-core/src/language/TypeScriptFlow/language.ts).flattenUnionsnow skips flattening a non-canonical union when all itsmembers are class/object types and the target language supports
unions of object types — except when that union still participates in
an
IntersectionType, sinceIntersectionAccumulatorcan onlyrepresent one member per kind and still needs the union flattened
there.
Run.tstoflattenUnions.This only changes behavior for target languages that opt in
(TypeScript); all other languages keep flattening class unions exactly
as before.
Test coverage
Added a new JSON Schema fixture:
test/inputs/schema/anyof-object-union.schema— theanyOf-of-two-objects scenario from the issue (plus an array/object
anyOf, whichalready worked, to guard against regressing that case).
test/inputs/schema/anyof-object-union.1.json— a passing round-tripsample.
test/inputs/schema/anyof-object-union.1.fail.class-union.json— asample mixing properties from both object types, which must be
rejected now that the two objects are validated as a real union
(
additionalProperties: falseon both).Wired up via a new
"class-union"fixture feature enabled for thetypescriptlanguage intest/languages.ts, so the.fail.class-unionsample is exercised for
schema-typescript.Verification (local)
npm run buildpassesQUICKTEST=true FIXTURE=schema-typescript script/test: 71/71 passed,including the new fixture and its fail sample
QUICKTEST=true FIXTURE=schema-python script/test: 71/71 passed(confirms non-opted-in languages are unaffected)
QUICKTEST=true FIXTURE=schema-golang script/test: 71/71 passednpm run test:unit: 170/170 passedfirstProperty?: MyObjectA | MyObjectB;instead of a merged
MyObjectinterfaceCI will run the full fixture/unit suite across all languages.
Fixes #1338
🤖 Generated with Claude Code