fix(typescript-flow): emit type aliases for top-level non-object types#3035
Open
schani wants to merge 8 commits into
Open
fix(typescript-flow): emit type aliases for top-level non-object types#3035schani wants to merge 8 commits into
schani wants to merge 8 commits into
Conversation
#1122) With --just-types, a top-level array or map type (e.g. echo '[1,2,3.14]' | quicktype --lang ts --just-types --top-level Foo) silently emitted no output. The TypeScript/Flow renderer only emitted a top-level alias when the top-level type was primitive; arrays and maps fell through with nothing emitted. Named types (interfaces/enums/unions) were fine since they get their own declaration elsewhere. Fix emitTypes() to emit a top-level alias whenever the top-level type has no directly-reachable named type of its own (using the existing directlyReachableSingleNamedType helper, also used by other renderers for the analogous decision), instead of only checking t.isPrimitive(). Also add the "export" keyword to match the export interface/export enum/export type convention used elsewhere in this renderer. Test coverage: - test/unit/typescript-flow-top-level-alias.test.ts: asserts the emitted top-level alias text for TS/Flow top-level arrays and maps, and that a map whose value type claims the top-level name isn't double-declared. - test/inputs/schema/empty-object.schema (+ .1.json): a bare top-level map JSON Schema fixture input, enabled for typescript/flow; skipped for renderers (JavaScript PropTypes, JavaScript, PHP, TypeScript Zod, TypeScript Effect Schema) that don't support a bare top-level map. Verified locally: npm run build, npm run test:unit (176 passed), and the schema-typescript and typescript fixtures (QUICKTEST=true) all pass. CI will additionally validate the other language fixtures. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Co-Authored-By: Claude <noreply@anthropic.com>
Elixir's top-level from_json/to_json facade for non-array, non-class types just does Jason.decode!/Jason.encode! with no validation, so it can never reject a wrong-shaped .fail.json input. The new empty-object.schema fixture (a bare top-level map) hits exactly this case; skip it for Elixir the same way it's already skipped for JavaScriptPropTypes, Flow, PHP, TypeScriptZod, and TypeScriptEffectSchema. Co-Authored-By: gpt-5.6-sol via pi <noreply@openai.com>
Member
Author
|
Reopening to retrigger CI (no run was dispatched for the latest commit). |
Resolve test/languages.ts skipSchema conflicts by combining both sides: keep this branch's empty-object.schema skips (JavaScriptPropTypes, Smithy4s, PHP, TypeScriptZod, TypeScriptEffectSchema, Elixir) alongside master's integer-before-number.schema skips. Co-Authored-By: Claude <noreply@anthropic.com>
The Haskell fixture driver encodes the Maybe result of decodeTopLevel, so a failed decode prints "null" and exits 0 rather than signalling an error. The empty-object.schema fail sample ([1,2,3] against a top-level HashMap) therefore cannot be detected as an expected failure, exactly like the already-skipped boolean-subschema.schema. Skip it for Haskell under the same rationale. Co-Authored-By: Claude <noreply@anthropic.com>
Generated-output differences34 files differ — 6 modified, 28 new, 0 deleted |
Generated-output differences39 files differ — 10 modified, 29 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
--just-typeson a top-level array, the TypeScript/Flow renderer emitted no output at all (exit 0, silent). A top-level map had the same problem. A top-level primitive (e.g. a string) worked fine and correctly emittedtype Foo = string;.Repro:
Before: nothing is printed.
After:
export type Foo = number[];Root cause
TypeScriptFlowBaseRenderer.emitTypes()only emitted a top-level type alias when the top-level type was primitive (t.isPrimitive()). Arrays and maps are not primitives, so they fell through the check and got no declaration at all — and since they also have no name of their own (unlike classes/enums/unions, which get emitted separately byforEachNamedType), nothing was ever emitted for them.Fix
Changed the condition in
emitTypes()to emit a top-level alias whenever the top-level type has no directly-reachable named type of its own, using the existingdirectlyReachableSingleNamedTypehelper (already used by several other language renderers — C#, Dart, Java, JavaScript, PHP, JavaScript PropTypes — for the equivalent decision). This correctly covers primitives, arrays, and maps, while still skipping top levels that are themselves (or resolve to) a named class/enum/union, since those already get their own declaration viaforEachNamedType.Also added the
exportkeyword to the emitted alias to match theexport interface/export enum/export typeconvention used everywhere else in this renderer (and matching the output the original issue expected:export type Foo = number[];).Test coverage
test/unit/typescript-flow-top-level-alias.test.ts: unit tests asserting the emitted top-level alias text for TypeScript/Flow top-level arrays and top-level maps, plus a regression test that a map whose value type claims the top-level name is not double-declared. A unit test was used here (per repo conventions) because the primary e2e round-trip fixtures compile/run the full converter code, which doesn't exercise--just-typesoutput — the driver programs would compile identically whether or not the top-level alias declaration itself is present, so a fixture can't detect this specific regression.test/inputs/schema/empty-object.schema(+.1.json): a new bare top-level map JSON Schema fixture input, enabled for thetypescriptandflowfixtures (which exercise the fix in the full/non---just-typescode path too), and skipped for renderers that don't support a bare top-level map at all (JavaScript PropTypes, JavaScript, PHP, TypeScript Zod, TypeScript Effect Schema) — a pre-existing, unrelated limitation.Verification (local)
npm run buildpasses.npm run test:unit— 176 tests passed, including the new ones.QUICKTEST=true FIXTURE=typescript script/test— all 79 tests passed.QUICKTEST=true FIXTURE=schema-typescript script/test— all 71 tests passed (including the newempty-object.schema).flow check/flow-node) could not be run locally because theflowtoolchain isn't installed in this environment; CI will validate it.Fixes #1122
🤖 Generated with Claude Code