fix(json-schema): recognize $defs when deriving type names#2970
Merged
Conversation
The definitionName getter in JSONSchemaInput.ts only recognized the legacy "definitions" keyword, so types defined under the standard "$defs" keyword (draft 2019-09+) never had their schema-given name treated as a given name. combineNames then merged the type's other candidate names by common prefix, producing truncated/wrong names (e.g. "LightParams" became "Light"). Fix: also recognize "$defs" in definitionName. Adds a schema fixture (light.schema/light.1.json) exercising the round trip, and a focused unit test asserting the generated TypeScript interface keeps its given name, since fixture tests don't observe generated symbol names. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
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
Fixes #2778.
A JSON Schema that defines a type under
$defsand references it got adifferent, truncated name in generated output than the name given in the
schema. For example, a type named
LightParamsunder$defswas emittedas
Lightin generated TypeScript. Renaming$defsto the legacydefinitionskeyword in the same schema produced the correct name,which pointed at the root cause.
Root cause
Ref.definitionNameinpackages/quicktype-core/src/input/JSONSchemaInput.tsonly recognized the legacy
"definitions"JSON Schema keyword, not"$defs"(the standard keyword for local definitions since draft 2019-09, used by the
2020-12 draft referenced in the issue). Because of this, a type's
$defskeywas never treated as a "given name" for the type. The type kept only its
inferred names (e.g. the referencing property name, plus a name derived
from the input file name), and
combineNamesinpackages/quicktype-core/src/attributes/TypeNames.tsmerged those candidatesby common prefix — which produced the truncated/wrong name.
Fix
definitionNamenow also recognizes"$defs"alongside"definitions":Test coverage
test/inputs/schema/light.schema+test/inputs/schema/light.1.json, reproducing the schema from the issue,runs end-to-end (round-trip) for every schema-input language fixture.
test/unit/json-schema-definitions.test.tsasserts thegenerated TypeScript interface is named
LightParams, notLight. Thisis a unit test rather than a fixture-only check because fixture tests
validate round-tripping, not the exact generated symbol name — the actual
regression here.
test/inputs/schema/*.schemafor any other existing input using$defs;light.schemais the only one, so no other fixture snapshots areaffected by this change.
Verification
npm run build— passes.npm run test:unit— 164/164 tests pass (26 files).export interface Light { ... }(referenced asLight).After:
export interface LightParams { ... }(referenced asLightParams), matching the reporter's expectation.QUICKTEST=true FIXTURE=schema-typescript script/test test/inputs/schema/light.schema— passes.biome checkon changed files — clean.schema-typescriptfixture set locally; confirmed one pre-existing, unrelated failure (vega-lite.schemahits pre-existing TypeScript compile errors, TS2411, on unmodified master as well — unrelated to this change). CI will validate the rest of the language matrix.🤖 Generated with Claude Code