From 79bffce5b8db44dc3cdbb93527fc67ac4038cb25 Mon Sep 17 00:00:00 2001 From: gideon Date: Fri, 19 Sep 2025 11:55:37 -0700 Subject: [PATCH 1/2] change refs emission to be for all named non-container types (#1765) * change refs emission to be for all named non-container types (still do not auto-promote root) * format * doc with both short- and long-term strategies for finishing this implementation * progress towards working implementation, still WIP * update fixtures to no longer expect root types to always be promoted to named defs (only promote if referenced elsewhere) * and finish updating js-runtime fixtures' expectations too * don't include callables/function-like things in json schemas * update README about skipping callables * handle anonymous type cycles more properly by synthesizing a name for them AnonymousType_{counter} * fix linting * factor out structure for special-casing certain built-in types' schemas * handle certain built-in types specially * fix bug i just introduced * fix built-in type handling * improve handling of boolean schemas as per robin's PR comment * fmt --- ...array-cell-remove-intersection.expected.ts | 44 ++-- .../date-and-map-types.expected.ts | 98 ++------- .../jsx-complex-mixed.expected.tsx | 41 ++-- .../jsx-property-access.expected.tsx | 110 +++++----- .../opaque-ref-map.expected.ts | 30 ++- .../recipe-with-types.expected.tsx | 73 ++++--- packages/runner/src/builder/node-utils.ts | 5 +- packages/schema-generator/README.md | 56 +++++ .../promote-all-named-options.txt | 98 +++++++++ .../src/formatters/intersection-formatter.ts | 144 ++++++------ .../src/formatters/object-formatter.ts | 51 +++-- .../src/formatters/union-formatter.ts | 15 +- packages/schema-generator/src/interface.ts | 4 + .../schema-generator/src/schema-generator.ts | 206 +++++++++++++----- packages/schema-generator/src/type-utils.ts | 141 +++++++++++- .../schema/alias-edge-cases.expected.json | 40 ++-- .../fixtures/schema/date-types.expected.json | 19 +- .../default-nullable-null.expected.json | 4 +- .../schema/default-type.expected.json | 34 ++- .../defaults-complex-array.expected.json | 31 +-- .../defaults-complex-object.expected.json | 8 +- .../schema/descriptions-nested.expected.json | 51 +++-- .../nested-default-aliases.expected.json | 14 +- .../recipe-with-types-input.expected.json | 41 ++-- .../recipe-with-types-output.expected.json | 45 ++-- .../schema/simple-interface.expected.json | 4 +- .../test/native-type-parameters.test.ts | 24 ++ .../test/schema-generator.test.ts | 137 ++++++++++++ .../test/schema/complex-defaults.test.ts | 14 +- .../schema/type-aliases-and-shared.test.ts | 13 +- .../test/schema/type-to-schema.test.ts | 9 +- 31 files changed, 1124 insertions(+), 480 deletions(-) create mode 100644 packages/schema-generator/README.md create mode 100644 packages/schema-generator/promote-all-named-options.txt create mode 100644 packages/schema-generator/test/native-type-parameters.test.ts 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 020db9e29f..8ad618361e 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 41a9087205..e99cf9f8e7 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,23 @@ 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: {} } }, - required: ["clear", "delete", "forEach", "get", "has", "set", "size", "entries", "keys", "values"] + required: ["size"] } - }, - required: ["timestamp", "data"] + } } as const satisfies JSONSchema, { + $schema: "https://json-schema.org/draft-07/schema#", type: "object", properties: { lastUpdate: { @@ -70,56 +41,25 @@ 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: {} } }, - required: ["clear", "delete", "forEach", "get", "has", "set", "size", "entries", "keys", "values"] + required: ["size"] } - }, - 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 76c0809802..48f2ca5307 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,27 @@ 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"] + } + } } as const satisfies JSONSchema, (state) => { return { [UI]: (
@@ -84,4 +90,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 9d6344f6d0..18a056eeef 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,67 @@ 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: { + Config: { + type: "object", + properties: { + theme: { + type: "object", + properties: { + primaryColor: { + type: "string" + }, + secondaryColor: { + type: "string" + }, + fontSize: { + type: "number" + } + }, + required: ["primaryColor", "secondaryColor", "fontSize"] + }, + features: { + type: "object", + properties: { + darkMode: { + type: "boolean" + }, + beta: { + type: "boolean" + } + }, + required: ["darkMode", "beta"] + } + }, + required: ["theme", "features"] + }, + User: { type: "object", properties: { name: { @@ -72,57 +130,8 @@ export default recipe({ } }, required: ["name", "age", "active", "profile"] - }, - config: { - type: "object", - properties: { - theme: { - type: "object", - properties: { - primaryColor: { - type: "string" - }, - secondaryColor: { - type: "string" - }, - fontSize: { - type: "number" - } - }, - required: ["primaryColor", "secondaryColor", "fontSize"] - }, - features: { - type: "object", - properties: { - darkMode: { - type: "boolean" - }, - beta: { - type: "boolean" - } - }, - required: ["darkMode", "beta"] - } - }, - required: ["theme", "features"] - }, - items: { - type: "array", - items: { - type: "string" - } - }, - index: { - type: "number" - }, - numbers: { - type: "array", - items: { - type: "number" - } } - }, - required: ["user", "config", "items", "index", "numbers"] + } } as const satisfies JSONSchema, (state) => { return { [UI]: (
@@ -170,4 +179,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 880092ff6b..861461698c 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 e16cec786a..45a3a017f4 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,27 @@ 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"] + } + } } as const satisfies JSONSchema; const outputSchema = { + $schema: "https://json-schema.org/draft-07/schema#", type: "object", properties: { items_count: { @@ -53,19 +60,24 @@ 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"] + } + } } as const satisfies JSONSchema; // Handler that logs the message event const addItem = handler({ @@ -83,24 +95,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 +142,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 86b5d765a2..b9fef7b8a2 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/README.md b/packages/schema-generator/README.md new file mode 100644 index 0000000000..3108db2028 --- /dev/null +++ b/packages/schema-generator/README.md @@ -0,0 +1,56 @@ +# @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 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 + 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). + +## Native Type Schemas + +- Maps ECMAScript built-ins directly when they appear as properties: + - `Date` → `{ type: "string", format: "date-time" }` + - `URL` → `{ type: "string", format: "uri" }` + - 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 + +- 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` +- Run tests: `deno task test` 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 0000000000..f5a3c9b305 --- /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. + diff --git a/packages/schema-generator/src/formatters/intersection-formatter.ts b/packages/schema-generator/src/formatters/intersection-formatter.ts index 2a7f409996..cc9271e16b 100644 --- a/packages/schema-generator/src/formatters/intersection-formatter.ts +++ b/packages/schema-generator/src/formatters/intersection-formatter.ts @@ -5,29 +5,37 @@ 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"; 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; } 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 ?? []; 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 +45,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 +54,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 +70,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 +88,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 +103,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 +170,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 +221,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/formatters/object-formatter.ts b/packages/schema-generator/src/formatters/object-formatter.ts index 9c96a6b458..a6823b24a7 100644 --- a/packages/schema-generator/src/formatters/object-formatter.ts +++ b/packages/schema-generator/src/formatters/object-formatter.ts @@ -4,7 +4,12 @@ import type { SchemaDefinition, TypeFormatter, } from "../interface.ts"; -import { safeGetPropertyType } from "../type-utils.ts"; +import { + cloneSchemaDefinition, + getNativeTypeSchema, + 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"; @@ -40,20 +45,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, checker); + 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. @@ -66,13 +59,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 +76,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 +86,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, @@ -165,4 +170,12 @@ export class ObjectFormatter implements TypeFormatter { return schema; } + + private lookupBuiltInSchema( + type: ts.Type, + checker: ts.TypeChecker, + ): SchemaDefinition | boolean | undefined { + const builtin = getNativeTypeSchema(type, checker); + return builtin === undefined ? undefined : cloneSchemaDefinition(builtin); + } } diff --git a/packages/schema-generator/src/formatters/union-formatter.ts b/packages/schema-generator/src/formatters/union-formatter.ts index 6fb4d4d771..f67c83bccf 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/interface.ts b/packages/schema-generator/src/interface.ts index 865d9f9c7a..a50724d218 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 32759a253a..c4393ff298 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 @@ -56,6 +54,8 @@ export class SchemaGenerator implements ISchemaGenerator { // Accumulating state definitions: {}, emittedRefs: new Set(), + anonymousNames: new WeakMap(), + anonymousNameCounter: 0, // Stack state definitionStack: new Set(), @@ -90,21 +90,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(",") : ""; @@ -117,6 +120,17 @@ export class SchemaGenerator implements ISchemaGenerator { return type; } + private ensureSyntheticName( + type: ts.Type, + context: GenerationContext, + ): string { + const 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 */ @@ -125,22 +139,40 @@ 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); - const namedKey = getNamedTypeKey(type); - const inCycleByName = !!(namedKey && context.cyclicNames.has(namedKey)); + 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 {}; + } - if (namedKey && (inCycle || inCycleByName)) { + // 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. + 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] ) { + // 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); } @@ -155,13 +187,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 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 @@ -174,18 +203,19 @@ 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 - context.definitions[namedKey] = result; - context.inProgressNames.delete(namedKey); + // If this is a named type (all-named policy), store in definitions. + 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 } // Pop after formatting context.definitionStack.delete( @@ -195,7 +225,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), ); @@ -204,7 +235,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.", ); } @@ -224,40 +256,39 @@ export class SchemaGenerator implements ISchemaGenerator { return rootSchema; } - // Check if root schema should be promoted to a definition - const namedKey = getNamedTypeKey(type); + // Decide if we promote root to a $ref + const namedKey = getNamedTypeKey(type) ?? context.anonymousNames.get(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; } + base = { $ref: `#/definitions/${namedKey}` } as SchemaDefinition; + } else { + base = rootSchema; + } - // Return schema with $ref to root and definitions - return { + // 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#", - $ref: `#/definitions/${namedKey}`, - definitions, + not: true, }; } - // Return root schema with definitions - // Handle case where rootSchema might be boolean (per JSON Schema spec) - if (typeof rootSchema === "boolean") { - return rootSchema === false - ? { - $schema: "https://json-schema.org/draft-07/schema#", - not: true, - definitions, - } - : { $schema: "https://json-schema.org/draft-07/schema#", definitions }; - } - 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; } /** @@ -271,7 +302,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)); } @@ -352,9 +384,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, @@ -369,4 +400,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 88a9eab588..5a1a44c312 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,93 @@ 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. + */ +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; +} + +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 @@ -124,7 +213,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,15 +226,57 @@ 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 || + (symFlags & ts.SymbolFlags.TypeParameter) !== 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") { return undefined; } - if (name === "Date") return undefined; + if (name && NATIVE_TYPE_NAMES.has(name)) return undefined; 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 */ @@ -250,6 +382,11 @@ export function getArrayElementInfo( if ((type.flags & ts.TypeFlags.Object) === 0) { return undefined; } + + const native = getNativeTypeSchema(type, checker); + if (native !== undefined) { + return undefined; + } // Check ObjectFlags.Reference for Array/ReadonlyArray const objectFlags = (type as ts.ObjectType).objectFlags ?? 0; 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 275d4df4ce..fcc5c334c8 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 fa9efa7207..9f7c674f40 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,7 @@ ], "type": "object" }, - "userProfile": { + "UserProfile": { "properties": { "createdAt": { "format": "date-time", @@ -94,6 +94,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 f65f9d84c4..2f39deab22 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 1d707fcac6..f453b3f993 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,7 @@ ], "type": "object" }, - "userSettings": { + "UserSettings": { "properties": { "fontSize": { "default": 16, @@ -199,6 +199,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 122d84e678..7f1602d1d6 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,26 @@ { + "definitions": { + "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 3970e50771..5d9de49ae4 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/descriptions-nested.expected.json b/packages/schema-generator/test/fixtures/schema/descriptions-nested.expected.json index fbb4a14106..7b08f1a32b 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/nested-default-aliases.expected.json b/packages/schema-generator/test/fixtures/schema/nested-default-aliases.expected.json index f3d3feb0b8..2b4ad71397 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 42498ef9fe..7bd1b3a8c5 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,34 @@ { - "type": "object", + "definitions": { + "Item": { + "properties": { + "text": { + "default": "", + "type": "string" + } + }, + "required": [ + "text" + ], + "type": "object" + } + }, "properties": { - "title": { - "type": "string", - "default": "untitled" - }, "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 d979bbaf07..d9a2ce187d 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,38 @@ { - "type": "object", + "definitions": { + "Item": { + "properties": { + "text": { + "default": "", + "type": "string" + } + }, + "required": [ + "text" + ], + "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/simple-interface.expected.json b/packages/schema-generator/test/fixtures/schema/simple-interface.expected.json index d0d5e85222..4dde458d66 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/native-type-parameters.test.ts b/packages/schema-generator/test/native-type-parameters.test.ts new file mode 100644 index 0000000000..10943f7737 --- /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 85f8d24664..6a4cde3d84 100644 --- a/packages/schema-generator/test/schema-generator.test.ts +++ b/packages/schema-generator/test/schema-generator.test.ts @@ -93,4 +93,141 @@ 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", + ); + }); + }); + + 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", + }); + }); + + 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); + }); + + 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" }, + ], + }); + }); + }); }); diff --git a/packages/schema-generator/test/schema/complex-defaults.test.ts b/packages/schema-generator/test/schema/complex-defaults.test.ts index 39fc05e92b..7417ce3366 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 0377412731..b1631b2a9f 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 a28d195fe1..5e90545591 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 2188232550f99d2178ba02104198973a502539f9 Mon Sep 17 00:00:00 2001 From: Ellyse <141240083+ellyxir@users.noreply.github.com> Date: Fri, 19 Sep 2025 21:54:30 +0200 Subject: [PATCH 2/2] simplify patterns for charm references in cell (#1799) * simplify a bit, removed uneccesasary lifts, use Default to create cell instead of calling cell() although i still create isInitialized from cell(), not sure how to turn that into a default via the json schema if thats possible. improved comments * use toSchema and ifElse to simplify code * set the sub charms name so its easier distinguish * undefined is not legal in JSONSchema, changed cellRef to default to null instead --- packages/patterns/charm-ref-in-cell.tsx | 184 +++++++++++------------- 1 file changed, 87 insertions(+), 97 deletions(-) diff --git a/packages/patterns/charm-ref-in-cell.tsx b/packages/patterns/charm-ref-in-cell.tsx index 133305a789..55ebe4d797 100644 --- a/packages/patterns/charm-ref-in-cell.tsx +++ b/packages/patterns/charm-ref-in-cell.tsx @@ -3,73 +3,55 @@ import { Cell, cell, createCell, + Default, derive, h, handler, + ifElse, lift, NAME, navigateTo, recipe, + toSchema, UI, } from "commontools"; +// full recipe state +interface RecipeState { + charm: any; + cellRef: Cell<{ charm: any }>; + isInitialized: Cell; +} +const RecipeStateSchema = toSchema(); + +// what we pass into the recipe as input +// wraps the charm reference in an object { charm: any } +// instead of storing the charm directly. This avoids a "pointer of pointers" +// error that occurs when a Cell directly contains another Cell/charm reference. +type RecipeInOutput = { + cellRef: Default<{ charm: any }, { charm: null }>; +}; + // the simple charm (to which we'll store a reference within a cell) -const SimpleRecipe = recipe("Simple Recipe", () => ({ - [NAME]: "Some Simple Recipe", - [UI]:
Some Simple Recipe
, +const SimpleRecipe = recipe<{ id: string }>("Simple Recipe", ({ id }) => ({ + [NAME]: derive(id, (idValue) => `SimpleRecipe: ${idValue}`), + [UI]:
Simple Recipe id {id}
, })); -// We are going to dynamically create a charm via the `createCounter` function -// and store it (the reference to it) in a cell. We create the cell here. -// There are a few ways to do this: -// - Default values -// - cell() -// - createCell within a lift or derive (we'll use this for now) -// Use isInitialized and storedCellRef to ensure we only create the cell once -const createCellRef = lift( - { - type: "object", - properties: { - isInitialized: { type: "boolean", default: false, asCell: true }, - storedCellRef: { type: "object", asCell: true }, - }, - }, - undefined, - ({ isInitialized, storedCellRef }) => { - if (!isInitialized.get()) { - console.log("Creating cellRef"); - const newCellRef = createCell(undefined, "cellRef"); - storedCellRef.set(newCellRef); - isInitialized.set(true); - return { - cellRef: newCellRef, - }; - } else { - console.log("cellRef already initialized"); - } - // If already initialized, return the stored cellRef - return { - cellRef: storedCellRef, - }; - }, -); - -// this will be called whenever charm or cellRef changes -// pass isInitialized to make sure we dont call this each time -// we change cellRef, otherwise creates a loop -// also, we need to only navigateTo if not initialized so that -// the other lifts we created compete and try to -// navigateTo at the same time. -// note there is a separate isInitialized for each created charm +// Lift that stores a charm reference in a cell and navigates to it. +// Triggered when any input changes (charm, cellRef, or isInitialized). +// +// The isInitialized flag prevents infinite loops: +// - Without it: lift runs → sets cellRef → cellRef changes → lift runs again → loop +// - With it: lift runs once → sets isInitialized → subsequent runs skip the logic +// +// Each handler invocation creates its own isInitialized cell, ensuring +// independent tracking for multiple charm creations. +// +// We use a lift() here instead of executing inside of a handler because +// we want to know the passed in charm is initialized const storeCharmAndNavigate = lift( - { - type: "object", - properties: { - charm: { type: "object" }, - cellRef: { type: "object", asCell: true }, - isInitialized: { type: "boolean", asCell: true }, - }, - }, + RecipeStateSchema, undefined, ({ charm, cellRef, isInitialized }) => { if (!isInitialized.get()) { @@ -78,7 +60,7 @@ const storeCharmAndNavigate = lift( "storeCharmAndNavigate storing charm:", JSON.stringify(charm), ); - cellRef.set(charm); + cellRef.set({ charm }); isInitialized.set(true); return navigateTo(charm); } else { @@ -91,17 +73,19 @@ const storeCharmAndNavigate = lift( }, ); -// create a simple subrecipe -// we will save a reference to it in a cell so make it as simple as -// possible. -// we then call navigateTo() which will redirect the -// browser to the newly created charm -const createSimpleRecipe = handler }>( +// Handler that creates a new charm instance and stores its reference. +// 1. Creates a local isInitialized cell to track one-time execution +// 2. Instantiates SimpleRecipe charm +// 3. Uses storeCharmAndNavigate lift to save reference and navigate +const createSimpleRecipe = handler }>( (_, { cellRef }) => { const isInitialized = cell(false); + // Create a random 5-digit ID + const randomId = Math.floor(10000 + Math.random() * 90000).toString(); + // create the charm - const charm = SimpleRecipe({}); + const charm = SimpleRecipe({ id: randomId }); // store the charm ref in a cell (pass isInitialized to prevent recursive calls) return storeCharmAndNavigate({ charm, cellRef, isInitialized }); @@ -109,46 +93,52 @@ const createSimpleRecipe = handler }>( ); // Handler to navigate to the stored charm (just console.log for now) -const goToStoredCharm = handler }>( +const goToStoredCharm = handler }>( (_, { cellRef }) => { console.log("goToStoredCharm clicked"); - return navigateTo(cellRef); + const cellValue = cellRef.get(); + if (!cellValue.charm) { + console.error("No charm found in cell!"); + return; + } + return navigateTo(cellValue.charm); }, ); -// create the named cell inside the recipe body, so we do it just once -export default recipe("Launcher", () => { - // cell to store to the last charm we created - const { cellRef } = createCellRef({ - isInitialized: cell(false), - storedCellRef: cell(), - }); - - return { - [NAME]: "Launcher", - [UI]: ( -
+export default recipe( + "Launcher", + ({ cellRef }) => { + return { + [NAME]: "Launcher", + [UI]: (
- Stored charm ID: {derive(cellRef, (innerCell) => { - if (!innerCell) return "undefined"; - return innerCell[UI]; - })} +
+ Stored charm ID: {derive(cellRef, (innerCell) => { + if (!innerCell) return "undefined"; + if (!innerCell.charm) return "no charm stored yet"; + return innerCell.charm[UI] || "charm has no UI"; + })} +
+ + Create Sub Charm + + + {ifElse( + cellRef.charm, + ( + + Go to Stored Charm + + ), + ( +
no subcharm
+ ), + )}
- - Create Sub Charm - - {derive(cellRef, (innerCell) => { - if (!innerCell) return "no subcharm yet!"; - return ( - - Go to Stored Charm - - ); - })} -
- ), - cellRef, - }; -}); + ), + cellRef, + }; + }, +);