fix(typescript-input): dedupe object type aliases referenced by properties#3043
Open
schani wants to merge 3 commits into
Open
fix(typescript-input): dedupe object type aliases referenced by properties#3043schani wants to merge 3 commits into
schani wants to merge 3 commits into
Conversation
…rties (#879) Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
…e-76a-76 # Conflicts: # test/unit/typescript-input.test.ts
…age-3043-1784733889
No generated-output differences✅ Generated outputs are unchanged between the PR base and head revisions. |
No generated-output differences✅ Generated outputs are unchanged between the PR base and head revisions. |
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
TypeScript source used as quicktype's input, where an object type alias
(
type X = { ... }) is referenced by a property, produced a duplicatetype. For example:
node dist/index.js --src-lang typescript --lang typescript input.tsproducedboth an
AuthTokensinterface and a spurious duplicateTokensinterface,with
UserInfo.tokenstypedTokensinstead ofAuthTokens. This did nothappen when
AuthTokenswas aclassorinterface, matching the originalreport.
Root cause
quicktype-typescript-inputbuilds a JSON Schema from TypeScript source viathe pinned
typescript-json-schemapackage. That package'sgetTypeDefinitiononly emits a
$refto a named definition when the referenced symbol isref-able; for
type X = {...}aliases, the resolved property type is an"anonymous object type" internally (as opposed to a
class/interface,which is not anonymous), so it gets fully inlined instead of
$ref'd unlessthe generator's
aliasRefoption is enabled — and enabling that optionintroduces its own indirection artifacts (extra
__type/_1-suffixeddefinitions, chained refs) rather than a clean fix.
The fix
packages/quicktype-typescript-input/src/index.tsalready monkey-patches thetypescript-json-schemagenerator'sgetTypeDefinitionin one place(
patchGeneratorForBuiltinTypes, forMap/unsupported builtins). This PRadds a second, analogous patch,
patchGeneratorForTypeAliases: when aproperty's type resolves to an anonymous object type that is directly backed
by a (non-generic) top-level type-alias symbol, it rewrites the definition
into a
$refpointing at that alias's own top-level schema definitioninstead of letting
typescript-json-schemainline a structural duplicate.Test coverage
Added to
test/unit/typescript-input.test.ts(the existing unit-test filefor
schemaForTypeScriptSources):schema has exactly
AuthTokensandUserInfodefinitions (no duplicate),and that
UserInfo.tokensis a$reftoAuthTokens.class-based property types is unchanged.Note: quicktype's fixture-test framework (
test/inputs/json,test/inputs/schema,test/languages.ts,test/fixtures.ts) only drivesquicktype's output side — TypeScript is registered there solely as an
output language, never as an input source — so there is no fixture mechanism
for "TypeScript source as input."
schemaForTypeScriptSourcesalready hasdirect unit-test coverage for this reason, which is where this regression
test was added.
Verification
npm run buildpasses.npm run test:unit— all 26 files / 172 tests pass (was 12 tests in theTypeScript-input file, now includes the 2 new ones).
(
node dist/index.js --src-lang typescript --lang typescript input.ts)before and after: before, output has both
AuthTokensand duplicateTokens; after, onlyAuthTokensis emitted andUserInfo.tokens: AuthTokens.interface, array-of-alias, and generic-alias shapes toconfirm no regressions in already-working cases.
CI will additionally run the full fixture suite across all output languages.
Fixes #879
🤖 Generated with Claude Code