From 3edd2cdb57f694992afadcb140c7c087b9d04b91 Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 11 Sep 2025 13:33:56 -0700 Subject: [PATCH 01/16] change refs emission to be for all named non-container types (still do not auto-promote root) --- packages/schema-generator/README.md | 28 +++++++ .../schema-generator/src/schema-generator.ts | 19 ++--- .../schema/alias-edge-cases.expected.json | 40 +++++----- .../fixtures/schema/date-types.expected.json | 38 ++++++++- .../default-nullable-null.expected.json | 4 +- .../schema/default-type.expected.json | 65 ++++++++++++++-- .../defaults-complex-array.expected.json | 77 +++++++++++++++---- .../defaults-complex-object.expected.json | 8 +- .../nested-default-aliases.expected.json | 14 ++-- .../recipe-with-types-input.expected.json | 61 ++++++++++----- .../recipe-with-types-output.expected.json | 69 ++++++++++++----- .../recursive-type-nested.expected.json | 11 +++ .../schema/simple-interface.expected.json | 4 +- .../test/schema/complex-defaults.test.ts | 14 +++- .../schema/type-aliases-and-shared.test.ts | 13 ++-- .../test/schema/type-to-schema.test.ts | 9 ++- 16 files changed, 358 insertions(+), 116 deletions(-) create mode 100644 packages/schema-generator/README.md diff --git a/packages/schema-generator/README.md b/packages/schema-generator/README.md new file mode 100644 index 000000000..1f2e31411 --- /dev/null +++ b/packages/schema-generator/README.md @@ -0,0 +1,28 @@ +# @commontools/schema-generator + +Short notes on the JSON Schema generator and ref/definitions behavior. + +## All‑Named Hoisting (default) + +- Hoists every named type into `definitions` and emits `$ref` for non‑root + occurrences. +- Excludes wrapper/container names from hoisting: `Array`, `ReadonlyArray`, + `Cell`, `Stream`, `Default`, `Date`. +- Root types remain inline; `definitions` are included only if at least one + `$ref` is emitted. +- Anonymous/type‑literal shapes (including aliases that resolve to anonymous + types) are inlined. +- `$ref` may appear with Common Tools extensions as siblings (e.g. + `{ "$ref": "#/definitions/Foo", asStream: true }`). + +Rationale: Improves human readability and re‑use of complex shared shapes while +keeping wrapper semantics explicit and simple. + +Implementation: see `src/schema-generator.ts` (`formatType`) and +`src/type-utils.ts` (`getNamedTypeKey` filtering). + +## Running + +- Check typings: `deno task check` +- Run tests: `deno task test` + diff --git a/packages/schema-generator/src/schema-generator.ts b/packages/schema-generator/src/schema-generator.ts index 32759a253..5c03aa613 100644 --- a/packages/schema-generator/src/schema-generator.ts +++ b/packages/schema-generator/src/schema-generator.ts @@ -125,22 +125,23 @@ export class SchemaGenerator implements ISchemaGenerator { context: GenerationContext, isRootType: boolean = false, ): SchemaDefinition { - // Early named-type handling for named types (wrappers/anonymous filtered) - const inCycle = context.cyclicTypes.has(type); + // All-named strategy: + // Hoist every named type (excluding wrappers filtered by getNamedTypeKey) + // into definitions and return $ref for non-root uses. Cycle detection + // still applies via definitionStack. const namedKey = getNamedTypeKey(type); - const inCycleByName = !!(namedKey && context.cyclicNames.has(namedKey)); - - if (namedKey && (inCycle || inCycleByName)) { + if (namedKey) { if ( context.inProgressNames.has(namedKey) || context.definitions[namedKey] ) { + // Already being built or exists: emit a ref context.emittedRefs.add(namedKey); context.definitionStack.delete( this.createStackKey(type, context.typeNode, context.typeChecker), ); return { "$ref": `#/definitions/${namedKey}` }; } - // Mark as in-progress but delay writing definition until filled to preserve post-order + // Start building this named type; we'll store the result below context.inProgressNames.add(namedKey); } @@ -174,9 +175,8 @@ export class SchemaGenerator implements ISchemaGenerator { if (formatter.supportsType(type, context)) { const result = formatter.formatType(type, context); - // If we seeded a named placeholder, fill it and return $ref for non-root - if (namedKey && (inCycle || inCycleByName)) { - // Finish cyclic def + // If this is a named type (all-named policy), store in definitions. + if (namedKey) { context.definitions[namedKey] = result; context.inProgressNames.delete(namedKey); context.definitionStack.delete( @@ -186,6 +186,7 @@ export class SchemaGenerator implements ISchemaGenerator { context.emittedRefs.add(namedKey); return { "$ref": `#/definitions/${namedKey}` }; } + // For root, keep inline; buildFinalSchema may promote if we choose } // Pop after formatting context.definitionStack.delete( diff --git a/packages/schema-generator/test/fixtures/schema/alias-edge-cases.expected.json b/packages/schema-generator/test/fixtures/schema/alias-edge-cases.expected.json index 275d4df4c..fcc5c334c 100644 --- a/packages/schema-generator/test/fixtures/schema/alias-edge-cases.expected.json +++ b/packages/schema-generator/test/fixtures/schema/alias-edge-cases.expected.json @@ -1,18 +1,10 @@ { "properties": { - "veryDeepCell": { - "asCell": true, - "type": "string" - }, - "deepStream": { + "deepNestedStream": { "asStream": true, "type": "number" }, - "stringCellStream": { - "asStream": true, - "type": "string" - }, - "deepNestedStream": { + "deepStream": { "asStream": true, "type": "number" }, @@ -30,39 +22,47 @@ }, "type": "array" }, + "stringCellStream": { + "asStream": true, + "type": "string" + }, + "userStore": { + "asCell": true, + "type": "string" + }, "users": { "asCell": true, "items": { "properties": { - "name": { - "type": "string" - }, "id": { "type": "number" + }, + "name": { + "type": "string" } }, "required": [ - "name", - "id" + "id", + "name" ], "type": "object" }, "type": "array" }, - "userStore": { + "veryDeepCell": { "asCell": true, "type": "string" } }, "required": [ - "veryDeepCell", - "deepStream", - "stringCellStream", "deepNestedStream", + "deepStream", "indirectArray", "numberListCell", + "stringCellStream", + "userStore", "users", - "userStore" + "veryDeepCell" ], "type": "object" } diff --git a/packages/schema-generator/test/fixtures/schema/date-types.expected.json b/packages/schema-generator/test/fixtures/schema/date-types.expected.json index fa9efa720..f787616ca 100644 --- a/packages/schema-generator/test/fixtures/schema/date-types.expected.json +++ b/packages/schema-generator/test/fixtures/schema/date-types.expected.json @@ -1,6 +1,6 @@ { - "properties": { - "document": { + "definitions": { + "Document": { "properties": { "archivedAt": { "anyOf": [ @@ -31,7 +31,7 @@ ], "type": "object" }, - "eventLog": { + "EventLog": { "properties": { "eventType": { "type": "string" @@ -48,7 +48,26 @@ ], "type": "object" }, - "userProfile": { + "SchemaRoot": { + "properties": { + "document": { + "$ref": "#/definitions/Document" + }, + "eventLog": { + "$ref": "#/definitions/EventLog" + }, + "userProfile": { + "$ref": "#/definitions/UserProfile" + } + }, + "required": [ + "document", + "eventLog", + "userProfile" + ], + "type": "object" + }, + "UserProfile": { "properties": { "createdAt": { "format": "date-time", @@ -94,6 +113,17 @@ "type": "object" } }, + "properties": { + "document": { + "$ref": "#/definitions/Document" + }, + "eventLog": { + "$ref": "#/definitions/EventLog" + }, + "userProfile": { + "$ref": "#/definitions/UserProfile" + } + }, "required": [ "document", "eventLog", diff --git a/packages/schema-generator/test/fixtures/schema/default-nullable-null.expected.json b/packages/schema-generator/test/fixtures/schema/default-nullable-null.expected.json index f65f9d84c..2f39deab2 100644 --- a/packages/schema-generator/test/fixtures/schema/default-nullable-null.expected.json +++ b/packages/schema-generator/test/fixtures/schema/default-nullable-null.expected.json @@ -1,7 +1,6 @@ { "properties": { "maybe": { - "default": null, "anyOf": [ { "type": "null" @@ -9,7 +8,8 @@ { "type": "string" } - ] + ], + "default": null } }, "required": [ diff --git a/packages/schema-generator/test/fixtures/schema/default-type.expected.json b/packages/schema-generator/test/fixtures/schema/default-type.expected.json index 1d707fcac..0d9da597c 100644 --- a/packages/schema-generator/test/fixtures/schema/default-type.expected.json +++ b/packages/schema-generator/test/fixtures/schema/default-type.expected.json @@ -1,6 +1,6 @@ { - "properties": { - "appConfig": { + "definitions": { + "AppConfig": { "properties": { "features": { "properties": { @@ -55,7 +55,7 @@ ], "type": "object" }, - "cellDefaults": { + "CellDefaults": { "properties": { "counter": { "asCell": true, @@ -77,7 +77,7 @@ ], "type": "object" }, - "complexDefault": { + "ComplexDefault": { "properties": { "config": { "default": { @@ -124,7 +124,7 @@ ], "type": "object" }, - "listConfig": { + "ListConfig": { "properties": { "items": { "default": [ @@ -152,7 +152,7 @@ ], "type": "object" }, - "optionalDefaults": { + "OptionalWithDefaults": { "properties": { "nestedOptional": { "properties": { @@ -176,7 +176,38 @@ ], "type": "object" }, - "userSettings": { + "SchemaRoot": { + "properties": { + "appConfig": { + "$ref": "#/definitions/AppConfig" + }, + "cellDefaults": { + "$ref": "#/definitions/CellDefaults" + }, + "complexDefault": { + "$ref": "#/definitions/ComplexDefault" + }, + "listConfig": { + "$ref": "#/definitions/ListConfig" + }, + "optionalDefaults": { + "$ref": "#/definitions/OptionalWithDefaults" + }, + "userSettings": { + "$ref": "#/definitions/UserSettings" + } + }, + "required": [ + "appConfig", + "cellDefaults", + "complexDefault", + "listConfig", + "optionalDefaults", + "userSettings" + ], + "type": "object" + }, + "UserSettings": { "properties": { "fontSize": { "default": 16, @@ -199,6 +230,26 @@ "type": "object" } }, + "properties": { + "appConfig": { + "$ref": "#/definitions/AppConfig" + }, + "cellDefaults": { + "$ref": "#/definitions/CellDefaults" + }, + "complexDefault": { + "$ref": "#/definitions/ComplexDefault" + }, + "listConfig": { + "$ref": "#/definitions/ListConfig" + }, + "optionalDefaults": { + "$ref": "#/definitions/OptionalWithDefaults" + }, + "userSettings": { + "$ref": "#/definitions/UserSettings" + } + }, "required": [ "appConfig", "cellDefaults", diff --git a/packages/schema-generator/test/fixtures/schema/defaults-complex-array.expected.json b/packages/schema-generator/test/fixtures/schema/defaults-complex-array.expected.json index 122d84e67..69b0bcb63 100644 --- a/packages/schema-generator/test/fixtures/schema/defaults-complex-array.expected.json +++ b/packages/schema-generator/test/fixtures/schema/defaults-complex-array.expected.json @@ -1,21 +1,72 @@ { + "definitions": { + "SchemaRoot": { + "properties": { + "emptyItems": { + "default": [], + "items": { + "$ref": "#/definitions/TodoItem" + }, + "type": "array" + }, + "matrix": { + "default": [ + [ + 1, + 2 + ], + [ + 3, + 4 + ] + ], + "items": { + "items": { + "type": "number" + }, + "type": "array" + }, + "type": "array" + }, + "prefilledItems": { + "default": [ + "item1", + "item2" + ], + "items": { + "type": "string" + }, + "type": "array" + } + }, + "required": [ + "emptyItems", + "matrix", + "prefilledItems" + ], + "type": "object" + }, + "TodoItem": { + "properties": { + "done": { + "type": "boolean" + }, + "title": { + "type": "string" + } + }, + "required": [ + "done", + "title" + ], + "type": "object" + } + }, "properties": { "emptyItems": { "default": [], "items": { - "properties": { - "done": { - "type": "boolean" - }, - "title": { - "type": "string" - } - }, - "required": [ - "done", - "title" - ], - "type": "object" + "$ref": "#/definitions/TodoItem" }, "type": "array" }, diff --git a/packages/schema-generator/test/fixtures/schema/defaults-complex-object.expected.json b/packages/schema-generator/test/fixtures/schema/defaults-complex-object.expected.json index 3970e5077..5d9de49ae 100644 --- a/packages/schema-generator/test/fixtures/schema/defaults-complex-object.expected.json +++ b/packages/schema-generator/test/fixtures/schema/defaults-complex-object.expected.json @@ -14,8 +14,8 @@ } }, "required": [ - "theme", - "count" + "count", + "theme" ], "type": "object" }, @@ -41,8 +41,8 @@ } }, "required": [ - "notifications", - "email" + "email", + "notifications" ], "type": "object" } diff --git a/packages/schema-generator/test/fixtures/schema/nested-default-aliases.expected.json b/packages/schema-generator/test/fixtures/schema/nested-default-aliases.expected.json index f3d3feb0b..2b4ad7139 100644 --- a/packages/schema-generator/test/fixtures/schema/nested-default-aliases.expected.json +++ b/packages/schema-generator/test/fixtures/schema/nested-default-aliases.expected.json @@ -4,19 +4,19 @@ "default": "direct", "type": "string" }, - "singleAlias": { - "default": "single", - "type": "string" - }, "doubleAlias": { "default": "double", "type": "string" + }, + "singleAlias": { + "default": "single", + "type": "string" } }, "required": [ "directDefault", - "singleAlias", - "doubleAlias" + "doubleAlias", + "singleAlias" ], "type": "object" -} \ No newline at end of file +} diff --git a/packages/schema-generator/test/fixtures/schema/recipe-with-types-input.expected.json b/packages/schema-generator/test/fixtures/schema/recipe-with-types-input.expected.json index 42498ef9f..0ea6c8dbd 100644 --- a/packages/schema-generator/test/fixtures/schema/recipe-with-types-input.expected.json +++ b/packages/schema-generator/test/fixtures/schema/recipe-with-types-input.expected.json @@ -1,29 +1,54 @@ { - "type": "object", - "properties": { - "title": { - "type": "string", - "default": "untitled" + "definitions": { + "InputSchemaInterface": { + "properties": { + "items": { + "default": [], + "items": { + "$ref": "#/definitions/Item" + }, + "type": "array" + }, + "title": { + "default": "untitled", + "type": "string" + } + }, + "required": [ + "items", + "title" + ], + "type": "object" }, + "Item": { + "properties": { + "text": { + "default": "", + "type": "string" + } + }, + "required": [ + "text" + ], + "type": "object" + } + }, + "properties": { "items": { - "type": "array", + "default": [], "items": { - "type": "object", - "properties": { - "text": { - "type": "string", - "default": "" - } - }, - "required": [ - "text" - ] + "$ref": "#/definitions/Item" }, - "default": [] + "type": "array" + }, + "title": { + "default": "untitled", + "type": "string" } }, "required": [ "items", "title" - ] + ], + "type": "object" } diff --git a/packages/schema-generator/test/fixtures/schema/recipe-with-types-output.expected.json b/packages/schema-generator/test/fixtures/schema/recipe-with-types-output.expected.json index d979bbaf0..038dd0b22 100644 --- a/packages/schema-generator/test/fixtures/schema/recipe-with-types-output.expected.json +++ b/packages/schema-generator/test/fixtures/schema/recipe-with-types-output.expected.json @@ -1,33 +1,62 @@ { - "type": "object", + "definitions": { + "Item": { + "properties": { + "text": { + "default": "", + "type": "string" + } + }, + "required": [ + "text" + ], + "type": "object" + }, + "OutputSchemaInterface": { + "properties": { + "items": { + "default": [], + "items": { + "$ref": "#/definitions/Item" + }, + "type": "array" + }, + "items_count": { + "type": "number" + }, + "title": { + "default": "untitled", + "type": "string" + } + }, + "required": [ + "items", + "items_count", + "title" + ], + "type": "object" + } + }, "properties": { + "items": { + "default": [], + "items": { + "$ref": "#/definitions/Item" + }, + "type": "array" + }, "items_count": { "type": "number" }, "title": { - "type": "string", - "default": "untitled" - }, - "items": { - "type": "array", - "items": { - "type": "object", - "properties": { - "text": { - "type": "string", - "default": "" - } - }, - "required": [ - "text" - ] - }, - "default": [] + "default": "untitled", + "type": "string" } }, "required": [ "items", "items_count", "title" - ] + ], + "type": "object" } diff --git a/packages/schema-generator/test/fixtures/schema/recursive-type-nested.expected.json b/packages/schema-generator/test/fixtures/schema/recursive-type-nested.expected.json index c3d6afcd8..070358624 100644 --- a/packages/schema-generator/test/fixtures/schema/recursive-type-nested.expected.json +++ b/packages/schema-generator/test/fixtures/schema/recursive-type-nested.expected.json @@ -13,6 +13,17 @@ "value" ], "type": "object" + }, + "RootType": { + "properties": { + "list": { + "$ref": "#/definitions/LinkedList" + } + }, + "required": [ + "list" + ], + "type": "object" } }, "properties": { diff --git a/packages/schema-generator/test/fixtures/schema/simple-interface.expected.json b/packages/schema-generator/test/fixtures/schema/simple-interface.expected.json index d0d5e8522..4dde458d6 100644 --- a/packages/schema-generator/test/fixtures/schema/simple-interface.expected.json +++ b/packages/schema-generator/test/fixtures/schema/simple-interface.expected.json @@ -8,8 +8,8 @@ } }, "required": [ - "name", - "age" + "age", + "name" ], "type": "object" } diff --git a/packages/schema-generator/test/schema/complex-defaults.test.ts b/packages/schema-generator/test/schema/complex-defaults.test.ts index 39fc05e92..7417ce336 100644 --- a/packages/schema-generator/test/schema/complex-defaults.test.ts +++ b/packages/schema-generator/test/schema/complex-defaults.test.ts @@ -22,8 +22,18 @@ describe("Schema: Complex defaults", () => { const empty = s.properties?.emptyItems as any; expect(empty.type).toBe("array"); - expect(empty.items?.type).toBe("object"); - expect(empty.items?.required).toEqual(["title", "done"]); + const emptyItems = empty.items as any; + if (emptyItems.$ref) { + expect(emptyItems.$ref).toBe("#/definitions/TodoItem"); + const def = (s as any).definitions?.TodoItem as any; + expect(def.type).toBe("object"); + expect(def.properties?.title?.type).toBe("string"); + expect(def.properties?.done?.type).toBe("boolean"); + expect(def.required).toEqual(["title", "done"]); + } else { + expect(emptyItems.type).toBe("object"); + expect(emptyItems.required).toEqual(["title", "done"]); + } expect(Array.isArray(empty.default)).toBe(true); const pre = s.properties?.prefilledItems as any; diff --git a/packages/schema-generator/test/schema/type-aliases-and-shared.test.ts b/packages/schema-generator/test/schema/type-aliases-and-shared.test.ts index 037741273..b1631b2a9 100644 --- a/packages/schema-generator/test/schema/type-aliases-and-shared.test.ts +++ b/packages/schema-generator/test/schema/type-aliases-and-shared.test.ts @@ -50,7 +50,7 @@ describe("Schema: Type aliases and shared types", () => { expect(na.items?.items?.type).toBe("string"); }); - it("duplicates shared object type structure where referenced twice", async () => { + it("hoists shared object type to definitions and references via $ref", async () => { const code = ` interface B { value: string; } interface A { b1: B; b2: B; } @@ -59,10 +59,11 @@ describe("Schema: Type aliases and shared types", () => { const s = createSchemaTransformerV2()(type, checker); const b1 = s.properties?.b1 as any; const b2 = s.properties?.b2 as any; - for (const bx of [b1, b2]) { - expect(bx.type).toBe("object"); - expect(bx.properties?.value?.type).toBe("string"); - expect(bx.required).toContain("value"); - } + expect(b1.$ref).toBe("#/definitions/B"); + expect(b2.$ref).toBe("#/definitions/B"); + const defB = (s as any).definitions?.B as any; + expect(defB.type).toBe("object"); + expect(defB.properties?.value?.type).toBe("string"); + expect(defB.required).toContain("value"); }); }); diff --git a/packages/schema-generator/test/schema/type-to-schema.test.ts b/packages/schema-generator/test/schema/type-to-schema.test.ts index a28d195fe..5e9054559 100644 --- a/packages/schema-generator/test/schema/type-to-schema.test.ts +++ b/packages/schema-generator/test/schema/type-to-schema.test.ts @@ -34,7 +34,12 @@ describe("Schema: type-to-schema parity", () => { // RecipeOutput expect(o.properties?.values?.type).toBe("array"); expect(o.properties?.values?.items?.type).toBe("string"); - expect(o.properties?.updater?.type).toBe("object"); - expect(o.properties?.updater?.asStream).toBe(true); + const upd = o.properties?.updater as any; + expect(upd.asStream).toBe(true); + expect(upd.$ref).toBe("#/definitions/UpdaterInput"); + const defU = (o as any).definitions?.UpdaterInput as any; + expect(defU.type).toBe("object"); + expect(defU.properties?.newValues?.type).toBe("array"); + expect(defU.properties?.newValues?.items?.type).toBe("string"); }); }); From 8d34b600db95bf076d370445dd63bf7d2fc5b51a Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 11 Sep 2025 13:38:36 -0700 Subject: [PATCH 02/16] format --- packages/schema-generator/README.md | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/schema-generator/README.md b/packages/schema-generator/README.md index 1f2e31411..a699fad25 100644 --- a/packages/schema-generator/README.md +++ b/packages/schema-generator/README.md @@ -25,4 +25,3 @@ Implementation: see `src/schema-generator.ts` (`formatType`) and - Check typings: `deno task check` - Run tests: `deno task test` - From f3eb1cccc99dd241cc6c29cb0888f157d16340c9 Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 11 Sep 2025 13:50:33 -0700 Subject: [PATCH 03/16] doc with both short- and long-term strategies for finishing this implementation --- .../promote-all-named-options.txt | 98 +++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 packages/schema-generator/promote-all-named-options.txt diff --git a/packages/schema-generator/promote-all-named-options.txt b/packages/schema-generator/promote-all-named-options.txt new file mode 100644 index 000000000..f5a3c9b30 --- /dev/null +++ b/packages/schema-generator/promote-all-named-options.txt @@ -0,0 +1,98 @@ +Title: Options for making hoisted named-type $ref schemas resolvable + +Context + We changed the generator to an "all-named" policy: Every named type (except + wrappers/containers like Array, ReadonlyArray, Cell, Stream, Default, Date) + is hoisted into `definitions`, and non-root uses emit `$ref` to that entry. + +Problem observed in CT builder (chatbot-outliner) + The builder sometimes operates on schema fragments (e.g., after merges or + link steps). Those fragments can now contain `$ref` without a local + `definitions`, leading to unresolved `$ref` at runtime. + +Goal + Keep the all-named behavior (for readability/reuse), while ensuring + `$ref`-bearing fragments remain resolvable in the CT runner/builder. + +Option A — Embed subset `definitions` into non-root fragments (short-term) + Idea: + - When the generator returns a non-root schema that contains `$ref`, attach + the minimal subset of `definitions` required by that fragment (including + transitive dependencies). + - This makes fragments self-contained and resolvable, regardless of how the + builder slices schemas later. + Changes required: + - Generator: After formatting a non-root schema, detect referenced names, + compute a transitive closure over `definitions`, and attach that subset + under `definitions` on the fragment. + - Tests/fixtures: Update expectations for places where nested fragments are + asserted to include a small `definitions` block. + Pros: + - Unblocks current use-cases without touching the CT builder. + - Keeps the all-named policy intact. + Cons: + - Slight duplication/bloat in nested fragments. + - Requires fixture updates and careful subset computation. + +Option B — Inline in fragments, hoist only at root (hybrid) + Idea: + - Keep all-named hoisting at the root level, but when returning schemas for + nested positions (likely to be lifted out by the builder), inline named + types (no `$ref`). + Changes required: + - Generator: Introduce a policy toggle or heuristics to inline named types + when formatting non-root schemas that are likely to be used in isolation. + - Tests/fixtures: Update to expect inlined shapes in fragments. + Pros: + - Fragments remain self-contained; fewer nested `definitions`. + Cons: + - Hybrid policy is more complex and less predictable for readers. + - Loses some reuse within fragments. + +Option C — Patch CT runner to resolve `$ref` from parent/root (long-term) + Idea: + - In the CT builder/runner, when operating on a fragment that contains a + `$ref` without local `definitions`, resolve against the nearest parent + schema that carries `definitions` (e.g., the component's input/output + root). Alternatively, thread a shared `definitions` map through the + joinSchema/resolveRef code paths. + Changes required: + - packages/runner: ContextualFlowControl.resolveSchemaRefOrThrow and + related joinSchema helpers need to accept an optional "definitions + provider" or resolve upward to find a root `definitions` map. + - Tests: Add builder-level tests where fragments contain `$ref` and validate + successful resolution from the parent context. + Pros: + - Clean architecture; avoids duplicating `definitions` into fragments. + - Keeps all-named behavior consistent and DRY. + Cons: + - Requires a multi-package change (runner + tests); coordination needed. + +Recommended sequence + 1) Implement Option A now to unblock current work: + - Add a helper: collectReferencedDefinitions(fragment, fullDefs) → subset + map of needed definitions (including transitive deps). + - Attach subset `definitions` for non-root results that contain `$ref`. + - Update fixtures to account for nested `definitions` where applicable. + + 2) Plan Option C as a follow-up: + - Add a "definitions context" resolver in the CT runner so fragments with + `$ref` can be resolved against the root schema. This preserves compact + fragments and centralizes definition storage. + - Once runner support lands, Option A's nested `definitions` can be + simplified or removed (guards remain for extreme edge cases). + +Implementation notes for Option A + - Definition subset: + • Start with the set of `$ref` names found in the fragment (scan keys and + nested values). For each referenced name N, include `definitions[N]` and + recursively scan that definition for further `$ref`s. + • Use a visited set to avoid cycles. + - Attachment point: + • Only attach `definitions` on non-root returns from the generator (root is + handled by buildFinalSchema which already includes full `definitions`). + • Keep the subset minimal; do not attach the entire `definitions` map. + - Tests: + • Where unit tests assert nested fragments, expect a small `definitions` + block, and assert that required definitions exist and shape is correct. + From 64a51a6038b9d5e5218ec5fc22cdea99dbcd22b1 Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 11 Sep 2025 16:08:04 -0700 Subject: [PATCH 04/16] progress towards working implementation, still WIP --- ...array-cell-remove-intersection.expected.ts | 44 ++++-- .../date-and-map-types.expected.ts | 120 +++++---------- .../jsx-complex-mixed.expected.tsx | 62 +++++--- .../jsx-property-access.expected.tsx | 71 ++++++--- .../opaque-ref-map.expected.ts | 30 ++-- .../recipe-with-types.expected.tsx | 110 ++++++++++---- packages/runner/src/builder/node-utils.ts | 5 +- .../src/formatters/intersection-formatter.ts | 139 +++++++++-------- .../schema-generator/src/schema-generator.ts | 142 ++++++++++++++---- packages/schema-generator/src/type-utils.ts | 22 ++- 10 files changed, 469 insertions(+), 276 deletions(-) diff --git a/packages/js-runtime/test/fixtures/handler-schema/array-cell-remove-intersection.expected.ts b/packages/js-runtime/test/fixtures/handler-schema/array-cell-remove-intersection.expected.ts index 020db9e29..8ad618361 100644 --- a/packages/js-runtime/test/fixtures/handler-schema/array-cell-remove-intersection.expected.ts +++ b/packages/js-runtime/test/fixtures/handler-schema/array-cell-remove-intersection.expected.ts @@ -7,18 +7,13 @@ interface ListState { items: Cell; } const removeItem = handler({} as const satisfies JSONSchema, { + $schema: "https://json-schema.org/draft-07/schema#", type: "object", properties: { items: { type: "array", items: { - type: "object", - properties: { - text: { - type: "string" - } - }, - required: ["text"] + $ref: "#/definitions/Item" }, asCell: true }, @@ -26,7 +21,18 @@ const removeItem = handler({} as const satisfies JSONSchema, { type: "number" } }, - required: ["items", "index"] + required: ["items", "index"], + definitions: { + Item: { + type: "object", + properties: { + text: { + type: "string" + } + }, + required: ["text"] + } + } } as const satisfies JSONSchema, (_, { items, index }) => { const next = items.get().slice(); if (index >= 0 && index < next.length) @@ -38,18 +44,13 @@ type ListStateWithIndex = ListState & { index: number; }; const removeItemAlias = handler({} as const satisfies JSONSchema, { + $schema: "https://json-schema.org/draft-07/schema#", type: "object", properties: { items: { type: "array", items: { - type: "object", - properties: { - text: { - type: "string" - } - }, - required: ["text"] + $ref: "#/definitions/Item" }, asCell: true }, @@ -57,7 +58,18 @@ const removeItemAlias = handler({} as const satisfies JSONSchema, { type: "number" } }, - required: ["items", "index"] + required: ["items", "index"], + definitions: { + Item: { + type: "object", + properties: { + text: { + type: "string" + } + }, + required: ["text"] + } + } } as const satisfies JSONSchema, (_, { items, index }) => { const next = items.get().slice(); if (index >= 0 && index < next.length) diff --git a/packages/js-runtime/test/fixtures/handler-schema/date-and-map-types.expected.ts b/packages/js-runtime/test/fixtures/handler-schema/date-and-map-types.expected.ts index 41a908720..f435de500 100644 --- a/packages/js-runtime/test/fixtures/handler-schema/date-and-map-types.expected.ts +++ b/packages/js-runtime/test/fixtures/handler-schema/date-and-map-types.expected.ts @@ -9,6 +9,7 @@ interface TimedState { history: Map; } const timedHandler = handler({ + $schema: "https://json-schema.org/draft-07/schema#", type: "object", properties: { timestamp: { @@ -16,53 +17,30 @@ const timedHandler = handler({ format: "date-time" }, data: { + $ref: "#/definitions/Map" + } + }, + required: ["timestamp", "data"], + definitions: { + Map: { type: "object", properties: { - clear: { - type: "object", - properties: {} - }, - delete: { - type: "object", - properties: {} - }, - forEach: { - type: "object", - properties: {} - }, - get: { - type: "object", - properties: {} - }, - has: { - type: "object", - properties: {} - }, - set: { - type: "object", - properties: {} - }, - size: { - type: "number" - }, - entries: { - type: "object", - properties: {} - }, - keys: { - type: "object", - properties: {} - }, - values: { - type: "object", - properties: {} - } + clear: { type: "object", properties: {} }, + delete: { type: "object", properties: {} }, + forEach: { type: "object", properties: {} }, + get: { type: "object", properties: {} }, + has: { type: "object", properties: {} }, + set: { type: "object", properties: {} }, + size: { type: "number" }, + entries: { type: "object", properties: {} }, + keys: { type: "object", properties: {} }, + values: { type: "object", properties: {} } }, required: ["clear", "delete", "forEach", "get", "has", "set", "size", "entries", "keys", "values"] } - }, - required: ["timestamp", "data"] + } } as const satisfies JSONSchema, { + $schema: "https://json-schema.org/draft-07/schema#", type: "object", properties: { lastUpdate: { @@ -70,56 +48,32 @@ const timedHandler = handler({ format: "date-time" }, history: { + $ref: "#/definitions/Map" + } + }, + required: ["lastUpdate", "history"], + definitions: { + Map: { type: "object", properties: { - clear: { - type: "object", - properties: {} - }, - delete: { - type: "object", - properties: {} - }, - forEach: { - type: "object", - properties: {} - }, - get: { - type: "object", - properties: {} - }, - has: { - type: "object", - properties: {} - }, - set: { - type: "object", - properties: {} - }, - size: { - type: "number" - }, - entries: { - type: "object", - properties: {} - }, - keys: { - type: "object", - properties: {} - }, - values: { - type: "object", - properties: {} - } + clear: { type: "object", properties: {} }, + delete: { type: "object", properties: {} }, + forEach: { type: "object", properties: {} }, + get: { type: "object", properties: {} }, + has: { type: "object", properties: {} }, + set: { type: "object", properties: {} }, + size: { type: "number" }, + entries: { type: "object", properties: {} }, + keys: { type: "object", properties: {} }, + values: { type: "object", properties: {} } }, required: ["clear", "delete", "forEach", "get", "has", "set", "size", "entries", "keys", "values"] } - }, - required: ["lastUpdate", "history"] + } } as const satisfies JSONSchema, (event, state) => { state.lastUpdate = event.timestamp; event.data.forEach((value, key) => { state.history.set(key, new Date()); }); }); -export { timedHandler }; \ No newline at end of file +export { timedHandler }; diff --git a/packages/js-runtime/test/fixtures/jsx-expressions/jsx-complex-mixed.expected.tsx b/packages/js-runtime/test/fixtures/jsx-expressions/jsx-complex-mixed.expected.tsx index 76c080980..1c8501801 100644 --- a/packages/js-runtime/test/fixtures/jsx-expressions/jsx-complex-mixed.expected.tsx +++ b/packages/js-runtime/test/fixtures/jsx-expressions/jsx-complex-mixed.expected.tsx @@ -13,27 +13,13 @@ interface State { taxRate: number; } export default recipe({ + $schema: "https://json-schema.org/draft-07/schema#", type: "object", properties: { items: { type: "array", items: { - type: "object", - properties: { - id: { - type: "number" - }, - name: { - type: "string" - }, - price: { - type: "number" - }, - active: { - type: "boolean" - } - }, - required: ["id", "name", "price", "active"] + $ref: "#/definitions/Item" } }, filter: { @@ -46,7 +32,48 @@ export default recipe({ type: "number" } }, - required: ["items", "filter", "discount", "taxRate"] + required: ["items", "filter", "discount", "taxRate"], + definitions: { + Item: { + type: "object", + properties: { + id: { + type: "number" + }, + name: { + type: "string" + }, + price: { + type: "number" + }, + active: { + type: "boolean" + } + }, + required: ["id", "name", "price", "active"] + }, + State: { + type: "object", + properties: { + items: { + type: "array", + items: { + $ref: "#/definitions/Item" + } + }, + filter: { + type: "string" + }, + discount: { + type: "number" + }, + taxRate: { + type: "number" + } + }, + required: ["items", "filter", "discount", "taxRate"] + } + } } as const satisfies JSONSchema, (state) => { return { [UI]: (
@@ -84,4 +111,3 @@ export default recipe({
), }; }); - diff --git a/packages/js-runtime/test/fixtures/jsx-expressions/jsx-property-access.expected.tsx b/packages/js-runtime/test/fixtures/jsx-expressions/jsx-property-access.expected.tsx index 9d6344f6d..672c8a20a 100644 --- a/packages/js-runtime/test/fixtures/jsx-expressions/jsx-property-access.expected.tsx +++ b/packages/js-runtime/test/fixtures/jsx-expressions/jsx-property-access.expected.tsx @@ -32,9 +32,34 @@ interface State { numbers: number[]; } export default recipe({ + $schema: "https://json-schema.org/draft-07/schema#", type: "object", properties: { user: { + $ref: "#/definitions/User" + }, + config: { + $ref: "#/definitions/Config" + }, + items: { + type: "array", + items: { + type: "string" + } + }, + index: { + type: "number" + }, + numbers: { + type: "array", + items: { + type: "number" + } + } + }, + required: ["user", "config", "items", "index", "numbers"], + definitions: { + User: { type: "object", properties: { name: { @@ -73,7 +98,7 @@ export default recipe({ }, required: ["name", "age", "active", "profile"] }, - config: { + Config: { type: "object", properties: { theme: { @@ -106,23 +131,34 @@ export default recipe({ }, required: ["theme", "features"] }, - items: { - type: "array", - items: { - type: "string" - } - }, - index: { - type: "number" - }, - numbers: { - type: "array", - items: { - type: "number" - } + State: { + type: "object", + properties: { + user: { + $ref: "#/definitions/User" + }, + config: { + $ref: "#/definitions/Config" + }, + items: { + type: "array", + items: { + type: "string" + } + }, + index: { + type: "number" + }, + numbers: { + type: "array", + items: { + type: "number" + } + } + }, + required: ["user", "config", "items", "index", "numbers"] } - }, - required: ["user", "config", "items", "index", "numbers"] + } } as const satisfies JSONSchema, (state) => { return { [UI]: (
@@ -170,4 +206,3 @@ export default recipe({
), }; }); - diff --git a/packages/js-runtime/test/fixtures/schema-transform/opaque-ref-map.expected.ts b/packages/js-runtime/test/fixtures/schema-transform/opaque-ref-map.expected.ts index 880092ff6..861461698 100644 --- a/packages/js-runtime/test/fixtures/schema-transform/opaque-ref-map.expected.ts +++ b/packages/js-runtime/test/fixtures/schema-transform/opaque-ref-map.expected.ts @@ -4,25 +4,31 @@ interface TodoItem { done: boolean; } export default recipe({ + $schema: "https://json-schema.org/draft-07/schema#", type: "object", properties: { items: { type: "array", items: { - type: "object", - properties: { - title: { - type: "string" - }, - done: { - type: "boolean" - } - }, - required: ["title", "done"] + $ref: "#/definitions/TodoItem" } } }, - required: ["items"] + required: ["items"], + definitions: { + TodoItem: { + type: "object", + properties: { + title: { + type: "string" + }, + done: { + type: "boolean" + } + }, + required: ["title", "done"] + } + } } as const satisfies JSONSchema, ({ items }) => { // This should NOT be transformed to items.get().map() // because OpaqueRef has its own map method @@ -34,4 +40,4 @@ export default recipe({ position: index })); return { mapped, filtered }; -}); \ No newline at end of file +}); diff --git a/packages/js-runtime/test/fixtures/schema-transform/recipe-with-types.expected.tsx b/packages/js-runtime/test/fixtures/schema-transform/recipe-with-types.expected.tsx index e16cec786..fe9646d64 100644 --- a/packages/js-runtime/test/fixtures/schema-transform/recipe-with-types.expected.tsx +++ b/packages/js-runtime/test/fixtures/schema-transform/recipe-with-types.expected.tsx @@ -17,6 +17,7 @@ type InputEventType = { }; }; const inputSchema = { + $schema: "https://json-schema.org/draft-07/schema#", type: "object", properties: { title: { @@ -26,21 +27,44 @@ const inputSchema = { items: { type: "array", items: { - type: "object", - properties: { - text: { - type: "string", - default: "" - } - }, - required: ["text"] + $ref: "#/definitions/Item" }, default: [] } }, - required: ["title", "items"] + required: ["title", "items"], + definitions: { + Item: { + type: "object", + properties: { + text: { + type: "string", + default: "" + } + }, + required: ["text"] + }, + InputSchemaInterface: { + type: "object", + properties: { + title: { + type: "string", + default: "untitled" + }, + items: { + type: "array", + items: { + $ref: "#/definitions/Item" + }, + default: [] + } + }, + required: ["title", "items"] + } + } } as const satisfies JSONSchema; const outputSchema = { + $schema: "https://json-schema.org/draft-07/schema#", type: "object", properties: { items_count: { @@ -53,19 +77,44 @@ const outputSchema = { items: { type: "array", items: { - type: "object", - properties: { - text: { - type: "string", - default: "" - } - }, - required: ["text"] + $ref: "#/definitions/Item" }, default: [] } }, - required: ["items_count", "title", "items"] + required: ["items_count", "title", "items"], + definitions: { + Item: { + type: "object", + properties: { + text: { + type: "string", + default: "" + } + }, + required: ["text"] + }, + OutputSchemaInterface: { + type: "object", + properties: { + items_count: { + type: "number" + }, + title: { + type: "string", + default: "untitled" + }, + items: { + type: "array", + items: { + $ref: "#/definitions/Item" + }, + default: [] + } + }, + required: ["items_count", "title", "items"] + } + } } as const satisfies JSONSchema; // Handler that logs the message event const addItem = handler({ @@ -83,24 +132,30 @@ const addItem = handler({ }, required: ["detail"] } as const satisfies JSONSchema, { + $schema: "https://json-schema.org/draft-07/schema#", type: "object", properties: { items: { type: "array", items: { - type: "object", - properties: { - text: { - type: "string", - default: "" - } - }, - required: ["text"] + $ref: "#/definitions/Item" }, asCell: true } }, - required: ["items"] + required: ["items"], + definitions: { + Item: { + type: "object", + properties: { + text: { + type: "string", + default: "" + } + }, + required: ["text"] + } + } } as const satisfies JSONSchema, (event: InputEventType, { items }: { items: Cell; }) => { @@ -124,4 +179,3 @@ export default recipe(inputSchema, outputSchema, ({ title, items }) => { items_count }; }); - diff --git a/packages/runner/src/builder/node-utils.ts b/packages/runner/src/builder/node-utils.ts index 86b5d765a..b9fef7b8a 100644 --- a/packages/runner/src/builder/node-utils.ts +++ b/packages/runner/src/builder/node-utils.ts @@ -56,12 +56,13 @@ export function applyInputIfcToOutput( const cfc = new ContextualFlowControl(); traverseValue(inputs, (item: unknown) => { if (isOpaqueRef(item)) { - const { schema: inputSchema } = (item as OpaqueRef).export(); + const { schema: inputSchema, rootSchema } = (item as OpaqueRef) + .export(); if (inputSchema !== undefined) { ContextualFlowControl.joinSchema( collectedClassifications, inputSchema, - inputSchema, + rootSchema ?? inputSchema, ); } } diff --git a/packages/schema-generator/src/formatters/intersection-formatter.ts b/packages/schema-generator/src/formatters/intersection-formatter.ts index 2a7f40999..7e37a5af4 100644 --- a/packages/schema-generator/src/formatters/intersection-formatter.ts +++ b/packages/schema-generator/src/formatters/intersection-formatter.ts @@ -10,11 +10,13 @@ import { isRecord } from "@commontools/utils/types"; import { extractDocFromType } from "../doc-utils.ts"; const logger = getLogger("schema-generator.intersection"); +const DOC_CONFLICT_COMMENT = + "Conflicting docs across intersection constituents; using first"; export class IntersectionFormatter implements TypeFormatter { constructor(private schemaGenerator: SchemaGenerator) {} - supportsType(type: ts.Type, context: GenerationContext): boolean { + supportsType(type: ts.Type, _context: GenerationContext): boolean { return (type.flags & ts.TypeFlags.Intersection) !== 0; } @@ -24,10 +26,11 @@ export class IntersectionFormatter implements TypeFormatter { const parts = inter.types ?? []; if (parts.length === 0) { - throw new Error("IntersectionFormatter received empty intersection type"); + throw new Error( + "IntersectionFormatter received empty intersection type", + ); } - // Validate constituents to ensure safe merging const failureReason = this.validateIntersectionParts(parts, checker); if (failureReason) { return { @@ -37,7 +40,6 @@ export class IntersectionFormatter implements TypeFormatter { }; } - // Merge object-like constituents: combine properties and required arrays const merged = this.mergeIntersectionParts(parts, context); return this.applyIntersectionDocs(merged); } @@ -47,13 +49,11 @@ export class IntersectionFormatter implements TypeFormatter { checker: ts.TypeChecker, ): string | null { for (const part of parts) { - // Only support object-like types for safe merging if ((part.flags & ts.TypeFlags.Object) === 0) { return "non-object constituent"; } try { - // Reject types with index signatures as they can't be safely merged const stringIndex = checker.getIndexTypeOfType( part, ts.IndexKind.String, @@ -65,15 +65,12 @@ export class IntersectionFormatter implements TypeFormatter { if (stringIndex || numberIndex) { return "index signature on constituent"; } - - // Note: Call/construct signatures are ignored (consistent with other formatters) - // They cannot be represented in JSON Schema, so we just extract regular properties } catch (error) { return `checker error while validating intersection: ${error}`; } } - return null; // All parts are valid + return null; } private mergeIntersectionParts( @@ -86,7 +83,7 @@ export class IntersectionFormatter implements TypeFormatter { missingSources: string[]; } { const mergedProps: Record = {}; - const requiredSet: Set = new Set(); + const requiredSet = new Set(); const docTexts: string[] = []; const documentedSources: string[] = []; @@ -101,56 +98,44 @@ export class IntersectionFormatter implements TypeFormatter { missingSources.push(docInfo.typeName); } - const schema = this.schemaGenerator.generateSchema( - part, - context.typeChecker, - undefined, // No specific type node for intersection parts - ); - - if (this.isObjectSchema(schema)) { - // Merge properties from this part - if (schema.properties) { - for (const [key, value] of Object.entries(schema.properties)) { - const existing = mergedProps[key]; - if (existing) { - // If both are object schemas, check description conflicts - if (isRecord(existing) && isRecord(value)) { - const aDesc = typeof existing.description === "string" - ? (existing.description as string) - : undefined; - const bDesc = typeof value.description === "string" - ? (value.description as string) + const schema = this.schemaGenerator.formatChildType(part, context); + const objSchema = this.resolveObjectSchema(schema, context); + if (!objSchema) continue; + + if (objSchema.properties) { + for (const [key, value] of Object.entries(objSchema.properties)) { + const existing = mergedProps[key]; + if (existing) { + if (isRecord(existing) && isRecord(value)) { + const aDesc = typeof existing.description === "string" + ? existing.description as string + : undefined; + const bDesc = typeof value.description === "string" + ? value.description as string + : undefined; + if (aDesc && bDesc && aDesc !== bDesc) { + const priorComment = typeof existing.$comment === "string" + ? existing.$comment as string : undefined; - if (aDesc && bDesc && aDesc !== bDesc) { - const priorComment = typeof existing.$comment === "string" - ? (existing.$comment as string) - : undefined; - (existing as Record).$comment = - priorComment ?? - "Conflicting docs across intersection constituents; using first"; - logger.warn(() => - `Intersection doc conflict for '${key}'; using first` - ); - } + (existing as Record).$comment = priorComment ?? + DOC_CONFLICT_COMMENT; + logger.warn(() => + `Intersection doc conflict for '${key}'; using first` + ); } - // Prefer the first definition by default; emit debug for visibility - logger.debug(() => - `Intersection kept first definition for '${key}'` - ); - // Keep existing - continue; } - mergedProps[key] = value as SchemaDefinition; + logger.debug(() => + `Intersection kept first definition for '${key}'` + ); + continue; } + mergedProps[key] = value as SchemaDefinition; } + } - // Merge required properties - if (Array.isArray(schema.required)) { - for (const req of schema.required) { - if (typeof req === "string") { - requiredSet.add(req); - } - } + if (Array.isArray(objSchema.required)) { + for (const req of objSchema.required) { + if (typeof req === "string") requiredSet.add(req); } } } @@ -180,6 +165,32 @@ export class IntersectionFormatter implements TypeFormatter { ); } + private resolveObjectSchema( + schema: SchemaDefinition, + context: GenerationContext, + ): + | (SchemaDefinition & { + properties?: Record; + required?: string[]; + }) + | undefined { + if (this.isObjectSchema(schema)) return schema; + if ( + typeof schema === "object" && + schema !== null && + typeof (schema as Record).$ref === "string" + ) { + const ref = (schema as Record).$ref as string; + const prefix = "#/definitions/"; + if (ref.startsWith(prefix)) { + const name = ref.slice(prefix.length); + const def = context.definitions[name]; + if (def && this.isObjectSchema(def)) return def; + } + } + return undefined; + } + private applyIntersectionDocs( data: { schema: SchemaDefinition; @@ -205,26 +216,24 @@ export class IntersectionFormatter implements TypeFormatter { const commentParts: string[] = []; const existingComment = typeof schema.$comment === "string" - ? (schema.$comment as string) + ? schema.$comment as string : undefined; const uniqueDocumented = Array.from(new Set(documentedSources)).filter(( - s, - ) => s); - const uniqueMissing = Array.from(new Set(missingSources)).filter((s) => s); + name, + ) => name); + const uniqueMissing = Array.from(new Set(missingSources)).filter((name) => + name + ); if (uniqueDocTexts.length > 0) { commentParts.push("Docs inherited from intersection constituents."); } if (uniqueDocTexts.length > 1 && uniqueDocumented.length > 0) { - commentParts.push( - `Sources: ${uniqueDocumented.join(", ")}.`, - ); + commentParts.push(`Sources: ${uniqueDocumented.join(", ")}.`); } if (uniqueDocTexts.length > 0 && uniqueMissing.length > 0) { - commentParts.push( - `Missing docs for: ${uniqueMissing.join(", ")}.`, - ); + commentParts.push(`Missing docs for: ${uniqueMissing.join(", ")}.`); } if (commentParts.length > 0) { diff --git a/packages/schema-generator/src/schema-generator.ts b/packages/schema-generator/src/schema-generator.ts index 5c03aa613..e4bb4dfaa 100644 --- a/packages/schema-generator/src/schema-generator.ts +++ b/packages/schema-generator/src/schema-generator.ts @@ -1,4 +1,6 @@ import ts from "typescript"; +import { isRecord } from "@commontools/utils/types"; + import type { GenerationContext, SchemaDefinition, @@ -17,11 +19,7 @@ import { safeGetIndexTypeOfType, safeGetTypeOfSymbolAtLocation, } from "./type-utils.ts"; -import { - extractDocFromSymbolAndDecls, - extractDocFromType, -} from "./doc-utils.ts"; -import { isRecord } from "@commontools/utils/types"; +import { extractDocFromType } from "./doc-utils.ts"; /** * Main schema generator that uses a chain of formatters @@ -90,21 +88,24 @@ export class SchemaGenerator implements ISchemaGenerator { } /** - * Create a stack key that distinguishes erased wrapper types from their inner types + * Create a stack key that distinguishes erased wrapper types from their + * inner types */ private createStackKey( type: ts.Type, typeNode?: ts.TypeNode, checker?: ts.TypeChecker, ): string | ts.Type { - // Handle Default types (both direct and aliased) with enhanced keys to avoid false cycles + // Handle Default types (both direct and aliased) with enhanced keys to + // avoid false cycles if (typeNode && ts.isTypeReferenceNode(typeNode)) { const isDirectDefault = ts.isIdentifier(typeNode.typeName) && typeNode.typeName.text === "Default"; const isAliasedDefault = checker && isDefaultTypeRef(typeNode, checker); if (isDirectDefault || isAliasedDefault) { - // Create a more specific key that includes type argument info to avoid false cycles + // Create a more specific key that includes type argument info to + // avoid false cycles const argTexts = typeNode.typeArguments ? typeNode.typeArguments.map((arg) => arg.getText()).join(",") : ""; @@ -156,7 +157,8 @@ export class SchemaGenerator implements ISchemaGenerator { context.emittedRefs.add(namedKey); return { "$ref": `#/definitions/${namedKey}` }; } - // Anonymous recursive type - can't create $ref, use permissive fallback to break cycle + // Anonymous recursive type - can't create $ref, use permissive fallback + // to break the cycle return { type: "object", additionalProperties: true, @@ -196,7 +198,8 @@ export class SchemaGenerator implements ISchemaGenerator { } } - // If no formatter supports this type, this is an error - we should have complete coverage + // If no formatter supports this type, this is an error - we should have + // complete coverage context.definitionStack.delete( this.createStackKey(type, context.typeNode, context.typeChecker), ); @@ -205,7 +208,8 @@ export class SchemaGenerator implements ISchemaGenerator { const typeFlags = type.flags; throw new Error( `No formatter found for type: ${typeName} (flags: ${typeFlags}). ` + - `This indicates incomplete formatter coverage - every TypeScript type should be handled by a formatter.`, + "This indicates incomplete formatter coverage - every TypeScript " + + "type should be handled by a formatter.", ); } @@ -225,40 +229,47 @@ export class SchemaGenerator implements ISchemaGenerator { return rootSchema; } - // Check if root schema should be promoted to a definition + // Decide if we promote root to a $ref const namedKey = getNamedTypeKey(type); const shouldPromoteRoot = this.shouldPromoteToRef(namedKey, context); + let base: SchemaDefinition; + if (shouldPromoteRoot && namedKey) { - // Add root schema to definitions if not already there + // Ensure root is present in definitions if (!definitions[namedKey]) { definitions[namedKey] = rootSchema; } - - // Return schema with $ref to root and definitions - return { - $schema: "https://json-schema.org/draft-07/schema#", - $ref: `#/definitions/${namedKey}`, - definitions, - }; + base = { $ref: `#/definitions/${namedKey}` } as SchemaDefinition; + } else { + base = rootSchema; } - // Return root schema with definitions - // Handle case where rootSchema might be boolean (per JSON Schema spec) - if (typeof rootSchema === "boolean") { - return rootSchema === false + // Handle boolean schemas (rare, but supported by JSON Schema) + if (typeof base === "boolean") { + const filtered = this.collectReferencedDefinitions(base, definitions); + if (Object.keys(filtered).length === 0) return base; + return base === false ? { $schema: "https://json-schema.org/draft-07/schema#", not: true, - definitions, + definitions: filtered, } - : { $schema: "https://json-schema.org/draft-07/schema#", definitions }; + : { + $schema: "https://json-schema.org/draft-07/schema#", + definitions: filtered, + }; } - return { + + // Object schema: attach only the definitions actually referenced by the + // final output + const filtered = this.collectReferencedDefinitions(base, definitions); + const out: Record = { $schema: "https://json-schema.org/draft-07/schema#", - ...rootSchema, - definitions, + ...(base as Record), }; + if (Object.keys(filtered).length > 0) out.definitions = filtered; + return out as SchemaDefinition; } /** @@ -272,7 +283,8 @@ export class SchemaGenerator implements ISchemaGenerator { const { definitions, emittedRefs } = context; - // If the root type already exists in definitions and has been referenced, promote it + // If the root type already exists in definitions and has been referenced, + // promote it return !!(definitions[namedKey] && emittedRefs.has(namedKey)); } @@ -353,9 +365,8 @@ export class SchemaGenerator implements ISchemaGenerator { } /** - * Attach a root-level description from JSDoc on the type symbol when - * available and when the root schema is an object that does not already have - * a description. + * Attach a root-level description from JSDoc when the root schema does not + * already supply one. */ private attachRootDescription( schema: SchemaDefinition, @@ -370,4 +381,69 @@ export class SchemaGenerator implements ISchemaGenerator { } return schema; } + + /** + * Recursively scan a schema fragment to collect referenced definition names + * and return the minimal subset of definitions required to resolve them, + * including transitive dependencies. + */ + private collectReferencedDefinitions( + fragment: SchemaDefinition, + allDefs: Record, + ): Record { + const needed = new Set(); + const visited = new Set(); + + const enqueueFromRef = (ref: string) => { + const prefix = "#/definitions/"; + if (typeof ref === "string" && ref.startsWith(prefix)) { + const name = ref.slice(prefix.length); + if (name) needed.add(name); + } + }; + + const scan = (node: unknown) => { + if (!node || typeof node !== "object") return; + if (Array.isArray(node)) { + for (const item of node) scan(item); + return; + } + const obj = node as Record; + for (const [k, v] of Object.entries(obj)) { + if (k === "$ref" && typeof v === "string") enqueueFromRef(v); + // Skip descending into existing definitions blocks to avoid pulling in + // already-attached subsets recursively + if (k === "definitions") continue; + scan(v); + } + }; + + // Find initial set of needed names from the fragment + scan(fragment); + + // Compute transitive closure by following refs inside included definitions + const stack: string[] = Array.from(needed); + while (stack.length > 0) { + const name = stack.pop()!; + if (visited.has(name)) continue; + visited.add(name); + const def = allDefs[name]; + if (!def) continue; + // Scan definition body for further refs + scan(def); + for (const n of Array.from(needed)) { + if (!visited.has(n)) { + // Only push newly discovered names + if (!stack.includes(n)) stack.push(n); + } + } + } + + // Build the subset map + const subset: Record = {}; + for (const name of visited) { + if (allDefs[name]) subset[name] = allDefs[name]; + } + return subset; + } } diff --git a/packages/schema-generator/src/type-utils.ts b/packages/schema-generator/src/type-utils.ts index 88a9eab58..180c88ad9 100644 --- a/packages/schema-generator/src/type-utils.ts +++ b/packages/schema-generator/src/type-utils.ts @@ -124,7 +124,8 @@ export function getNamedTypeKey( type: ts.Type, ): string | undefined { // Prefer direct symbol name; fall back to target symbol for TypeReference - let name = type.symbol?.name; + const symbol = type.symbol; + let name = symbol?.name; const objectFlags = (type as ts.ObjectType).objectFlags ?? 0; if (!name && (objectFlags & ts.ObjectFlags.Reference)) { const ref = type as unknown as ts.TypeReference; @@ -136,6 +137,25 @@ export function getNamedTypeKey( if (aliasName) name = aliasName; } if (!name || name === "__type") return undefined; + // Exclude property/method-like symbols (member names), which are not real named types + const symFlags = symbol?.flags ?? 0; + if ( + (symFlags & ts.SymbolFlags.Property) !== 0 || + (symFlags & ts.SymbolFlags.Method) !== 0 || + (symFlags & ts.SymbolFlags.Signature) !== 0 || + (symFlags & ts.SymbolFlags.Function) !== 0 + ) { + return undefined; + } + const decls = symbol?.declarations ?? []; + if ( + decls.some((d) => + ts.isPropertySignature(d) || ts.isMethodSignature(d) || + ts.isPropertyDeclaration(d) || ts.isMethodDeclaration(d) + ) + ) { + return undefined; + } // Avoid promoting wrappers/containers into definitions if (name === "Array" || name === "ReadonlyArray") return undefined; if (name === "Cell" || name === "Stream" || name === "Default") { From 85f2a18099dcaa657032fa926bd9d6262cb7618c Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 18 Sep 2025 12:37:29 -0700 Subject: [PATCH 05/16] update fixtures to no longer expect root types to always be promoted to named defs (only promote if referenced elsewhere) --- .../fixtures/schema/date-types.expected.json | 19 ------- .../schema/default-type.expected.json | 31 ----------- .../defaults-complex-array.expected.json | 46 ----------------- .../schema/descriptions-nested.expected.json | 51 ++++++++++--------- .../recipe-with-types-input.expected.json | 20 -------- .../recipe-with-types-output.expected.json | 24 --------- .../recursive-type-nested.expected.json | 11 ---- 7 files changed, 28 insertions(+), 174 deletions(-) diff --git a/packages/schema-generator/test/fixtures/schema/date-types.expected.json b/packages/schema-generator/test/fixtures/schema/date-types.expected.json index f787616ca..9f7c674f4 100644 --- a/packages/schema-generator/test/fixtures/schema/date-types.expected.json +++ b/packages/schema-generator/test/fixtures/schema/date-types.expected.json @@ -48,25 +48,6 @@ ], "type": "object" }, - "SchemaRoot": { - "properties": { - "document": { - "$ref": "#/definitions/Document" - }, - "eventLog": { - "$ref": "#/definitions/EventLog" - }, - "userProfile": { - "$ref": "#/definitions/UserProfile" - } - }, - "required": [ - "document", - "eventLog", - "userProfile" - ], - "type": "object" - }, "UserProfile": { "properties": { "createdAt": { diff --git a/packages/schema-generator/test/fixtures/schema/default-type.expected.json b/packages/schema-generator/test/fixtures/schema/default-type.expected.json index 0d9da597c..f453b3f99 100644 --- a/packages/schema-generator/test/fixtures/schema/default-type.expected.json +++ b/packages/schema-generator/test/fixtures/schema/default-type.expected.json @@ -176,37 +176,6 @@ ], "type": "object" }, - "SchemaRoot": { - "properties": { - "appConfig": { - "$ref": "#/definitions/AppConfig" - }, - "cellDefaults": { - "$ref": "#/definitions/CellDefaults" - }, - "complexDefault": { - "$ref": "#/definitions/ComplexDefault" - }, - "listConfig": { - "$ref": "#/definitions/ListConfig" - }, - "optionalDefaults": { - "$ref": "#/definitions/OptionalWithDefaults" - }, - "userSettings": { - "$ref": "#/definitions/UserSettings" - } - }, - "required": [ - "appConfig", - "cellDefaults", - "complexDefault", - "listConfig", - "optionalDefaults", - "userSettings" - ], - "type": "object" - }, "UserSettings": { "properties": { "fontSize": { diff --git a/packages/schema-generator/test/fixtures/schema/defaults-complex-array.expected.json b/packages/schema-generator/test/fixtures/schema/defaults-complex-array.expected.json index 69b0bcb63..7f1602d1d 100644 --- a/packages/schema-generator/test/fixtures/schema/defaults-complex-array.expected.json +++ b/packages/schema-generator/test/fixtures/schema/defaults-complex-array.expected.json @@ -1,51 +1,5 @@ { "definitions": { - "SchemaRoot": { - "properties": { - "emptyItems": { - "default": [], - "items": { - "$ref": "#/definitions/TodoItem" - }, - "type": "array" - }, - "matrix": { - "default": [ - [ - 1, - 2 - ], - [ - 3, - 4 - ] - ], - "items": { - "items": { - "type": "number" - }, - "type": "array" - }, - "type": "array" - }, - "prefilledItems": { - "default": [ - "item1", - "item2" - ], - "items": { - "type": "string" - }, - "type": "array" - } - }, - "required": [ - "emptyItems", - "matrix", - "prefilledItems" - ], - "type": "object" - }, "TodoItem": { "properties": { "done": { diff --git a/packages/schema-generator/test/fixtures/schema/descriptions-nested.expected.json b/packages/schema-generator/test/fixtures/schema/descriptions-nested.expected.json index fbb4a1410..7b08f1a32 100644 --- a/packages/schema-generator/test/fixtures/schema/descriptions-nested.expected.json +++ b/packages/schema-generator/test/fixtures/schema/descriptions-nested.expected.json @@ -1,4 +1,30 @@ { + "definitions": { + "ChildNode": { + "properties": { + "attachments": { + "description": "Attachments associated with the child node", + "items": true, + "type": "array" + }, + "body": { + "description": "The main text content of the child node", + "type": "string" + }, + "children": { + "description": "Children of the child node", + "items": true, + "type": "array" + } + }, + "required": [ + "attachments", + "body", + "children" + ], + "type": "object" + } + }, "description": "Outliner document", "properties": { "attachments": { @@ -13,28 +39,7 @@ "children": { "description": "Child nodes of this node", "items": { - "properties": { - "attachments": { - "description": "Attachments associated with the child node", - "items": true, - "type": "array" - }, - "body": { - "description": "The main text content of the child node", - "type": "string" - }, - "children": { - "description": "Children of the child node", - "items": true, - "type": "array" - } - }, - "required": [ - "body", - "children", - "attachments" - ], - "type": "object" + "$ref": "#/definitions/ChildNode" }, "type": "array" }, @@ -44,9 +49,9 @@ } }, "required": [ + "attachments", "body", "children", - "attachments", "version" ], "type": "object" diff --git a/packages/schema-generator/test/fixtures/schema/recipe-with-types-input.expected.json b/packages/schema-generator/test/fixtures/schema/recipe-with-types-input.expected.json index 0ea6c8dbd..7bd1b3a8c 100644 --- a/packages/schema-generator/test/fixtures/schema/recipe-with-types-input.expected.json +++ b/packages/schema-generator/test/fixtures/schema/recipe-with-types-input.expected.json @@ -1,25 +1,5 @@ { "definitions": { - "InputSchemaInterface": { - "properties": { - "items": { - "default": [], - "items": { - "$ref": "#/definitions/Item" - }, - "type": "array" - }, - "title": { - "default": "untitled", - "type": "string" - } - }, - "required": [ - "items", - "title" - ], - "type": "object" - }, "Item": { "properties": { "text": { diff --git a/packages/schema-generator/test/fixtures/schema/recipe-with-types-output.expected.json b/packages/schema-generator/test/fixtures/schema/recipe-with-types-output.expected.json index 038dd0b22..d9a2ce187 100644 --- a/packages/schema-generator/test/fixtures/schema/recipe-with-types-output.expected.json +++ b/packages/schema-generator/test/fixtures/schema/recipe-with-types-output.expected.json @@ -11,30 +11,6 @@ "text" ], "type": "object" - }, - "OutputSchemaInterface": { - "properties": { - "items": { - "default": [], - "items": { - "$ref": "#/definitions/Item" - }, - "type": "array" - }, - "items_count": { - "type": "number" - }, - "title": { - "default": "untitled", - "type": "string" - } - }, - "required": [ - "items", - "items_count", - "title" - ], - "type": "object" } }, "properties": { diff --git a/packages/schema-generator/test/fixtures/schema/recursive-type-nested.expected.json b/packages/schema-generator/test/fixtures/schema/recursive-type-nested.expected.json index 070358624..c3d6afcd8 100644 --- a/packages/schema-generator/test/fixtures/schema/recursive-type-nested.expected.json +++ b/packages/schema-generator/test/fixtures/schema/recursive-type-nested.expected.json @@ -13,17 +13,6 @@ "value" ], "type": "object" - }, - "RootType": { - "properties": { - "list": { - "$ref": "#/definitions/LinkedList" - } - }, - "required": [ - "list" - ], - "type": "object" } }, "properties": { From 3ea091eb3b246a6441ce7de7f123a12214267d51 Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 18 Sep 2025 12:41:48 -0700 Subject: [PATCH 06/16] and finish updating js-runtime fixtures' expectations too --- .../date-and-map-types.expected.ts | 98 +++++++++++++++---- .../jsx-complex-mixed.expected.tsx | 21 ---- .../jsx-property-access.expected.tsx | 89 ++++++----------- .../recipe-with-types.expected.tsx | 37 ------- 4 files changed, 109 insertions(+), 136 deletions(-) diff --git a/packages/js-runtime/test/fixtures/handler-schema/date-and-map-types.expected.ts b/packages/js-runtime/test/fixtures/handler-schema/date-and-map-types.expected.ts index f435de500..5494d024c 100644 --- a/packages/js-runtime/test/fixtures/handler-schema/date-and-map-types.expected.ts +++ b/packages/js-runtime/test/fixtures/handler-schema/date-and-map-types.expected.ts @@ -25,16 +25,45 @@ const timedHandler = handler({ Map: { type: "object", properties: { - clear: { type: "object", properties: {} }, - delete: { type: "object", properties: {} }, - forEach: { type: "object", properties: {} }, - get: { type: "object", properties: {} }, - has: { type: "object", properties: {} }, - set: { type: "object", properties: {} }, - size: { type: "number" }, - entries: { type: "object", properties: {} }, - keys: { type: "object", properties: {} }, - values: { type: "object", properties: {} } + clear: { + type: "object", + properties: {} + }, + delete: { + type: "object", + properties: {} + }, + forEach: { + type: "object", + properties: {} + }, + get: { + type: "object", + properties: {} + }, + has: { + type: "object", + properties: {} + }, + set: { + type: "object", + properties: {} + }, + size: { + type: "number" + }, + entries: { + type: "object", + properties: {} + }, + keys: { + type: "object", + properties: {} + }, + values: { + type: "object", + properties: {} + } }, required: ["clear", "delete", "forEach", "get", "has", "set", "size", "entries", "keys", "values"] } @@ -56,16 +85,45 @@ const timedHandler = handler({ Map: { type: "object", properties: { - clear: { type: "object", properties: {} }, - delete: { type: "object", properties: {} }, - forEach: { type: "object", properties: {} }, - get: { type: "object", properties: {} }, - has: { type: "object", properties: {} }, - set: { type: "object", properties: {} }, - size: { type: "number" }, - entries: { type: "object", properties: {} }, - keys: { type: "object", properties: {} }, - values: { type: "object", properties: {} } + clear: { + type: "object", + properties: {} + }, + delete: { + type: "object", + properties: {} + }, + forEach: { + type: "object", + properties: {} + }, + get: { + type: "object", + properties: {} + }, + has: { + type: "object", + properties: {} + }, + set: { + type: "object", + properties: {} + }, + size: { + type: "number" + }, + entries: { + type: "object", + properties: {} + }, + keys: { + type: "object", + properties: {} + }, + values: { + type: "object", + properties: {} + } }, required: ["clear", "delete", "forEach", "get", "has", "set", "size", "entries", "keys", "values"] } diff --git a/packages/js-runtime/test/fixtures/jsx-expressions/jsx-complex-mixed.expected.tsx b/packages/js-runtime/test/fixtures/jsx-expressions/jsx-complex-mixed.expected.tsx index 1c8501801..48f2ca530 100644 --- a/packages/js-runtime/test/fixtures/jsx-expressions/jsx-complex-mixed.expected.tsx +++ b/packages/js-runtime/test/fixtures/jsx-expressions/jsx-complex-mixed.expected.tsx @@ -51,27 +51,6 @@ export default recipe({ } }, required: ["id", "name", "price", "active"] - }, - State: { - type: "object", - properties: { - items: { - type: "array", - items: { - $ref: "#/definitions/Item" - } - }, - filter: { - type: "string" - }, - discount: { - type: "number" - }, - taxRate: { - type: "number" - } - }, - required: ["items", "filter", "discount", "taxRate"] } } } as const satisfies JSONSchema, (state) => { diff --git a/packages/js-runtime/test/fixtures/jsx-expressions/jsx-property-access.expected.tsx b/packages/js-runtime/test/fixtures/jsx-expressions/jsx-property-access.expected.tsx index 672c8a20a..18a056eee 100644 --- a/packages/js-runtime/test/fixtures/jsx-expressions/jsx-property-access.expected.tsx +++ b/packages/js-runtime/test/fixtures/jsx-expressions/jsx-property-access.expected.tsx @@ -59,45 +59,6 @@ export default recipe({ }, required: ["user", "config", "items", "index", "numbers"], definitions: { - User: { - type: "object", - properties: { - name: { - type: "string" - }, - age: { - type: "number" - }, - active: { - type: "boolean" - }, - profile: { - type: "object", - properties: { - bio: { - type: "string" - }, - location: { - type: "string" - }, - settings: { - type: "object", - properties: { - theme: { - type: "string" - }, - notifications: { - type: "boolean" - } - }, - required: ["theme", "notifications"] - } - }, - required: ["bio", "location", "settings"] - } - }, - required: ["name", "age", "active", "profile"] - }, Config: { type: "object", properties: { @@ -131,32 +92,44 @@ export default recipe({ }, required: ["theme", "features"] }, - State: { + User: { type: "object", properties: { - user: { - $ref: "#/definitions/User" - }, - config: { - $ref: "#/definitions/Config" - }, - items: { - type: "array", - items: { - type: "string" - } + name: { + type: "string" }, - index: { + age: { type: "number" }, - numbers: { - type: "array", - items: { - type: "number" - } + active: { + type: "boolean" + }, + profile: { + type: "object", + properties: { + bio: { + type: "string" + }, + location: { + type: "string" + }, + settings: { + type: "object", + properties: { + theme: { + type: "string" + }, + notifications: { + type: "boolean" + } + }, + required: ["theme", "notifications"] + } + }, + required: ["bio", "location", "settings"] } }, - required: ["user", "config", "items", "index", "numbers"] + required: ["name", "age", "active", "profile"] } } } as const satisfies JSONSchema, (state) => { diff --git a/packages/js-runtime/test/fixtures/schema-transform/recipe-with-types.expected.tsx b/packages/js-runtime/test/fixtures/schema-transform/recipe-with-types.expected.tsx index fe9646d64..45a3a017f 100644 --- a/packages/js-runtime/test/fixtures/schema-transform/recipe-with-types.expected.tsx +++ b/packages/js-runtime/test/fixtures/schema-transform/recipe-with-types.expected.tsx @@ -43,23 +43,6 @@ const inputSchema = { } }, required: ["text"] - }, - InputSchemaInterface: { - type: "object", - properties: { - title: { - type: "string", - default: "untitled" - }, - items: { - type: "array", - items: { - $ref: "#/definitions/Item" - }, - default: [] - } - }, - required: ["title", "items"] } } } as const satisfies JSONSchema; @@ -93,26 +76,6 @@ const outputSchema = { } }, required: ["text"] - }, - OutputSchemaInterface: { - type: "object", - properties: { - items_count: { - type: "number" - }, - title: { - type: "string", - default: "untitled" - }, - items: { - type: "array", - items: { - $ref: "#/definitions/Item" - }, - default: [] - } - }, - required: ["items_count", "title", "items"] } } } as const satisfies JSONSchema; From cf5ab9c37076ef3a1de6ef6d05c1bbfbfd44a132 Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 18 Sep 2025 12:55:13 -0700 Subject: [PATCH 07/16] don't include callables/function-like things in json schemas --- .../date-and-map-types.expected.ts | 76 +------------------ .../src/formatters/object-formatter.ts | 22 ++++-- packages/schema-generator/src/type-utils.ts | 22 ++++++ 3 files changed, 41 insertions(+), 79 deletions(-) diff --git a/packages/js-runtime/test/fixtures/handler-schema/date-and-map-types.expected.ts b/packages/js-runtime/test/fixtures/handler-schema/date-and-map-types.expected.ts index 5494d024c..e99cf9f8e 100644 --- a/packages/js-runtime/test/fixtures/handler-schema/date-and-map-types.expected.ts +++ b/packages/js-runtime/test/fixtures/handler-schema/date-and-map-types.expected.ts @@ -25,47 +25,11 @@ const timedHandler = handler({ Map: { type: "object", properties: { - clear: { - type: "object", - properties: {} - }, - delete: { - type: "object", - properties: {} - }, - forEach: { - type: "object", - properties: {} - }, - get: { - type: "object", - properties: {} - }, - has: { - type: "object", - properties: {} - }, - set: { - type: "object", - properties: {} - }, size: { type: "number" - }, - entries: { - type: "object", - properties: {} - }, - keys: { - type: "object", - properties: {} - }, - values: { - type: "object", - properties: {} } }, - required: ["clear", "delete", "forEach", "get", "has", "set", "size", "entries", "keys", "values"] + required: ["size"] } } } as const satisfies JSONSchema, { @@ -85,47 +49,11 @@ const timedHandler = handler({ Map: { type: "object", properties: { - clear: { - type: "object", - properties: {} - }, - delete: { - type: "object", - properties: {} - }, - forEach: { - type: "object", - properties: {} - }, - get: { - type: "object", - properties: {} - }, - has: { - type: "object", - properties: {} - }, - set: { - type: "object", - properties: {} - }, size: { type: "number" - }, - entries: { - type: "object", - properties: {} - }, - keys: { - type: "object", - properties: {} - }, - values: { - type: "object", - properties: {} } }, - required: ["clear", "delete", "forEach", "get", "has", "set", "size", "entries", "keys", "values"] + required: ["size"] } } } as const satisfies JSONSchema, (event, state) => { diff --git a/packages/schema-generator/src/formatters/object-formatter.ts b/packages/schema-generator/src/formatters/object-formatter.ts index 9c96a6b45..91c7927bd 100644 --- a/packages/schema-generator/src/formatters/object-formatter.ts +++ b/packages/schema-generator/src/formatters/object-formatter.ts @@ -4,7 +4,7 @@ import type { SchemaDefinition, TypeFormatter, } from "../interface.ts"; -import { safeGetPropertyType } from "../type-utils.ts"; +import { isFunctionLike, safeGetPropertyType } from "../type-utils.ts"; import type { SchemaGenerator } from "../schema-generator.ts"; import { extractDocFromSymbolAndDecls, getDeclDocs } from "../doc-utils.ts"; import { getLogger } from "@commontools/utils/logger"; @@ -66,13 +66,16 @@ export class ObjectFormatter implements TypeFormatter { const propName = prop.getName(); if (propName.startsWith("__")) continue; // Skip internal properties - const isOptional = (prop.flags & ts.SymbolFlags.Optional) !== 0; - if (!isOptional) required.push(propName); - let propTypeNode: ts.TypeNode | undefined; const propDecl = prop.valueDeclaration ?? - (prop.declarations?.[0] as ts.Declaration); + (prop.declarations?.[0] as ts.Declaration | undefined); + if (propDecl) { + if ( + ts.isMethodSignature(propDecl) || ts.isMethodDeclaration(propDecl) + ) { + continue; + } if ( ts.isPropertySignature(propDecl) || ts.isPropertyDeclaration(propDecl) ) { @@ -80,6 +83,8 @@ export class ObjectFormatter implements TypeFormatter { } } + if ((prop.flags & ts.SymbolFlags.Method) !== 0) continue; + // Get the actual property type and recursively delegate to the main schema generator const resolvedPropType = safeGetPropertyType( prop, @@ -88,6 +93,13 @@ export class ObjectFormatter implements TypeFormatter { propTypeNode, ); + if (isFunctionLike(resolvedPropType)) { + continue; + } + + const isOptional = (prop.flags & ts.SymbolFlags.Optional) !== 0; + if (!isOptional) required.push(propName); + // Delegate to the main generator (specific formatters handle wrappers/defaults) const generated: SchemaDefinition = this.schemaGenerator.formatChildType( resolvedPropType, diff --git a/packages/schema-generator/src/type-utils.ts b/packages/schema-generator/src/type-utils.ts index 180c88ad9..5e5cd77de 100644 --- a/packages/schema-generator/src/type-utils.ts +++ b/packages/schema-generator/src/type-utils.ts @@ -165,6 +165,28 @@ export function getNamedTypeKey( return name; } +/** + * Determine if a type represents a callable/constructable function value. + */ +export function isFunctionLike(type: ts.Type): boolean { + if (type.getCallSignatures().length > 0) return true; + if (type.getConstructSignatures().length > 0) return true; + + const symbol = type.symbol; + if (!symbol) return false; + + const flags = symbol.flags; + if ( + (flags & ts.SymbolFlags.Function) !== 0 || + (flags & ts.SymbolFlags.Method) !== 0 || + (flags & ts.SymbolFlags.Signature) !== 0 + ) { + return true; + } + + return false; +} + /** * Helper to extract array element type using multiple detection methods */ From 5b29b7d8c1d043545f1592958c59f0ea2186ff62 Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 18 Sep 2025 13:36:50 -0700 Subject: [PATCH 08/16] update README about skipping callables --- packages/schema-generator/README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/schema-generator/README.md b/packages/schema-generator/README.md index a699fad25..eafaf2191 100644 --- a/packages/schema-generator/README.md +++ b/packages/schema-generator/README.md @@ -21,6 +21,20 @@ keeping wrapper semantics explicit and simple. Implementation: see `src/schema-generator.ts` (`formatType`) and `src/type-utils.ts` (`getNamedTypeKey` filtering). +## Function Properties + +- Properties whose resolved type is callable or constructable are skipped + entirely so we do not emit function shapes in JSON Schema output. +- Method signatures, declared methods, and properties whose type exposes call + signatures are all filtered before we decide on `required` membership or emit + attribute metadata (docs, default wrappers, etc.). +- This keeps schemas focused on serialisable data: JSON Schema cannot describe + runtime function values, and downstream tooling expects objects, arrays, and + primitives only. + +Implementation: see `src/formatters/object-formatter.ts` and +`src/type-utils.ts:isFunctionLike`. + ## Running - Check typings: `deno task check` From 990d39077216711be97282eeb254e8a328fac81c Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 18 Sep 2025 14:28:39 -0700 Subject: [PATCH 09/16] handle anonymous type cycles more properly by synthesizing a name for them AnonymousType_{counter} --- packages/schema-generator/src/interface.ts | 4 ++ .../schema-generator/src/schema-generator.ts | 44 ++++++++++++------- .../test/schema-generator.test.ts | 32 ++++++++++++++ 3 files changed, 65 insertions(+), 15 deletions(-) diff --git a/packages/schema-generator/src/interface.ts b/packages/schema-generator/src/interface.ts index 865d9f9c7..a50724d21 100644 --- a/packages/schema-generator/src/interface.ts +++ b/packages/schema-generator/src/interface.ts @@ -24,6 +24,10 @@ export interface GenerationContext { definitions: Record; /** Which $refs have been emitted */ emittedRefs: Set; + /** Synthetic names for anonymous recursive types */ + anonymousNames: WeakMap; + /** Counter to generate stable synthetic identifiers */ + anonymousNameCounter: number; // Stack state (push/pop during recursion) /** Current recursion path for cycle detection */ diff --git a/packages/schema-generator/src/schema-generator.ts b/packages/schema-generator/src/schema-generator.ts index e4bb4dfaa..8fad18cf2 100644 --- a/packages/schema-generator/src/schema-generator.ts +++ b/packages/schema-generator/src/schema-generator.ts @@ -54,6 +54,8 @@ export class SchemaGenerator implements ISchemaGenerator { // Accumulating state definitions: {}, emittedRefs: new Set(), + anonymousNames: new WeakMap(), + anonymousNameCounter: 0, // Stack state definitionStack: new Set(), @@ -118,6 +120,17 @@ export class SchemaGenerator implements ISchemaGenerator { return type; } + private ensureSyntheticName( + type: ts.Type, + context: GenerationContext, + ): string { + let existing = context.anonymousNames.get(type); + if (existing) return existing; + const synthetic = `AnonymousType_${++context.anonymousNameCounter}`; + context.anonymousNames.set(type, synthetic); + return synthetic; + } + /** * Format a type using the appropriate formatter */ @@ -130,7 +143,11 @@ export class SchemaGenerator implements ISchemaGenerator { // Hoist every named type (excluding wrappers filtered by getNamedTypeKey) // into definitions and return $ref for non-root uses. Cycle detection // still applies via definitionStack. - const namedKey = getNamedTypeKey(type); + let namedKey = getNamedTypeKey(type); + if (!namedKey) { + const synthetic = context.anonymousNames.get(type); + if (synthetic) namedKey = synthetic; + } if (namedKey) { if ( context.inProgressNames.has(namedKey) || context.definitions[namedKey] @@ -157,14 +174,10 @@ export class SchemaGenerator implements ISchemaGenerator { context.emittedRefs.add(namedKey); return { "$ref": `#/definitions/${namedKey}` }; } - // Anonymous recursive type - can't create $ref, use permissive fallback - // to break the cycle - return { - type: "object", - additionalProperties: true, - $comment: - "Anonymous recursive type - cannot create named reference to break cycle", - }; + const syntheticKey = this.ensureSyntheticName(type, context); + context.inProgressNames.add(syntheticKey); + context.emittedRefs.add(syntheticKey); + return { "$ref": `#/definitions/${syntheticKey}` }; } // Push current type onto the stack @@ -178,15 +191,16 @@ export class SchemaGenerator implements ISchemaGenerator { const result = formatter.formatType(type, context); // If this is a named type (all-named policy), store in definitions. - if (namedKey) { - context.definitions[namedKey] = result; - context.inProgressNames.delete(namedKey); + const keyForDef = namedKey ?? context.anonymousNames.get(type); + if (keyForDef) { + context.definitions[keyForDef] = result; + context.inProgressNames.delete(keyForDef); context.definitionStack.delete( this.createStackKey(type, context.typeNode, context.typeChecker), ); if (!isRootType) { - context.emittedRefs.add(namedKey); - return { "$ref": `#/definitions/${namedKey}` }; + context.emittedRefs.add(keyForDef); + return { "$ref": `#/definitions/${keyForDef}` }; } // For root, keep inline; buildFinalSchema may promote if we choose } @@ -230,7 +244,7 @@ export class SchemaGenerator implements ISchemaGenerator { } // Decide if we promote root to a $ref - const namedKey = getNamedTypeKey(type); + const namedKey = getNamedTypeKey(type) ?? context.anonymousNames.get(type); const shouldPromoteRoot = this.shouldPromoteToRef(namedKey, context); let base: SchemaDefinition; diff --git a/packages/schema-generator/test/schema-generator.test.ts b/packages/schema-generator/test/schema-generator.test.ts index 85f8d2466..40e8577bf 100644 --- a/packages/schema-generator/test/schema-generator.test.ts +++ b/packages/schema-generator/test/schema-generator.test.ts @@ -93,4 +93,36 @@ type CalculatorRequest = { expect(schema).toEqual({}); }); }); + + describe("anonymous recursion", () => { + it("hoists anonymous recursive types with synthetic definitions", async () => { + const generator = new SchemaGenerator(); + const code = ` +type Wrapper = { + node: { + value: string; + next?: Wrapper["node"]; + }; +};`; + const { type, checker } = await getTypeFromCode(code, "Wrapper"); + + const schema = generator.generateSchema(type, checker); + const root = schema as Record; + const properties = root.properties as + | Record> + | undefined; + const definitions = root.definitions as + | Record> + | undefined; + + expect(properties?.node).toEqual({ + $ref: "#/definitions/AnonymousType_1", + }); + expect(definitions).toBeDefined(); + expect(Object.keys(definitions ?? {})).toContain("AnonymousType_1"); + expect(JSON.stringify(schema)).not.toContain( + "Anonymous recursive type", + ); + }); + }); }); From 84c627b0f52d85b16f18ccf0ac0c61079a0b44fb Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 18 Sep 2025 14:28:59 -0700 Subject: [PATCH 10/16] fix linting --- packages/schema-generator/src/schema-generator.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/schema-generator/src/schema-generator.ts b/packages/schema-generator/src/schema-generator.ts index 8fad18cf2..41a34c392 100644 --- a/packages/schema-generator/src/schema-generator.ts +++ b/packages/schema-generator/src/schema-generator.ts @@ -124,7 +124,7 @@ export class SchemaGenerator implements ISchemaGenerator { type: ts.Type, context: GenerationContext, ): string { - let existing = context.anonymousNames.get(type); + const existing = context.anonymousNames.get(type); if (existing) return existing; const synthetic = `AnonymousType_${++context.anonymousNameCounter}`; context.anonymousNames.set(type, synthetic); From 0d77403c03987ffbd67f1c7a0f5dc7d0b40020bc Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 18 Sep 2025 14:41:03 -0700 Subject: [PATCH 11/16] factor out structure for special-casing certain built-in types' schemas --- .../src/formatters/object-formatter.ts | 50 +++++++++++++------ .../test/schema-generator.test.ts | 23 +++++++++ 2 files changed, 59 insertions(+), 14 deletions(-) diff --git a/packages/schema-generator/src/formatters/object-formatter.ts b/packages/schema-generator/src/formatters/object-formatter.ts index 91c7927bd..9b53139b1 100644 --- a/packages/schema-generator/src/formatters/object-formatter.ts +++ b/packages/schema-generator/src/formatters/object-formatter.ts @@ -40,20 +40,8 @@ export class ObjectFormatter implements TypeFormatter { return { type: "object", additionalProperties: true }; } - // Special-case Date to a string with date-time format (match old behavior) - if (type.symbol?.name === "Date" && type.symbol?.valueDeclaration) { - // Check if this is the built-in Date type (not a user-defined type named "Date") - const sourceFile = type.symbol.valueDeclaration.getSourceFile(); - - if ( - sourceFile.fileName.includes("lib.") || - sourceFile.fileName.includes("typescript/lib") || - sourceFile.fileName.includes("ES2023.d.ts") || - sourceFile.fileName.includes("DOM.d.ts") - ) { - return { type: "string", format: "date-time" }; - } - } + const builtin = this.lookupBuiltInSchema(type); + if (builtin) return builtin; // Do not early-return for empty object types. Instead, try to enumerate // properties via the checker to allow type literals to surface members. @@ -177,4 +165,38 @@ export class ObjectFormatter implements TypeFormatter { return schema; } + + private lookupBuiltInSchema(type: ts.Type): SchemaDefinition | undefined { + const symbol = type.symbol; + if (!symbol?.valueDeclaration) return undefined; + const builtin = BUILT_IN_SCHEMAS.find((entry) => entry.test(type)); + return builtin ? structuredClone(builtin.schema) : undefined; + } +} + +type BuiltInSchemaEntry = { + test: (type: ts.Type) => boolean; + schema: SchemaDefinition; +}; + +const BUILT_IN_SCHEMAS: BuiltInSchemaEntry[] = [ + { + test: isNativeType("Date"), + schema: { type: "string", format: "date-time" }, + }, +]; + +function isNativeType(name: string) { + return (type: ts.Type) => { + const symbol = type.symbol; + if (!symbol || symbol.name !== name || !symbol.valueDeclaration) { + return false; + } + const sourceFile = symbol.valueDeclaration.getSourceFile(); + const fileName = sourceFile.fileName; + return fileName.includes("lib.") || + fileName.includes("typescript/lib") || + fileName.includes("ES2023.d.ts") || + fileName.includes("DOM.d.ts"); + }; } diff --git a/packages/schema-generator/test/schema-generator.test.ts b/packages/schema-generator/test/schema-generator.test.ts index 40e8577bf..be99081d0 100644 --- a/packages/schema-generator/test/schema-generator.test.ts +++ b/packages/schema-generator/test/schema-generator.test.ts @@ -125,4 +125,27 @@ type Wrapper = { ); }); }); + + describe("built-in mappings", () => { + it("formats Date as string with date-time format without hoisting", async () => { + const generator = new SchemaGenerator(); + const code = ` +interface HasDate { + createdAt: Date; +}`; + const { type, checker } = await getTypeFromCode(code, "HasDate"); + + const schema = generator.generateSchema(type, checker); + const objectSchema = schema as Record; + const props = objectSchema.properties as + | Record> + | undefined; + + expect(objectSchema.definitions).toBeUndefined(); + expect(props?.createdAt).toEqual({ + type: "string", + format: "date-time", + }); + }); + }); }); From 9801b61647cd7be0a6e4668e0e8e3efc7493f971 Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 18 Sep 2025 15:40:40 -0700 Subject: [PATCH 12/16] handle certain built-in types specially --- .../src/formatters/object-formatter.ts | 42 ++++++++++---- packages/schema-generator/src/type-utils.ts | 24 +++++++- .../test/schema-generator.test.ts | 57 +++++++++++++++++++ 3 files changed, 111 insertions(+), 12 deletions(-) diff --git a/packages/schema-generator/src/formatters/object-formatter.ts b/packages/schema-generator/src/formatters/object-formatter.ts index 9b53139b1..7f1bf2de5 100644 --- a/packages/schema-generator/src/formatters/object-formatter.ts +++ b/packages/schema-generator/src/formatters/object-formatter.ts @@ -4,7 +4,11 @@ import type { SchemaDefinition, TypeFormatter, } from "../interface.ts"; -import { isFunctionLike, safeGetPropertyType } from "../type-utils.ts"; +import { + getPrimarySymbol, + isFunctionLike, + safeGetPropertyType, +} from "../type-utils.ts"; import type { SchemaGenerator } from "../schema-generator.ts"; import { extractDocFromSymbolAndDecls, getDeclDocs } from "../doc-utils.ts"; import { getLogger } from "@commontools/utils/logger"; @@ -167,8 +171,6 @@ export class ObjectFormatter implements TypeFormatter { } private lookupBuiltInSchema(type: ts.Type): SchemaDefinition | undefined { - const symbol = type.symbol; - if (!symbol?.valueDeclaration) return undefined; const builtin = BUILT_IN_SCHEMAS.find((entry) => entry.test(type)); return builtin ? structuredClone(builtin.schema) : undefined; } @@ -184,19 +186,37 @@ const BUILT_IN_SCHEMAS: BuiltInSchemaEntry[] = [ test: isNativeType("Date"), schema: { type: "string", format: "date-time" }, }, + { + test: isNativeType("URL"), + schema: { type: "string", format: "uri" }, + }, + { + test: isNativeType("Uint8Array"), + schema: true, + }, + { + test: isNativeType("ArrayBuffer"), + schema: true, + }, ]; function isNativeType(name: string) { return (type: ts.Type) => { - const symbol = type.symbol; - if (!symbol || symbol.name !== name || !symbol.valueDeclaration) { + const symbol = getPrimarySymbol(type); + if (!symbol || symbol.name !== name) { return false; } - const sourceFile = symbol.valueDeclaration.getSourceFile(); - const fileName = sourceFile.fileName; - return fileName.includes("lib.") || - fileName.includes("typescript/lib") || - fileName.includes("ES2023.d.ts") || - fileName.includes("DOM.d.ts"); + const declarations = [ + symbol.valueDeclaration, + ...(symbol.declarations ?? []), + ].filter((decl): decl is ts.Declaration => Boolean(decl)); + if (declarations.length === 0) return false; + return declarations.some((decl) => { + const fileName = decl.getSourceFile().fileName; + return fileName.includes("lib.") || + fileName.includes("typescript/lib") || + fileName.includes("ES2023.d.ts") || + fileName.includes("DOM.d.ts"); + }); }; } diff --git a/packages/schema-generator/src/type-utils.ts b/packages/schema-generator/src/type-utils.ts index 5e5cd77de..d7641690d 100644 --- a/packages/schema-generator/src/type-utils.ts +++ b/packages/schema-generator/src/type-utils.ts @@ -115,6 +115,19 @@ export interface TypeWithInternals extends ts.Type { intrinsicName?: string; } +/** + * Resolve the most relevant symbol for a type, accounting for references, + * aliases, and internal helper accessors exposed on some compiler objects. + */ +export function getPrimarySymbol(type: ts.Type): ts.Symbol | undefined { + if (type.symbol) return type.symbol; + const ref = type as ts.TypeReference; + if (ref.target?.symbol) return ref.target.symbol; + const alias = (type as TypeWithInternals).aliasSymbol; + if (alias) return alias; + return undefined; +} + /** * Return a public/stable named key for a type if and only if it has a useful * symbol name. Filters out anonymous ("__type") and wrapper/container names @@ -161,7 +174,10 @@ export function getNamedTypeKey( if (name === "Cell" || name === "Stream" || name === "Default") { return undefined; } - if (name === "Date") return undefined; + if ( + name === "Date" || name === "URL" || + name === "Uint8Array" || name === "ArrayBuffer" + ) return undefined; return name; } @@ -292,6 +308,12 @@ export function getArrayElementInfo( if ((type.flags & ts.TypeFlags.Object) === 0) { return undefined; } + + const primarySymbol = getPrimarySymbol(type); + const primaryName = primarySymbol?.name; + if (primaryName === "Uint8Array" || primaryName === "ArrayBuffer") { + return undefined; + } // Check ObjectFlags.Reference for Array/ReadonlyArray const objectFlags = (type as ts.ObjectType).objectFlags ?? 0; diff --git a/packages/schema-generator/test/schema-generator.test.ts b/packages/schema-generator/test/schema-generator.test.ts index be99081d0..476b77e1d 100644 --- a/packages/schema-generator/test/schema-generator.test.ts +++ b/packages/schema-generator/test/schema-generator.test.ts @@ -147,5 +147,62 @@ interface HasDate { format: "date-time", }); }); + + it("formats URL as string with uri format without hoisting", async () => { + const generator = new SchemaGenerator(); + const code = ` +interface HasUrl { + homepage: URL; +}`; + const { type, checker } = await getTypeFromCode(code, "HasUrl"); + + const schema = generator.generateSchema(type, checker); + const objectSchema = schema as Record; + const props = objectSchema.properties as + | Record> + | undefined; + + expect(objectSchema.definitions).toBeUndefined(); + expect(props?.homepage).toEqual({ + type: "string", + format: "uri", + }); + }); + + it("formats Uint8Array as permissive true schema", async () => { + const generator = new SchemaGenerator(); + const code = ` +interface BinaryHolder { + data: Uint8Array; +}`; + const { type, checker } = await getTypeFromCode(code, "BinaryHolder"); + + const schema = generator.generateSchema(type, checker); + const objectSchema = schema as Record; + const props = objectSchema.properties as + | Record + | undefined; + + expect(objectSchema.definitions).toBeUndefined(); + expect(props?.data).toBe(true); + }); + + it("formats ArrayBuffer as permissive true schema", async () => { + const generator = new SchemaGenerator(); + const code = ` +interface BufferHolder { + buffer: ArrayBuffer; +}`; + const { type, checker } = await getTypeFromCode(code, "BufferHolder"); + + const schema = generator.generateSchema(type, checker); + const objectSchema = schema as Record; + const props = objectSchema.properties as + | Record + | undefined; + + expect(objectSchema.definitions).toBeUndefined(); + expect(props?.buffer).toBe(true); + }); }); }); From 4d710c1d6d406b3034defeeef79262f3242e867e Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 18 Sep 2025 15:45:35 -0700 Subject: [PATCH 13/16] fix bug i just introduced --- packages/schema-generator/README.md | 14 ++++++++++++-- packages/schema-generator/src/schema-generator.ts | 13 +++++++++++++ packages/schema-generator/src/type-utils.ts | 3 ++- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/packages/schema-generator/README.md b/packages/schema-generator/README.md index eafaf2191..9fceea534 100644 --- a/packages/schema-generator/README.md +++ b/packages/schema-generator/README.md @@ -6,8 +6,9 @@ Short notes on the JSON Schema generator and ref/definitions behavior. - Hoists every named type into `definitions` and emits `$ref` for non‑root occurrences. -- Excludes wrapper/container names from hoisting: `Array`, `ReadonlyArray`, - `Cell`, `Stream`, `Default`, `Date`. +- Excludes wrapper/container names and native leaf types from hoisting: + `Array`, `ReadonlyArray`, `Cell`, `Stream`, `Default`, `Date`, `URL`, + `Uint8Array`, `ArrayBuffer`. - Root types remain inline; `definitions` are included only if at least one `$ref` is emitted. - Anonymous/type‑literal shapes (including aliases that resolve to anonymous @@ -21,6 +22,15 @@ keeping wrapper semantics explicit and simple. Implementation: see `src/schema-generator.ts` (`formatType`) and `src/type-utils.ts` (`getNamedTypeKey` filtering). +## Native Type Schemas + +- Maps ECMAScript built-ins directly when they appear as properties: + - `Date` → `{ type: "string", format: "date-time" }` + - `URL` → `{ type: "string", format: "uri" }` + - `Uint8Array` and `ArrayBuffer` → `true` (permissive JSON Schema leaf) +- These shortcuts keep schemas inline without emitting `$ref` definitions, + while avoiding conflicts with array detection or hoisting. + ## Function Properties - Properties whose resolved type is callable or constructable are skipped diff --git a/packages/schema-generator/src/schema-generator.ts b/packages/schema-generator/src/schema-generator.ts index 41a34c392..723797675 100644 --- a/packages/schema-generator/src/schema-generator.ts +++ b/packages/schema-generator/src/schema-generator.ts @@ -139,6 +139,19 @@ export class SchemaGenerator implements ISchemaGenerator { context: GenerationContext, isRootType: boolean = false, ): SchemaDefinition { + if ((type.flags & ts.TypeFlags.TypeParameter) !== 0) { + const checker = context.typeChecker; + const baseConstraint = checker.getBaseConstraintOfType(type); + if (baseConstraint && baseConstraint !== type) { + return this.formatType(baseConstraint, context, isRootType); + } + const defaultConstraint = checker.getDefaultFromTypeParameter?.(type); + if (defaultConstraint && defaultConstraint !== type) { + return this.formatType(defaultConstraint, context, isRootType); + } + return {}; + } + // All-named strategy: // Hoist every named type (excluding wrappers filtered by getNamedTypeKey) // into definitions and return $ref for non-root uses. Cycle detection diff --git a/packages/schema-generator/src/type-utils.ts b/packages/schema-generator/src/type-utils.ts index d7641690d..f32bea4c2 100644 --- a/packages/schema-generator/src/type-utils.ts +++ b/packages/schema-generator/src/type-utils.ts @@ -156,7 +156,8 @@ export function getNamedTypeKey( (symFlags & ts.SymbolFlags.Property) !== 0 || (symFlags & ts.SymbolFlags.Method) !== 0 || (symFlags & ts.SymbolFlags.Signature) !== 0 || - (symFlags & ts.SymbolFlags.Function) !== 0 + (symFlags & ts.SymbolFlags.Function) !== 0 || + (symFlags & ts.SymbolFlags.TypeParameter) !== 0 ) { return undefined; } From 701fd1c6c35c18cc6a31b2ed6b39108b327ff353 Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 18 Sep 2025 16:25:26 -0700 Subject: [PATCH 14/16] fix built-in type handling --- packages/schema-generator/README.md | 17 ++-- .../src/formatters/intersection-formatter.ts | 5 ++ .../src/formatters/object-formatter.ts | 59 ++----------- .../src/formatters/union-formatter.ts | 15 +++- packages/schema-generator/src/type-utils.ts | 86 +++++++++++++++++-- .../test/native-type-parameters.test.ts | 24 ++++++ .../test/schema-generator.test.ts | 25 ++++++ 7 files changed, 165 insertions(+), 66 deletions(-) create mode 100644 packages/schema-generator/test/native-type-parameters.test.ts diff --git a/packages/schema-generator/README.md b/packages/schema-generator/README.md index 9fceea534..3108db202 100644 --- a/packages/schema-generator/README.md +++ b/packages/schema-generator/README.md @@ -6,9 +6,9 @@ Short notes on the JSON Schema generator and ref/definitions behavior. - Hoists every named type into `definitions` and emits `$ref` for non‑root occurrences. -- Excludes wrapper/container names and native leaf types from hoisting: - `Array`, `ReadonlyArray`, `Cell`, `Stream`, `Default`, `Date`, `URL`, - `Uint8Array`, `ArrayBuffer`. +- Excludes wrapper/container names and native leaf types from hoisting: `Array`, + `ReadonlyArray`, `Cell`, `Stream`, `Default`, `Date`, `URL`, `Uint8Array`, + `ArrayBuffer`. - Root types remain inline; `definitions` are included only if at least one `$ref` is emitted. - Anonymous/type‑literal shapes (including aliases that resolve to anonymous @@ -27,9 +27,14 @@ Implementation: see `src/schema-generator.ts` (`formatType`) and - Maps ECMAScript built-ins directly when they appear as properties: - `Date` → `{ type: "string", format: "date-time" }` - `URL` → `{ type: "string", format: "uri" }` - - `Uint8Array` and `ArrayBuffer` → `true` (permissive JSON Schema leaf) -- These shortcuts keep schemas inline without emitting `$ref` definitions, - while avoiding conflicts with array detection or hoisting. + - Typed array family (`Uint8Array`, `Uint8ClampedArray`, `Int8Array`, + `Uint16Array`, `Int16Array`, `Uint32Array`, `Int32Array`, `Float32Array`, + `Float64Array`, `BigInt64Array`, `BigUint64Array`) plus `ArrayBuffer`, + `ArrayBufferLike`, `SharedArrayBuffer`, and `ArrayBufferView` → `true` + (permissive JSON Schema leaf) +- These shortcuts keep schemas inline without emitting `$ref` definitions, while + avoiding conflicts with array detection or hoisting, even when the compiler + widens them via intersections or aliases. ## Function Properties diff --git a/packages/schema-generator/src/formatters/intersection-formatter.ts b/packages/schema-generator/src/formatters/intersection-formatter.ts index 7e37a5af4..cc9271e16 100644 --- a/packages/schema-generator/src/formatters/intersection-formatter.ts +++ b/packages/schema-generator/src/formatters/intersection-formatter.ts @@ -5,6 +5,7 @@ import type { TypeFormatter, } from "../interface.ts"; import type { SchemaGenerator } from "../schema-generator.ts"; +import { cloneSchemaDefinition, getNativeTypeSchema } from "../type-utils.ts"; import { getLogger } from "@commontools/utils/logger"; import { isRecord } from "@commontools/utils/types"; import { extractDocFromType } from "../doc-utils.ts"; @@ -22,6 +23,10 @@ export class IntersectionFormatter implements TypeFormatter { formatType(type: ts.Type, context: GenerationContext): SchemaDefinition { const checker = context.typeChecker; + const native = getNativeTypeSchema(type, checker); + if (native !== undefined) { + return cloneSchemaDefinition(native); + } const inter = type as ts.IntersectionType; const parts = inter.types ?? []; diff --git a/packages/schema-generator/src/formatters/object-formatter.ts b/packages/schema-generator/src/formatters/object-formatter.ts index 7f1bf2de5..a6823b24a 100644 --- a/packages/schema-generator/src/formatters/object-formatter.ts +++ b/packages/schema-generator/src/formatters/object-formatter.ts @@ -5,7 +5,8 @@ import type { TypeFormatter, } from "../interface.ts"; import { - getPrimarySymbol, + cloneSchemaDefinition, + getNativeTypeSchema, isFunctionLike, safeGetPropertyType, } from "../type-utils.ts"; @@ -44,7 +45,7 @@ export class ObjectFormatter implements TypeFormatter { return { type: "object", additionalProperties: true }; } - const builtin = this.lookupBuiltInSchema(type); + const builtin = this.lookupBuiltInSchema(type, checker); if (builtin) return builtin; // Do not early-return for empty object types. Instead, try to enumerate @@ -170,53 +171,11 @@ export class ObjectFormatter implements TypeFormatter { return schema; } - private lookupBuiltInSchema(type: ts.Type): SchemaDefinition | undefined { - const builtin = BUILT_IN_SCHEMAS.find((entry) => entry.test(type)); - return builtin ? structuredClone(builtin.schema) : undefined; + private lookupBuiltInSchema( + type: ts.Type, + checker: ts.TypeChecker, + ): SchemaDefinition | boolean | undefined { + const builtin = getNativeTypeSchema(type, checker); + return builtin === undefined ? undefined : cloneSchemaDefinition(builtin); } } - -type BuiltInSchemaEntry = { - test: (type: ts.Type) => boolean; - schema: SchemaDefinition; -}; - -const BUILT_IN_SCHEMAS: BuiltInSchemaEntry[] = [ - { - test: isNativeType("Date"), - schema: { type: "string", format: "date-time" }, - }, - { - test: isNativeType("URL"), - schema: { type: "string", format: "uri" }, - }, - { - test: isNativeType("Uint8Array"), - schema: true, - }, - { - test: isNativeType("ArrayBuffer"), - schema: true, - }, -]; - -function isNativeType(name: string) { - return (type: ts.Type) => { - const symbol = getPrimarySymbol(type); - if (!symbol || symbol.name !== name) { - return false; - } - const declarations = [ - symbol.valueDeclaration, - ...(symbol.declarations ?? []), - ].filter((decl): decl is ts.Declaration => Boolean(decl)); - if (declarations.length === 0) return false; - return declarations.some((decl) => { - const fileName = decl.getSourceFile().fileName; - return fileName.includes("lib.") || - fileName.includes("typescript/lib") || - fileName.includes("ES2023.d.ts") || - fileName.includes("DOM.d.ts"); - }); - }; -} diff --git a/packages/schema-generator/src/formatters/union-formatter.ts b/packages/schema-generator/src/formatters/union-formatter.ts index 6fb4d4d77..f67c83bcc 100644 --- a/packages/schema-generator/src/formatters/union-formatter.ts +++ b/packages/schema-generator/src/formatters/union-formatter.ts @@ -5,7 +5,11 @@ import type { TypeFormatter, } from "../interface.ts"; import type { SchemaGenerator } from "../schema-generator.ts"; -import { TypeWithInternals } from "../type-utils.ts"; +import { + cloneSchemaDefinition, + getNativeTypeSchema, + TypeWithInternals, +} from "../type-utils.ts"; export class UnionFormatter implements TypeFormatter { constructor(private schemaGenerator: SchemaGenerator) {} @@ -31,8 +35,13 @@ export class UnionFormatter implements TypeFormatter { const hasNull = filtered.some((m) => (m.flags & ts.TypeFlags.Null) !== 0); const nonNull = filtered.filter((m) => (m.flags & ts.TypeFlags.Null) === 0); - const generate = (t: ts.Type, typeNode?: ts.TypeNode): SchemaDefinition => - this.schemaGenerator.formatChildType(t, context, typeNode); + const generate = (t: ts.Type, typeNode?: ts.TypeNode): SchemaDefinition => { + const native = getNativeTypeSchema(t, context.typeChecker); + if (native !== undefined) { + return cloneSchemaDefinition(native); + } + return this.schemaGenerator.formatChildType(t, context, typeNode); + }; // Case: exactly one non-null member + null => anyOf (nullable type) // Note: We use anyOf instead of oneOf for better consumer compatibility. diff --git a/packages/schema-generator/src/type-utils.ts b/packages/schema-generator/src/type-utils.ts index f32bea4c2..5a1a44c31 100644 --- a/packages/schema-generator/src/type-utils.ts +++ b/packages/schema-generator/src/type-utils.ts @@ -1,5 +1,7 @@ import ts from "typescript"; +import type { SchemaDefinition } from "./interface.ts"; + /** * Safe wrapper for TypeScript checker APIs that may throw in reduced environments */ @@ -115,6 +117,28 @@ export interface TypeWithInternals extends ts.Type { intrinsicName?: string; } +const NATIVE_TYPE_SCHEMAS: Record = { + Date: { type: "string", format: "date-time" }, + URL: { type: "string", format: "uri" }, + ArrayBuffer: true, + ArrayBufferLike: true, + SharedArrayBuffer: true, + ArrayBufferView: true, + Uint8Array: true, + Uint8ClampedArray: true, + Int8Array: true, + Uint16Array: true, + Int16Array: true, + Uint32Array: true, + Int32Array: true, + Float32Array: true, + Float64Array: true, + BigInt64Array: true, + BigUint64Array: true, +}; + +const NATIVE_TYPE_NAMES = new Set(Object.keys(NATIVE_TYPE_SCHEMAS)); + /** * Resolve the most relevant symbol for a type, accounting for references, * aliases, and internal helper accessors exposed on some compiler objects. @@ -128,6 +152,58 @@ export function getPrimarySymbol(type: ts.Type): ts.Symbol | undefined { return undefined; } +export function cloneSchemaDefinition( + schema: T, +): T { + return (typeof schema === "boolean" ? schema : structuredClone(schema)) as T; +} + +export function getNativeTypeSchema( + type: ts.Type, + checker: ts.TypeChecker, +): SchemaDefinition | boolean | undefined { + const visited = new Set(); + + const resolve = ( + current: ts.Type, + ): SchemaDefinition | boolean | undefined => { + if (visited.has(current)) return undefined; + visited.add(current); + + if ((current.flags & ts.TypeFlags.TypeParameter) !== 0) { + const base = checker.getBaseConstraintOfType(current); + if (base && base !== current) { + const resolved = resolve(base); + if (resolved !== undefined) return resolved; + } + const defaultConstraint = checker.getDefaultFromTypeParameter?.(current); + if (defaultConstraint && defaultConstraint !== current) { + const resolved = resolve(defaultConstraint); + if (resolved !== undefined) return resolved; + } + return undefined; + } + + if ((current.flags & ts.TypeFlags.Intersection) !== 0) { + const intersection = current as ts.IntersectionType; + for (const part of intersection.types) { + const resolved = resolve(part); + if (resolved !== undefined) return resolved; + } + } + + const symbol = getPrimarySymbol(current); + const name = symbol?.getName(); + if (name && NATIVE_TYPE_NAMES.has(name)) { + return cloneSchemaDefinition(NATIVE_TYPE_SCHEMAS[name]!); + } + + return undefined; + }; + + return resolve(type); +} + /** * Return a public/stable named key for a type if and only if it has a useful * symbol name. Filters out anonymous ("__type") and wrapper/container names @@ -175,10 +251,7 @@ export function getNamedTypeKey( if (name === "Cell" || name === "Stream" || name === "Default") { return undefined; } - if ( - name === "Date" || name === "URL" || - name === "Uint8Array" || name === "ArrayBuffer" - ) return undefined; + if (name && NATIVE_TYPE_NAMES.has(name)) return undefined; return name; } @@ -310,9 +383,8 @@ export function getArrayElementInfo( return undefined; } - const primarySymbol = getPrimarySymbol(type); - const primaryName = primarySymbol?.name; - if (primaryName === "Uint8Array" || primaryName === "ArrayBuffer") { + const native = getNativeTypeSchema(type, checker); + if (native !== undefined) { return undefined; } // Check ObjectFlags.Reference for Array/ReadonlyArray diff --git a/packages/schema-generator/test/native-type-parameters.test.ts b/packages/schema-generator/test/native-type-parameters.test.ts new file mode 100644 index 000000000..10943f773 --- /dev/null +++ b/packages/schema-generator/test/native-type-parameters.test.ts @@ -0,0 +1,24 @@ +import { expect } from "@std/expect"; +import { describe, it } from "@std/testing/bdd"; +import { SchemaGenerator } from "../src/schema-generator.ts"; +import { getTypeFromCode } from "./utils.ts"; + +describe("Native type parameters", () => { + it("unwraps Uint8Array with defaulted typed buffer", async () => { + const code = ` +interface Wrapper { + value: Uint8Array; + pointer: Uint8Array; +} +`; + const { type, checker } = await getTypeFromCode(code, "Wrapper"); + const generator = new SchemaGenerator(); + const schema = generator.generateSchema(type, checker); + const props = (schema as Record).properties as + | Record + | undefined; + expect(props?.value).toBe(true); + expect(props?.pointer).toBe(true); + expect((schema as Record).definitions).toBeUndefined(); + }); +}); diff --git a/packages/schema-generator/test/schema-generator.test.ts b/packages/schema-generator/test/schema-generator.test.ts index 476b77e1d..6a4cde3d8 100644 --- a/packages/schema-generator/test/schema-generator.test.ts +++ b/packages/schema-generator/test/schema-generator.test.ts @@ -204,5 +204,30 @@ interface BufferHolder { expect(objectSchema.definitions).toBeUndefined(); expect(props?.buffer).toBe(true); }); + + it("collapses unions of native binary types", async () => { + const generator = new SchemaGenerator(); + const code = ` +interface HasImage { + image: string | Uint8Array | ArrayBuffer | URL; +}`; + const { type, checker } = await getTypeFromCode(code, "HasImage"); + + const schema = generator.generateSchema(type, checker); + const objectSchema = schema as Record; + const props = objectSchema.properties as + | Record | boolean> + | undefined; + + expect(objectSchema.definitions).toBeUndefined(); + expect(props?.image).toEqual({ + anyOf: [ + { type: "string" }, + true, + true, + { type: "string", format: "uri" }, + ], + }); + }); }); }); From def478da09c13db47659b3875872746542c2baad Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 18 Sep 2025 16:40:27 -0700 Subject: [PATCH 15/16] improve handling of boolean schemas as per robin's PR comment --- packages/schema-generator/src/schema-generator.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/packages/schema-generator/src/schema-generator.ts b/packages/schema-generator/src/schema-generator.ts index 723797675..ee935c15e 100644 --- a/packages/schema-generator/src/schema-generator.ts +++ b/packages/schema-generator/src/schema-generator.ts @@ -274,17 +274,11 @@ export class SchemaGenerator implements ISchemaGenerator { // Handle boolean schemas (rare, but supported by JSON Schema) if (typeof base === "boolean") { - const filtered = this.collectReferencedDefinitions(base, definitions); - if (Object.keys(filtered).length === 0) return base; - return base === false - ? { - $schema: "https://json-schema.org/draft-07/schema#", - not: true, - definitions: filtered, - } + return base + ? { $schema: "https://json-schema.org/draft-07/schema#" } : { $schema: "https://json-schema.org/draft-07/schema#", - definitions: filtered, + not: true, }; } From f5e8945540e9860d60f378f19aad05cc2b089f15 Mon Sep 17 00:00:00 2001 From: Gideon Wald Date: Thu, 18 Sep 2025 16:43:18 -0700 Subject: [PATCH 16/16] fmt --- packages/schema-generator/src/schema-generator.ts | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/packages/schema-generator/src/schema-generator.ts b/packages/schema-generator/src/schema-generator.ts index ee935c15e..c4393ff29 100644 --- a/packages/schema-generator/src/schema-generator.ts +++ b/packages/schema-generator/src/schema-generator.ts @@ -274,12 +274,10 @@ export class SchemaGenerator implements ISchemaGenerator { // Handle boolean schemas (rare, but supported by JSON Schema) if (typeof base === "boolean") { - return base - ? { $schema: "https://json-schema.org/draft-07/schema#" } - : { - $schema: "https://json-schema.org/draft-07/schema#", - not: true, - }; + return base ? { $schema: "https://json-schema.org/draft-07/schema#" } : { + $schema: "https://json-schema.org/draft-07/schema#", + not: true, + }; } // Object schema: attach only the definitions actually referenced by the