Skip to content

fix(schema-input): unify $ref to another top-level schema source - #3020

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

fix(schema-input): unify $ref to another top-level schema source#3020
schani wants to merge 8 commits into
masterfrom
agent/fix-issue-1543

Conversation

@schani

@schani schani commented Jul 20, 2026

Copy link
Copy Markdown
Member

Bug

When generating from multiple JSON Schema sources where one schema's property $refs another top-level source, quicktype emitted two separate, structurally-identical interfaces instead of reusing the top-level type.

Repro (--src=<dir> with two files, one $ref-ing the other):

node dist/index.js --src=res --src-lang=schema --just-types --lang=ts

res/WebsiteOrder.schema has a property info that is "$ref": "OrderCustomer.schema#", and res/OrderCustomer.schema is itself a top-level source. Before the fix, quicktype produced both OrderCustomer (from the top-level source) and a duplicate Info interface (synthesized for the $ref-ed property), with WebsiteOrder.info: Info. Expected: a single OrderCustomer interface reused for WebsiteOrder.info: OrderCustomer.

Root cause

In addTypesInSchema (packages/quicktype-core/src/input/JSONSchemaInput.ts), a property schema that was a pure $ref alias (only $ref, a matching type, or the draft-3 boolean required keyword present) was still routed through the same "needs a union/intersection" path as schemas with real narrowing keywords (properties, items, enum, etc.). That path always built a fresh type for the schema instead of reusing the identity of the resolved $ref target, so a $ref pointing at another top-level source got re-synthesized as a brand-new type rather than unified with the type already generated for that source.

Fix

$ref resolution is hoisted earlier so it's available when deciding whether a union/intersection is needed. A new isRefAlias check recognizes schemas that are pure $ref aliases (no narrowing keywords beyond a same-typed type or boolean required) and skips the union path for them, letting the existing $ref-target-reuse logic (already present further down) unify the type instead of synthesizing a duplicate.

Test coverage

  • test/inputs/schema/multi-source-1543/ plus positive and negative JSON samples: the exact directory-as-source repro is registered for schema-typescript and runs end to end through the standard TypeScript compile/round-trip fixture.
  • test/unit/schema-multiple-sources.test.ts: the round trip cannot detect structurally identical duplicate interfaces, so this targeted assertion verifies that info reuses OrderCustomer and no redundant Info interface is emitted.

Verification

  • npm run build passes.
  • The exact base/head repro changes from duplicate OrderCustomer and Info interfaces to one OrderCustomer, with TopLevel.info: OrderCustomer.
  • npx vitest run test/unit/schema-multiple-sources.test.ts passes.
  • QUICKTEST=true CPUs=2 FIXTURE=schema-typescript npm run test:fixtures passes all 81 cases, including the positive and expected-failure multi-source-1543 samples.
  • A focused generated-output snapshot comparison reports exactly one added fixture output, schema-typescript/test/inputs/schema/multi-source-1543/default/TopLevel.ts, containing info: OrderCustomer and no Info interface.

Fixes #1543

🤖 Generated with Claude Code

schani and others added 4 commits July 20, 2026 17:48
Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
Co-Authored-By: Claude <noreply@anthropic.com>
The directory-source fixture for #1543 cannot run through the round-trip
harness: a directory always yields multiple top-level types (one per file),
and renderers emit per-type entry points (java `TopLevelFromJsonString`, elm
per-type decoders, etc.) instead of the single `fromJsonString`/top-level the
shared fixture drivers assume, so the sample failed to compile for java, elm,
and others. The scenario also can't be reduced to a single top-level, since
the bug requires two top-level sources where one $refs the other.

The round-trip harness also can't detect the actual bug anyway: the fix only
removes a duplicate structurally-identical interface, and both the buggy and
fixed output round-trip the sample identically. That regression is already
covered by test/unit/schema-multiple-sources.test.ts, which asserts the
duplicate `Info` interface is not generated and `info` reuses `OrderCustomer`.

Remove the fixture registration and its round-trip sample JSON; keep the
schema files under test/inputs/schema/multi-source-1543 as the unit test's
input, and clarify the test comment.

Co-Authored-By: Claude <noreply@anthropic.com>
@schani
schani force-pushed the agent/fix-issue-1543 branch from a0d5235 to 0ca2f44 Compare July 21, 2026 16:16
@schani

schani commented Jul 21, 2026

Copy link
Copy Markdown
Member Author

Heads up on a change to the test setup here. The multi-source-1543 directory fixture couldn't run through the round-trip harness: a directory source always produces multiple top-level types (one per file), and several backends emit per-type entry points for that case (java's Converter.TopLevelFromJsonString instead of fromJsonString, elm's per-type decoders, etc.) rather than the single top-level the shared fixture drivers assume. That made the sample fail to compile for java and elm, and it would fail the same way for other backends; the scenario can't be reduced to a single top-level because the bug specifically needs two top-level sources where one $refs the other.

The round-trip harness also can't observe the actual bug — the fix only removes a duplicate structurally-identical interface, so buggy and fixed output round-trip the sample identically. That regression is fully covered by test/unit/schema-multiple-sources.test.ts, which asserts the duplicate Info interface is not emitted and info reuses OrderCustomer.

I dropped the fixture registration and its round-trip sample JSON, kept the schema files under test/inputs/schema/multi-source-1543 as the unit test's input, and clarified the test comment. The core JSONSchemaInput fix is unchanged.

@github-actions

Copy link
Copy Markdown

No generated-output differences

✅ Generated outputs are unchanged between the PR base and head revisions.

@github-actions

github-actions Bot commented Jul 22, 2026

Copy link
Copy Markdown

Generated-output differences

1 files differ — 0 modified, 1 new, 0 deleted
219 changed lines — +219 / −0

Open the generated-output report →

@github-actions

Copy link
Copy Markdown

Generated-output differences

1 files differ — 0 modified, 1 new, 0 deleted
219 changed lines — +219 / −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.

Unable to combine defintions when generating TypeScript from multiple files

2 participants