Skip to content

fix(schema): unify $ref-to-$id resolution with directly supplied schemas#3025

Open
schani wants to merge 8 commits into
masterfrom
agent/fix-issue-1833
Open

fix(schema): unify $ref-to-$id resolution with directly supplied schemas#3025
schani wants to merge 8 commits into
masterfrom
agent/fix-issue-1833

Conversation

@schani

@schani schani commented Jul 20, 2026

Copy link
Copy Markdown
Member

Bug

When multiple JSON Schema files are passed as separate top-level inputs and one of them is also reached via a $ref to its $id from another file, quicktype generated two separate types for the same schema instead of unifying them into one.

Repro (from the issue):

node dist/index.js --src-lang schema --just-types -l typescript a.json b.json c.json

where a.json and b.json both have a property that's { "$ref": "/schemas/c" }, and c.json (which declares "$id": "/schemas/c") is passed directly alongside them.

Before the fix:

export interface A {
    in:   InElement[];
    ...
}
export interface InElement { name: string; ... }   // duplicate of C
export interface B {
    out:  InElement[];
    ...
}
export interface C { name: string; ... }            // dead, unreferenced

After the fix:

export interface A { in:  C[]; ... }
export interface B { out: C[]; ... }
export interface C { name: string; ... }

Root cause

In Canonizer.addSchema (packages/quicktype-core/src/input/JSONSchemaInput.ts), the canonical Location for a schema reached through $id lookup was built with Ref.root(address), while the Location for directly supplied top-level schemas is built via Ref.parse. These two representations of the same address didn't compare as equal, so the schema store treated the $ref-reached schema and the directly-supplied top-level schema as two distinct schemas, producing two separate types.

Fix

Use Ref.parse(address) consistently when registering $id-resolved schema roots, matching how top-level schema references are already built:

 this.addIDs(
     schema,
-    new Location(Ref.root(address), Ref.root(undefined)),
+    new Location(Ref.parse(address), Ref.root(undefined)),
 );

Test coverage

  • test/inputs/schema/issue-1833/ — a new multi-file JSON Schema fixture (a.schema, b.schema, c.schema, top-level.schema) reproducing the exact scenario from the issue. JSONSchemaFixture.getSamples in test/fixtures.ts was extended to treat a directory of multiple .schema files under test/inputs/schema/ as one combined sample (mirroring how the CLI treats a directory of schema files as multiple co-equal top-level inputs), so this now runs through every schema-<language> fixture.
  • test/unit/json-schema-multiple-sources.test.ts — a targeted unit test that builds a JSONSchemaInput from three named schema sources (mirroring the internal API) and asserts the generated TypeScript output reuses a single C interface for both A.in and B.out, with no duplicate InElement interface. This is a real regression guard: it fails against the unfixed code and passes with the fix.

Verification performed locally

  • npm run build passes cleanly.
  • Re-ran the exact CLI repro from the issue — output now matches the expected output (no InElement, C shared by both A and B).
  • Confirmed the new unit test fails on the reverted (buggy) source and passes with the fix, by reverting the source change, rebuilding, and re-running it.
  • Full unit suite (npx vitest run): 164/164 tests pass.
  • QUICKTEST=true FIXTURE=schema-typescript script/test: only the new issue-1833 fixture and pre-existing fixtures ran; the only failure is test/inputs/schema/vega-lite.schema failing to compile with TypeScript (TS2411 errors) — verified this failure is pre-existing on unmodified master as well and unrelated to this change.
  • npx biome check on the changed files passes with no issues.
  • CI will additionally validate the other language fixtures.

Fixes #1833

🤖 Generated with Claude Code

schani and others added 7 commits July 20, 2026 17:54
…mas (#1833)

When multiple JSON Schema files are passed as separate top-level inputs and
one of them is also reached via a $ref to its $id from another file (e.g.
a.json and b.json both $ref /schemas/c, and c.json is passed directly as a
top-level schema), quicktype generated two separate types for the same
schema instead of unifying them, leaving one as dead/unreferenced output.

Root cause: in Canonizer.addSchema (JSONSchemaInput.ts), the canonical
Location for a schema reached through $id lookup used `Ref.root(address)`,
while the Location used for directly supplied top-level schemas is derived
via `Ref.parse`. These two forms of the same address didn't compare equal,
so the schema store treated them as different schemas.

Fix: use `Ref.parse(address)` consistently when registering $id-resolved
schema roots, matching how top-level schema references are built.

Added test/inputs/schema/issue-1833/ as a new multi-file JSON Schema
fixture (extending JSONSchemaFixture.getSamples in test/fixtures.ts to
support directories of multiple .schema files as one sample) reproducing
the exact scenario from the issue, plus
test/unit/json-schema-multiple-sources.test.ts, a targeted unit test that
builds a JSONSchemaInput from three named schema sources and asserts the
generated output reuses one C interface and never emits a duplicate
InElement. The unit test fails on unfixed code and passes with the fix.

Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
Co-Authored-By: Claude <noreply@anthropic.com>
…p-level languages

The issue-1833 fixture's top-level.schema was a bare $ref to /schemas/a,
so after unification the 'A' and 'TopLevel' top-levels resolved to the same
type. Languages that emit a per-top-level (Un)marshal helper (e.g. Go)
generated an UnmarshalA helper referencing a type A that no longer existed,
failing to compile.

Make top-level.schema its own object type that references /schemas/a and
/schemas/b, so TopLevel, A, B and C are four distinct top-levels. This still
exercises the fix (C is directly supplied and reached via $ref-to-$id from
both A and B, and A/B are reached via $ref-to-$id from TopLevel) while
generating code that compiles and round-trips across all fixture languages.
Update the positive and no-defaults fail samples to match the new TopLevel
shape.

Co-Authored-By: Claude <noreply@anthropic.com>
…amed drivers

The Elm, Haskell and Objective-C fixture drivers deserialize a single
top-level type with a fixed name (QuickType / QTTopLevel) rather than
TopLevel. The issue-1833 fixture is a directory of co-equal schema files,
so its top-level names come from each file's $id/title (A, B, C, TopLevel)
and none matches those drivers' expected name, causing a compile-time
naming error. Skip the fixture for those three languages; it still runs its
positive and negative cases across the many TopLevel-named languages.

Co-Authored-By: Claude <noreply@anthropic.com>
The Java fixture driver deserializes through the generic
`Converter.fromJsonString`, which quicktype only emits for a single
top-level; multi-top-level generation emits per-type
`<Name>FromJsonString` methods instead, so the driver fails to compile
against the issue-1833 directory input. Skip it there, as already done for
the Elm, Haskell and Objective-C drivers.

Co-Authored-By: Claude <noreply@anthropic.com>
The issue-1833 fixture's only negative case is a missing required property.
The cJSON, Swift, typescript-zod, typescript-effect-schema and Elixir
fixtures already skip required.schema because their generated code does not
treat an absent required property as a failure, so the expected-failure
sample would not fail for them either. Skip issue-1833 for the same
renderers, mirroring the existing required.schema skips.

Co-Authored-By: Claude <noreply@anthropic.com>
@schani

schani commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

Followed up on the multi-file issue-1833 fixture so it runs cleanly across the fixture matrix.

The original top-level.schema was a bare $ref to /schemas/a, so after the $ref-to-$id unification the A and TopLevel top-levels collapsed onto one type; languages that emit a per-top-level (de)serializer (Go, for one) then generated helpers referencing a type that no longer existed and failed to compile. top-level.schema is now its own object that references /schemas/a and /schemas/b, so TopLevel, A, B and C are four distinct top-levels. C is still supplied directly and reached via $ref-to-$id from both A and B, and A/B are reached via $ref-to-$id from TopLevel, so the fixture keeps exercising the fix. The positive and no-defaults fail samples were updated to the new TopLevel shape.

A directory of co-equal schema files is inherently a multi-top-level input, which a few drivers can't consume, so the fixture is skipped for them via the existing skipSchema mechanism:

  • Elm, Haskell and Objective-C decode a single fixed-name top-level (QuickType/QTTopLevel) that this fixture doesn't produce;
  • Java deserializes through the generic Converter.fromJsonString, which is only emitted for a single top-level (multi-top-level emits per-type <Name>FromJsonString);
  • cJSON, Swift, typescript-zod, typescript-effect-schema and Elixir already skip required.schema because they don't treat an absent required property as a failure, and the fixture's only negative case is a missing required property.

It still runs both a positive and a negative case on the many TopLevel-named renderers (Go, Python, C#, C++, Rust, Kotlin, Dart, Scala 3, PHP, Ruby, TypeScript, and more). Full matrix is green.

@github-actions

Copy link
Copy Markdown

Generated-output differences

20 files differ — 0 modified, 20 new, 0 deleted
2844 changed lines — +2844 / −0

Open the generated-output report →

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.

Duplicate types when using several schemas and references

1 participant