From f2f7f4013544cd447aaf7d71ca6262fd58b3f295 Mon Sep 17 00:00:00 2001 From: Fiona Date: Mon, 7 Jul 2025 14:49:58 -0400 Subject: [PATCH 1/4] Add denormalization class and test --- packages/graphql/src/denormalization.ts | 136 +++++++ packages/graphql/test/denormalization.test.ts | 364 ++++++++++++++++++ 2 files changed, 500 insertions(+) create mode 100644 packages/graphql/src/denormalization.ts create mode 100644 packages/graphql/test/denormalization.test.ts diff --git a/packages/graphql/src/denormalization.ts b/packages/graphql/src/denormalization.ts new file mode 100644 index 00000000000..d46e1a974b8 --- /dev/null +++ b/packages/graphql/src/denormalization.ts @@ -0,0 +1,136 @@ +import type { Model, ModelProperty, Namespace } from "@typespec/compiler"; +import { UsageFlags, type EmitContext, type UsageTracker } from "@typespec/compiler"; +import { $ } from "@typespec/compiler/typekit"; + +/** + * Provides utilities to denormalize TypeSpec (TSP) model types for GraphQL emitters. + * + * This class analyzes model usage and creates GraphQL-compliant input model variants + * (e.g., FooInput) for models used as input types. It mutates the provided namespace + * in-place, recursively denormalizing nested model properties as needed. + * + * Name collisions will throw a JavaScript Error (consider using TypeSpec diagnostics in the future). + * Optionally, a debug flag will print a mapping of original models to their denormalized variants. + * + * Example usage: + * ```typescript + * const usageTracker = resolveUsages(namespace); + * GraphQLDenormalizer.denormalize(namespace, usageTracker, context, true); // debug output enabled + * ``` + */ +export class GraphQLDenormalizer { + /** + * Denormalizes TSP model types for GraphQL input usage. + * + * - Mutates the provided namespace in-place. + * - Recursively creates input model variants (e.g., FooInput) for models used as input types. + * - Throws a JavaScript Error for name collisions (TODO: use TypeSpec diagnostics). + * - Optionally logs a debug mapping of original models to their denormalized variants. + * + * @param namespace The TypeSpec namespace to mutate. + * @param usageTracker UsageTracker for determining input usage. + * @param context The TypeSpec emit context. + * @param debug If true, logs a mapping of original to denormalized models. + */ + static denormalize( + namespace: Namespace, + usageTracker: UsageTracker, + context: EmitContext, + debug: boolean = false, + ): void { + if (debug) { + // eslint-disable-next-line no-console + console.log(`[GraphQLDenormalizer] Starting denormalization for namespace: ${namespace.name}`); + } + for (const [_, model] of namespace.models) { + this.expandInputOutputTypes(model, usageTracker, context, namespace, debug); + // ...other steps will be called here in the future... + } + } + + /** + * Expands input/output types by creating GraphQL-compliant input model variants (e.g., FooInput) + * for models used as input types. Mutates the namespace in-place. + * Throws on name collisions. Debug output prints mapping from TSP Model to TSP Model variants. + */ + public static expandInputOutputTypes( + model: Model, + usageTracker: UsageTracker, + context: EmitContext, + namespace: Namespace, + debug: boolean + ) { + const typekit = $(context.program); + // Only process if this model is used as input + if (!usageTracker.isUsedAs(model, UsageFlags.Input)) return; + const inputName = model.name + "Input"; + if (namespace.models.has(inputName)) { + throw new Error(`Model name collision: ${inputName} already exists in namespace.`); + } + // Helper to recursively denormalize nested model properties + function getInputType(type: any): any { + if (type.kind === "Model" && usageTracker.isUsedAs(type, UsageFlags.Input)) { + const nestedInputName = type.name + "Input"; + if (namespace.models.has(nestedInputName)) { + return namespace.models.get(nestedInputName); + } + const inputModel = GraphQLDenormalizer.createInputModelVariant(type, typekit, getInputType); + if (namespace.models.has(nestedInputName)) { + throw new Error(`Model name collision: ${nestedInputName} already exists in namespace.`); + } + namespace.models.set(nestedInputName, inputModel); + if (debug) { + // eslint-disable-next-line no-console + console.log(`[GraphQLDenormalizer] Created input model: ${type.name} -> ${nestedInputName}`); + } + return inputModel; + } + return type; + } + const inputModel = GraphQLDenormalizer.createInputModelVariant(model, typekit, getInputType); + namespace.models.set(inputName, inputModel); + if (debug) { + // eslint-disable-next-line no-console + console.log(`[GraphQLDenormalizer] Created input model: ${model.name} -> ${inputName}`); + } + } + + /** + * Creates an input model variant from an output model. + * Recursively transforms nested model types to their input variants using getInputType. + */ + public static createInputModelVariant( + outputModel: Model, + typekit: ReturnType, + getInputType: (type: any) => any, + ): Model { + const inputProperties: Record = {}; + for (const [name, prop] of outputModel.properties) { + inputProperties[name] = typekit.modelProperty.create({ + name: prop.name, + type: getInputType(prop.type), + optional: prop.optional, + }); + } + const inputModel = typekit.model.create({ + name: outputModel.name + "Input", + properties: inputProperties, + }); + for (const [_, prop] of inputModel.properties) { + (prop as any).model = inputModel; + } + return inputModel; + } + + // --- Stubs for future denormalization steps --- + public static resolveDecorators(model: Model, context: EmitContext, debug: boolean) {} + public static renameIdentifiers(model: Model, context: EmitContext, debug: boolean) {} + public static transformBasicTypes(model: Model, context: EmitContext, debug: boolean) {} + public static transformIntrinsicTypes(model: Model, context: EmitContext, debug: boolean) {} + public static transformRecordTypes(model: Model, context: EmitContext, debug: boolean) {} + public static deanonymizeUnions(model: Model, context: EmitContext, debug: boolean) {} + public static materializeRootOperationTypes(model: Model, context: EmitContext, debug: boolean) {} + public static resolveSubschemaSelection(model: Model, context: EmitContext, debug: boolean) {} + public static makeFieldsOptionalByDefault(model: Model, context: EmitContext, debug: boolean) {} + public static removeNullFromUnions(model: Model, context: EmitContext, debug: boolean) {} +} diff --git a/packages/graphql/test/denormalization.test.ts b/packages/graphql/test/denormalization.test.ts new file mode 100644 index 00000000000..59c6ac0c842 --- /dev/null +++ b/packages/graphql/test/denormalization.test.ts @@ -0,0 +1,364 @@ +import type { Model, Namespace } from "@typespec/compiler"; +import { UsageFlags } from "@typespec/compiler"; +import { expectDiagnosticEmpty } from "@typespec/compiler/testing"; +import { describe, expect, it } from "vitest"; +import { GraphQLDenormalizer } from "../src/denormalization.js"; +import { compileAndDiagnose } from "./test-host.js"; + +describe("GraphQLDenormalizer", () => { + describe("expandInputOutputTypes", () => { + it("should create input model variant for models used as input", async () => { + const [program, { TestNamespace }, diagnostics] = await compileAndDiagnose<{ + TestNamespace: Namespace; + }>(` + @test namespace TestNamespace { + model User { + name: string; + age: int32; + } + } + `); + expectDiagnosticEmpty(diagnostics); + + // Create a mock usage tracker that marks User as used for input + const mockUsageTracker = { + isUsedAs: (model: Model, flag: UsageFlags) => { + if (model.name === "User" && flag === UsageFlags.Input) { + return true; + } + return false; + } + }; + + const userModel = TestNamespace.models.get("User")!; + expect(userModel).toBeDefined(); + expect(TestNamespace.models.has("UserInput")).toBe(false); + + // Run denormalization + GraphQLDenormalizer.expandInputOutputTypes( + userModel, + mockUsageTracker as any, + { program } as any, + TestNamespace, + false // no debug output + ); + + // Check that UserInput was created + expect(TestNamespace.models.has("UserInput")).toBe(true); + const userInputModel = TestNamespace.models.get("UserInput")!; + expect(userInputModel.name).toBe("UserInput"); + expect(userInputModel.properties.size).toBe(2); + expect(userInputModel.properties.has("name")).toBe(true); + expect(userInputModel.properties.has("age")).toBe(true); + }); + + it("should skip models not used as input", async () => { + const [program, { TestNamespace }, diagnostics] = await compileAndDiagnose<{ + TestNamespace: Namespace; + }>(` + @test namespace TestNamespace { + model User { + name: string; + age: int32; + } + } + `); + expectDiagnosticEmpty(diagnostics); + + // Create a mock usage tracker that does NOT mark User as used for input + const mockUsageTracker = { + isUsedAs: (model: Model, flag: UsageFlags) => { + return false; // User is not used as input + } + }; + + const userModel = TestNamespace.models.get("User")!; + expect(userModel).toBeDefined(); + expect(TestNamespace.models.has("UserInput")).toBe(false); + + // Run denormalization + GraphQLDenormalizer.expandInputOutputTypes( + userModel, + mockUsageTracker as any, + { program } as any, + TestNamespace, + false + ); + + // Check that UserInput was NOT created + expect(TestNamespace.models.has("UserInput")).toBe(false); + }); + + it("should handle recursive denormalization of nested models", async () => { + const [program, { TestNamespace }, diagnostics] = await compileAndDiagnose<{ + TestNamespace: Namespace; + }>(` + @test namespace TestNamespace { + model Address { + street: string; + city: string; + } + + model User { + name: string; + address: Address; + } + } + `); + expectDiagnosticEmpty(diagnostics); + + // Create a mock usage tracker that marks both User and Address as used for input + const mockUsageTracker = { + isUsedAs: (model: Model, flag: UsageFlags) => { + if (flag === UsageFlags.Input && (model.name === "User" || model.name === "Address")) { + return true; + } + return false; + } + }; + + const userModel = TestNamespace.models.get("User")!; + const addressModel = TestNamespace.models.get("Address")!; + expect(userModel).toBeDefined(); + expect(addressModel).toBeDefined(); + expect(TestNamespace.models.has("UserInput")).toBe(false); + expect(TestNamespace.models.has("AddressInput")).toBe(false); + + // Run denormalization on User (should recursively handle Address) + GraphQLDenormalizer.expandInputOutputTypes( + userModel, + mockUsageTracker as any, + { program } as any, + TestNamespace, + false + ); + + // Check that both UserInput and AddressInput were created + expect(TestNamespace.models.has("UserInput")).toBe(true); + expect(TestNamespace.models.has("AddressInput")).toBe(true); + + const userInputModel = TestNamespace.models.get("UserInput")!; + const addressInputModel = TestNamespace.models.get("AddressInput")!; + + // Verify UserInput has correct properties + expect(userInputModel.name).toBe("UserInput"); + expect(userInputModel.properties.size).toBe(2); + expect(userInputModel.properties.has("name")).toBe(true); + expect(userInputModel.properties.has("address")).toBe(true); + + // Verify AddressInput has correct properties + expect(addressInputModel.name).toBe("AddressInput"); + expect(addressInputModel.properties.size).toBe(2); + expect(addressInputModel.properties.has("street")).toBe(true); + expect(addressInputModel.properties.has("city")).toBe(true); + }); + + it("should throw error on name collision", async () => { + const [program, { TestNamespace }, diagnostics] = await compileAndDiagnose<{ + TestNamespace: Namespace; + }>(` + @test namespace TestNamespace { + model User { + name: string; + } + + model UserInput { + name: string; + } + } + `); + expectDiagnosticEmpty(diagnostics); + + // Create a mock usage tracker that marks User as used for input + const mockUsageTracker = { + isUsedAs: (model: Model, flag: UsageFlags) => { + if (model.name === "User" && flag === UsageFlags.Input) { + return true; + } + return false; + } + }; + + const userModel = TestNamespace.models.get("User")!; + expect(userModel).toBeDefined(); + expect(TestNamespace.models.has("UserInput")).toBe(true); // Already exists + + // Run denormalization - should throw error due to name collision + expect(() => { + GraphQLDenormalizer.expandInputOutputTypes( + userModel, + mockUsageTracker as any, + { program } as any, + TestNamespace, + false + ); + }).toThrow("Model name collision: UserInput already exists in namespace."); + }); + + it("should handle models with optional properties", async () => { + const [program, { TestNamespace }, diagnostics] = await compileAndDiagnose<{ + TestNamespace: Namespace; + }>(` + @test namespace TestNamespace { + model User { + name: string; + email?: string; + age?: int32; + } + } + `); + expectDiagnosticEmpty(diagnostics); + + // Create a mock usage tracker that marks User as used for input + const mockUsageTracker = { + isUsedAs: (model: Model, flag: UsageFlags) => { + if (model.name === "User" && flag === UsageFlags.Input) { + return true; + } + return false; + } + }; + + const userModel = TestNamespace.models.get("User")!; + expect(userModel).toBeDefined(); + + // Run denormalization + GraphQLDenormalizer.expandInputOutputTypes( + userModel, + mockUsageTracker as any, + { program } as any, + TestNamespace, + false + ); + + // Check that UserInput was created with correct optional properties + expect(TestNamespace.models.has("UserInput")).toBe(true); + const userInputModel = TestNamespace.models.get("UserInput")!; + + expect(userInputModel.name).toBe("UserInput"); + expect(userInputModel.properties.size).toBe(3); + + const nameProperty = userInputModel.properties.get("name")!; + const emailProperty = userInputModel.properties.get("email")!; + const ageProperty = userInputModel.properties.get("age")!; + + expect(nameProperty.optional).toBe(false); + expect(emailProperty.optional).toBe(true); + expect(ageProperty.optional).toBe(true); + }); + }); + + describe("denormalize", () => { + it("should denormalize all models in namespace that are used as input", async () => { + const [program, { TestNamespace }, diagnostics] = await compileAndDiagnose<{ + TestNamespace: Namespace; + }>(` + @test namespace TestNamespace { + model User { + name: string; + age: int32; + } + + model Book { + title: string; + author: string; + } + + model ReadOnlyModel { + id: string; + } + } + `); + expectDiagnosticEmpty(diagnostics); + + // Create a mock usage tracker + const mockUsageTracker = { + isUsedAs: (model: Model, flag: UsageFlags) => { + if (flag === UsageFlags.Input && (model.name === "User" || model.name === "Book")) { + return true; + } + return false; + } + }; + + expect(TestNamespace.models.has("UserInput")).toBe(false); + expect(TestNamespace.models.has("BookInput")).toBe(false); + expect(TestNamespace.models.has("ReadOnlyModelInput")).toBe(false); + + // Run denormalization + GraphQLDenormalizer.denormalize( + TestNamespace, + mockUsageTracker as any, + { program } as any, + false // no debug output + ); + + // Check that input models were created for User and Book, but not ReadOnlyModel + expect(TestNamespace.models.has("UserInput")).toBe(true); + expect(TestNamespace.models.has("BookInput")).toBe(true); + expect(TestNamespace.models.has("ReadOnlyModelInput")).toBe(false); + + const userInputModel = TestNamespace.models.get("UserInput")!; + const bookInputModel = TestNamespace.models.get("BookInput")!; + + expect(userInputModel.name).toBe("UserInput"); + expect(userInputModel.properties.size).toBe(2); + + expect(bookInputModel.name).toBe("BookInput"); + expect(bookInputModel.properties.size).toBe(2); + }); + }); + + describe("createInputModelVariant", () => { + it("should create input model variant with correct properties", async () => { + const [program, { TestNamespace }, diagnostics] = await compileAndDiagnose<{ + TestNamespace: Namespace; + }>(` + @test namespace TestNamespace { + model User { + name: string; + age: int32; + active: boolean; + } + } + `); + expectDiagnosticEmpty(diagnostics); + + const userModel = TestNamespace.models.get("User")!; + expect(userModel).toBeDefined(); + + // Mock typekit + const mockTypekit = { + modelProperty: { + create: (props: any) => ({ + name: props.name, + type: props.type, + optional: props.optional, + }), + }, + model: { + create: (props: any) => ({ + name: props.name, + properties: new Map(Object.entries(props.properties)), + }), + }, + }; + + // Mock getInputType function + const getInputType = (type: any) => type; + + // Create input model variant + const inputModel = GraphQLDenormalizer.createInputModelVariant( + userModel, + mockTypekit as any, + getInputType + ); + + expect(inputModel.name).toBe("UserInput"); + expect(inputModel.properties.size).toBe(3); + expect(inputModel.properties.has("name")).toBe(true); + expect(inputModel.properties.has("age")).toBe(true); + expect(inputModel.properties.has("active")).toBe(true); + }); + }); +}); From 7d32b29d3a25eba9eb0874275c6b502561655653 Mon Sep 17 00:00:00 2001 From: Fiona Date: Mon, 7 Jul 2025 14:55:52 -0400 Subject: [PATCH 2/4] Cleanup --- packages/graphql/src/denormalization.ts | 32 ++++--------------------- 1 file changed, 4 insertions(+), 28 deletions(-) diff --git a/packages/graphql/src/denormalization.ts b/packages/graphql/src/denormalization.ts index d46e1a974b8..324f5fa1383 100644 --- a/packages/graphql/src/denormalization.ts +++ b/packages/graphql/src/denormalization.ts @@ -4,12 +4,6 @@ import { $ } from "@typespec/compiler/typekit"; /** * Provides utilities to denormalize TypeSpec (TSP) model types for GraphQL emitters. - * - * This class analyzes model usage and creates GraphQL-compliant input model variants - * (e.g., FooInput) for models used as input types. It mutates the provided namespace - * in-place, recursively denormalizing nested model properties as needed. - * - * Name collisions will throw a JavaScript Error (consider using TypeSpec diagnostics in the future). * Optionally, a debug flag will print a mapping of original models to their denormalized variants. * * Example usage: @@ -22,11 +16,6 @@ export class GraphQLDenormalizer { /** * Denormalizes TSP model types for GraphQL input usage. * - * - Mutates the provided namespace in-place. - * - Recursively creates input model variants (e.g., FooInput) for models used as input types. - * - Throws a JavaScript Error for name collisions (TODO: use TypeSpec diagnostics). - * - Optionally logs a debug mapping of original models to their denormalized variants. - * * @param namespace The TypeSpec namespace to mutate. * @param usageTracker UsageTracker for determining input usage. * @param context The TypeSpec emit context. @@ -38,19 +27,16 @@ export class GraphQLDenormalizer { context: EmitContext, debug: boolean = false, ): void { - if (debug) { - // eslint-disable-next-line no-console - console.log(`[GraphQLDenormalizer] Starting denormalization for namespace: ${namespace.name}`); - } for (const [_, model] of namespace.models) { this.expandInputOutputTypes(model, usageTracker, context, namespace, debug); - // ...other steps will be called here in the future... + // TODO: Call methods for additional denormalization steps such as resolving decorators, de-anonymizing unions, etc. + } } /** * Expands input/output types by creating GraphQL-compliant input model variants (e.g., FooInput) - * for models used as input types. Mutates the namespace in-place. + * for models used as input types. Mutates the TypeSpec namespace in-place. * Throws on name collisions. Debug output prints mapping from TSP Model to TSP Model variants. */ public static expandInputOutputTypes( @@ -122,15 +108,5 @@ export class GraphQLDenormalizer { return inputModel; } - // --- Stubs for future denormalization steps --- - public static resolveDecorators(model: Model, context: EmitContext, debug: boolean) {} - public static renameIdentifiers(model: Model, context: EmitContext, debug: boolean) {} - public static transformBasicTypes(model: Model, context: EmitContext, debug: boolean) {} - public static transformIntrinsicTypes(model: Model, context: EmitContext, debug: boolean) {} - public static transformRecordTypes(model: Model, context: EmitContext, debug: boolean) {} - public static deanonymizeUnions(model: Model, context: EmitContext, debug: boolean) {} - public static materializeRootOperationTypes(model: Model, context: EmitContext, debug: boolean) {} - public static resolveSubschemaSelection(model: Model, context: EmitContext, debug: boolean) {} - public static makeFieldsOptionalByDefault(model: Model, context: EmitContext, debug: boolean) {} - public static removeNullFromUnions(model: Model, context: EmitContext, debug: boolean) {} + // TODO: Add methods for additional denormalization steps such as resolving decorators, de-anonymizing unions, etc. } From eaf2f0fa458ce59ab052e121e9825454fd5a1fcb Mon Sep 17 00:00:00 2001 From: Fiona Date: Mon, 7 Jul 2025 16:28:30 -0400 Subject: [PATCH 3/4] Refactor class and handle recursive model property input types --- packages/graphql/src/denormalization.ts | 117 ++++--- packages/graphql/test/denormalization.test.ts | 302 ++++++++---------- 2 files changed, 195 insertions(+), 224 deletions(-) diff --git a/packages/graphql/src/denormalization.ts b/packages/graphql/src/denormalization.ts index 324f5fa1383..a6828746a16 100644 --- a/packages/graphql/src/denormalization.ts +++ b/packages/graphql/src/denormalization.ts @@ -1,5 +1,5 @@ import type { Model, ModelProperty, Namespace } from "@typespec/compiler"; -import { UsageFlags, type EmitContext, type UsageTracker } from "@typespec/compiler"; +import { UsageFlags, resolveUsages, type EmitContext, type UsageTracker } from "@typespec/compiler"; import { $ } from "@typespec/compiler/typekit"; /** @@ -8,73 +8,90 @@ import { $ } from "@typespec/compiler/typekit"; * * Example usage: * ```typescript - * const usageTracker = resolveUsages(namespace); - * GraphQLDenormalizer.denormalize(namespace, usageTracker, context, true); // debug output enabled + * const denormalizer = new GraphQLTSPDenormalizer(namespace, context); + * denormalizer.denormalize(true); // with debug output * ``` */ -export class GraphQLDenormalizer { - /** - * Denormalizes TSP model types for GraphQL input usage. - * - * @param namespace The TypeSpec namespace to mutate. - * @param usageTracker UsageTracker for determining input usage. - * @param context The TypeSpec emit context. - * @param debug If true, logs a mapping of original to denormalized models. - */ - static denormalize( - namespace: Namespace, - usageTracker: UsageTracker, - context: EmitContext, - debug: boolean = false, - ): void { - for (const [_, model] of namespace.models) { - this.expandInputOutputTypes(model, usageTracker, context, namespace, debug); - // TODO: Call methods for additional denormalization steps such as resolving decorators, de-anonymizing unions, etc. +export class GraphQLTSPDenormalizer { + private usageTracker: UsageTracker; + private namespace: Namespace; + private context: EmitContext; + + constructor(namespace: Namespace, context: EmitContext) { + this.namespace = namespace; + this.context = context; + this.usageTracker = resolveUsages(namespace); + } + denormalize(debug: boolean = false): void { + for (const [_, model] of this.namespace.models) { + this.expandInputOutputTypes(model, debug); + // TODO: Call methods for additional denormalization steps such as resolving decorators, de-anonymizing unions, etc. } } /** - * Expands input/output types by creating GraphQL-compliant input model variants (e.g., FooInput) - * for models used as input types. Mutates the TypeSpec namespace in-place. - * Throws on name collisions. Debug output prints mapping from TSP Model to TSP Model variants. + * Creates an input variant for a model if it's used as input (e.g., User -> UserInput). + * Recursively processes nested models. Mutates namespace in-place. + * Throws on name collisions. */ - public static expandInputOutputTypes( - model: Model, - usageTracker: UsageTracker, - context: EmitContext, - namespace: Namespace, - debug: boolean - ) { - const typekit = $(context.program); + expandInputOutputTypes(model: Model, debug: boolean) { + const typekit = $(this.context.program); // Only process if this model is used as input - if (!usageTracker.isUsedAs(model, UsageFlags.Input)) return; + if (!this.usageTracker.isUsedAs(model, UsageFlags.Input)) return; const inputName = model.name + "Input"; - if (namespace.models.has(inputName)) { + if (this.namespace.models.has(inputName)) { throw new Error(`Model name collision: ${inputName} already exists in namespace.`); } - // Helper to recursively denormalize nested model properties - function getInputType(type: any): any { - if (type.kind === "Model" && usageTracker.isUsedAs(type, UsageFlags.Input)) { + // Recursively transform nested model types to their input variants + const getInputType = (type: any): any => { + if (type.kind === "Model" && this.usageTracker.isUsedAs(type, UsageFlags.Input)) { const nestedInputName = type.name + "Input"; - if (namespace.models.has(nestedInputName)) { - return namespace.models.get(nestedInputName); + if (this.namespace.models.has(nestedInputName)) { + return this.namespace.models.get(nestedInputName); + } + + // Create placeholder model first to prevent recursive creation + const placeholderModel = typekit.model.create({ + name: nestedInputName, + properties: {}, + }); + this.namespace.models.set(nestedInputName, placeholderModel); + + // Now populate the properties with recursive transformation + const inputProperties: Record = {}; + for (const [name, prop] of type.properties) { + inputProperties[name] = typekit.modelProperty.create({ + name: prop.name, + type: getInputType(prop.type), + optional: prop.optional, + }); } - const inputModel = GraphQLDenormalizer.createInputModelVariant(type, typekit, getInputType); - if (namespace.models.has(nestedInputName)) { - throw new Error(`Model name collision: ${nestedInputName} already exists in namespace.`); + + // Create the final input model with all properties + const inputModel = typekit.model.create({ + name: nestedInputName, + properties: inputProperties, + }); + + // Replace the placeholder with the fully populated model + this.namespace.models.set(nestedInputName, inputModel); + for (const [_, prop] of inputModel.properties) { + (prop as any).model = inputModel; } - namespace.models.set(nestedInputName, inputModel); + if (debug) { // eslint-disable-next-line no-console - console.log(`[GraphQLDenormalizer] Created input model: ${type.name} -> ${nestedInputName}`); + console.log( + `[GraphQLDenormalizer] Created input model: ${type.name} -> ${nestedInputName}`, + ); } return inputModel; } return type; - } - const inputModel = GraphQLDenormalizer.createInputModelVariant(model, typekit, getInputType); - namespace.models.set(inputName, inputModel); + }; + const inputModel = this.createInputModelVariant(model, typekit, getInputType); + this.namespace.models.set(inputName, inputModel); if (debug) { // eslint-disable-next-line no-console console.log(`[GraphQLDenormalizer] Created input model: ${model.name} -> ${inputName}`); @@ -82,10 +99,10 @@ export class GraphQLDenormalizer { } /** - * Creates an input model variant from an output model. - * Recursively transforms nested model types to their input variants using getInputType. + * Creates an input model variant with transformed properties. + * Uses getInputType to recursively transform nested model references. */ - public static createInputModelVariant( + private createInputModelVariant( outputModel: Model, typekit: ReturnType, getInputType: (type: any) => any, diff --git a/packages/graphql/test/denormalization.test.ts b/packages/graphql/test/denormalization.test.ts index 59c6ac0c842..3da083662b6 100644 --- a/packages/graphql/test/denormalization.test.ts +++ b/packages/graphql/test/denormalization.test.ts @@ -2,10 +2,61 @@ import type { Model, Namespace } from "@typespec/compiler"; import { UsageFlags } from "@typespec/compiler"; import { expectDiagnosticEmpty } from "@typespec/compiler/testing"; import { describe, expect, it } from "vitest"; -import { GraphQLDenormalizer } from "../src/denormalization.js"; +import { GraphQLTSPDenormalizer } from "../src/denormalization.js"; import { compileAndDiagnose } from "./test-host.js"; -describe("GraphQLDenormalizer", () => { +describe("GraphQLTSPDenormalizer", () => { + describe("denormalize", () => { + it("should denormalize all models in namespace that are used as input", async () => { + const [program, { TestNamespace }, diagnostics] = await compileAndDiagnose<{ + TestNamespace: Namespace; + }>(` + @test namespace TestNamespace { + model User { + name: string; + age: int32; + } + + model Book { + title: string; + author: string; + } + + model ReadOnlyModel { + id: string; + } + + op CreateUser(user: User): User; + op CreateBook(book: Book): Book; + op GetReadOnlyModel(): ReadOnlyModel; + } + `); + expectDiagnosticEmpty(diagnostics); + + expect(TestNamespace.models.has("UserInput")).toBe(false); + expect(TestNamespace.models.has("BookInput")).toBe(false); + expect(TestNamespace.models.has("ReadOnlyModelInput")).toBe(false); + + // Create denormalizer instance and run denormalization + const denormalizer = new GraphQLTSPDenormalizer(TestNamespace, { program } as any); + denormalizer.denormalize(false); // no debug output + + // Check that input models were created for User and Book, but not ReadOnlyModel + expect(TestNamespace.models.has("UserInput")).toBe(true); + expect(TestNamespace.models.has("BookInput")).toBe(true); + expect(TestNamespace.models.has("ReadOnlyModelInput")).toBe(false); + + const userInputModel = TestNamespace.models.get("UserInput")!; + const bookInputModel = TestNamespace.models.get("BookInput")!; + + expect(userInputModel.name).toBe("UserInput"); + expect(userInputModel.properties.size).toBe(2); + + expect(bookInputModel.name).toBe("BookInput"); + expect(bookInputModel.properties.size).toBe(2); + }); + }); + describe("expandInputOutputTypes", () => { it("should create input model variant for models used as input", async () => { const [program, { TestNamespace }, diagnostics] = await compileAndDiagnose<{ @@ -20,8 +71,15 @@ describe("GraphQLDenormalizer", () => { `); expectDiagnosticEmpty(diagnostics); - // Create a mock usage tracker that marks User as used for input - const mockUsageTracker = { + const userModel = TestNamespace.models.get("User")!; + expect(userModel).toBeDefined(); + expect(TestNamespace.models.has("UserInput")).toBe(false); + + // Create denormalizer instance + const denormalizer = new GraphQLTSPDenormalizer(TestNamespace, { program } as any); + + // Override the usage tracker for testing specific behavior + (denormalizer as any).usageTracker = { isUsedAs: (model: Model, flag: UsageFlags) => { if (model.name === "User" && flag === UsageFlags.Input) { return true; @@ -30,18 +88,8 @@ describe("GraphQLDenormalizer", () => { } }; - const userModel = TestNamespace.models.get("User")!; - expect(userModel).toBeDefined(); - expect(TestNamespace.models.has("UserInput")).toBe(false); - - // Run denormalization - GraphQLDenormalizer.expandInputOutputTypes( - userModel, - mockUsageTracker as any, - { program } as any, - TestNamespace, - false // no debug output - ); + // Run denormalization (testing the low-level method for specific behavior) + denormalizer.expandInputOutputTypes(userModel, false); // Check that UserInput was created expect(TestNamespace.models.has("UserInput")).toBe(true); @@ -61,31 +109,22 @@ describe("GraphQLDenormalizer", () => { name: string; age: int32; } + + // No operation uses User as input, so it should be skipped + op GetUser(): User; } `); expectDiagnosticEmpty(diagnostics); - // Create a mock usage tracker that does NOT mark User as used for input - const mockUsageTracker = { - isUsedAs: (model: Model, flag: UsageFlags) => { - return false; // User is not used as input - } - }; - const userModel = TestNamespace.models.get("User")!; expect(userModel).toBeDefined(); expect(TestNamespace.models.has("UserInput")).toBe(false); // Run denormalization - GraphQLDenormalizer.expandInputOutputTypes( - userModel, - mockUsageTracker as any, - { program } as any, - TestNamespace, - false - ); - - // Check that UserInput was NOT created + const denormalizer = new GraphQLTSPDenormalizer(TestNamespace, { program } as any); + denormalizer.expandInputOutputTypes(userModel, false); + + // Check that UserInput was NOT created since User is not used as input expect(TestNamespace.models.has("UserInput")).toBe(false); }); @@ -103,20 +142,14 @@ describe("GraphQLDenormalizer", () => { name: string; address: Address; } + + // Operations that use User and Address as input + op CreateUser(user: User): User; + op UpdateAddress(address: Address): Address; } `); expectDiagnosticEmpty(diagnostics); - // Create a mock usage tracker that marks both User and Address as used for input - const mockUsageTracker = { - isUsedAs: (model: Model, flag: UsageFlags) => { - if (flag === UsageFlags.Input && (model.name === "User" || model.name === "Address")) { - return true; - } - return false; - } - }; - const userModel = TestNamespace.models.get("User")!; const addressModel = TestNamespace.models.get("Address")!; expect(userModel).toBeDefined(); @@ -125,13 +158,8 @@ describe("GraphQLDenormalizer", () => { expect(TestNamespace.models.has("AddressInput")).toBe(false); // Run denormalization on User (should recursively handle Address) - GraphQLDenormalizer.expandInputOutputTypes( - userModel, - mockUsageTracker as any, - { program } as any, - TestNamespace, - false - ); + const denormalizer = new GraphQLTSPDenormalizer(TestNamespace, { program } as any); + denormalizer.expandInputOutputTypes(userModel, false); // Check that both UserInput and AddressInput were created expect(TestNamespace.models.has("UserInput")).toBe(true); @@ -146,6 +174,10 @@ describe("GraphQLDenormalizer", () => { expect(userInputModel.properties.has("name")).toBe(true); expect(userInputModel.properties.has("address")).toBe(true); + // Verify address property on UserInput references the AddressInput model + const addressProperty = userInputModel.properties.get("address")!; + expect(addressProperty.type).toBe(addressInputModel); + // Verify AddressInput has correct properties expect(addressInputModel.name).toBe("AddressInput"); expect(addressInputModel.properties.size).toBe(2); @@ -165,33 +197,21 @@ describe("GraphQLDenormalizer", () => { model UserInput { name: string; } + + // Operation that uses User as input + op CreateUser(user: User): User; } `); expectDiagnosticEmpty(diagnostics); - // Create a mock usage tracker that marks User as used for input - const mockUsageTracker = { - isUsedAs: (model: Model, flag: UsageFlags) => { - if (model.name === "User" && flag === UsageFlags.Input) { - return true; - } - return false; - } - }; - const userModel = TestNamespace.models.get("User")!; expect(userModel).toBeDefined(); expect(TestNamespace.models.has("UserInput")).toBe(true); // Already exists // Run denormalization - should throw error due to name collision + const denormalizer = new GraphQLTSPDenormalizer(TestNamespace, { program } as any); expect(() => { - GraphQLDenormalizer.expandInputOutputTypes( - userModel, - mockUsageTracker as any, - { program } as any, - TestNamespace, - false - ); + denormalizer.expandInputOutputTypes(userModel, false); }).toThrow("Model name collision: UserInput already exists in namespace."); }); @@ -205,31 +225,19 @@ describe("GraphQLDenormalizer", () => { email?: string; age?: int32; } + + // Operation that uses User as input + op CreateUser(user: User): User; } `); expectDiagnosticEmpty(diagnostics); - // Create a mock usage tracker that marks User as used for input - const mockUsageTracker = { - isUsedAs: (model: Model, flag: UsageFlags) => { - if (model.name === "User" && flag === UsageFlags.Input) { - return true; - } - return false; - } - }; - const userModel = TestNamespace.models.get("User")!; expect(userModel).toBeDefined(); // Run denormalization - GraphQLDenormalizer.expandInputOutputTypes( - userModel, - mockUsageTracker as any, - { program } as any, - TestNamespace, - false - ); + const denormalizer = new GraphQLTSPDenormalizer(TestNamespace, { program } as any); + denormalizer.expandInputOutputTypes(userModel, false); // Check that UserInput was created with correct optional properties expect(TestNamespace.models.has("UserInput")).toBe(true); @@ -246,119 +254,65 @@ describe("GraphQLDenormalizer", () => { expect(emailProperty.optional).toBe(true); expect(ageProperty.optional).toBe(true); }); - }); - describe("denormalize", () => { - it("should denormalize all models in namespace that are used as input", async () => { + it("should not create duplicate input models when same model is referenced multiple times", async () => { const [program, { TestNamespace }, diagnostics] = await compileAndDiagnose<{ TestNamespace: Namespace; }>(` @test namespace TestNamespace { - model User { - name: string; - age: int32; + model Address { + street: string; + city: string; } - model Book { - title: string; - author: string; + model User { + name: string; + homeAddress: Address; + workAddress: Address; } - model ReadOnlyModel { - id: string; - } + // Operation that uses User as input + op CreateUser(user: User): User; } `); expectDiagnosticEmpty(diagnostics); - // Create a mock usage tracker - const mockUsageTracker = { - isUsedAs: (model: Model, flag: UsageFlags) => { - if (flag === UsageFlags.Input && (model.name === "User" || model.name === "Book")) { - return true; - } - return false; - } - }; - + const userModel = TestNamespace.models.get("User")!; + expect(userModel).toBeDefined(); expect(TestNamespace.models.has("UserInput")).toBe(false); - expect(TestNamespace.models.has("BookInput")).toBe(false); - expect(TestNamespace.models.has("ReadOnlyModelInput")).toBe(false); + expect(TestNamespace.models.has("AddressInput")).toBe(false); // Run denormalization - GraphQLDenormalizer.denormalize( - TestNamespace, - mockUsageTracker as any, - { program } as any, - false // no debug output - ); + const denormalizer = new GraphQLTSPDenormalizer(TestNamespace, { program } as any); + denormalizer.expandInputOutputTypes(userModel, false); - // Check that input models were created for User and Book, but not ReadOnlyModel + // Check that both UserInput and AddressInput were created (only once each) expect(TestNamespace.models.has("UserInput")).toBe(true); - expect(TestNamespace.models.has("BookInput")).toBe(true); - expect(TestNamespace.models.has("ReadOnlyModelInput")).toBe(false); + expect(TestNamespace.models.has("AddressInput")).toBe(true); const userInputModel = TestNamespace.models.get("UserInput")!; - const bookInputModel = TestNamespace.models.get("BookInput")!; - - expect(userInputModel.name).toBe("UserInput"); - expect(userInputModel.properties.size).toBe(2); + const addressInputModel = TestNamespace.models.get("AddressInput")!; - expect(bookInputModel.name).toBe("BookInput"); - expect(bookInputModel.properties.size).toBe(2); - }); - }); + // Verify UserInput has correct properties + expect(userInputModel.properties.size).toBe(3); + expect(userInputModel.properties.has("name")).toBe(true); + expect(userInputModel.properties.has("homeAddress")).toBe(true); + expect(userInputModel.properties.has("workAddress")).toBe(true); - describe("createInputModelVariant", () => { - it("should create input model variant with correct properties", async () => { - const [program, { TestNamespace }, diagnostics] = await compileAndDiagnose<{ - TestNamespace: Namespace; - }>(` - @test namespace TestNamespace { - model User { - name: string; - age: int32; - active: boolean; - } - } - `); - expectDiagnosticEmpty(diagnostics); - - const userModel = TestNamespace.models.get("User")!; - expect(userModel).toBeDefined(); - - // Mock typekit - const mockTypekit = { - modelProperty: { - create: (props: any) => ({ - name: props.name, - type: props.type, - optional: props.optional, - }), - }, - model: { - create: (props: any) => ({ - name: props.name, - properties: new Map(Object.entries(props.properties)), - }), - }, - }; - - // Mock getInputType function - const getInputType = (type: any) => type; - - // Create input model variant - const inputModel = GraphQLDenormalizer.createInputModelVariant( - userModel, - mockTypekit as any, - getInputType - ); - - expect(inputModel.name).toBe("UserInput"); - expect(inputModel.properties.size).toBe(3); - expect(inputModel.properties.has("name")).toBe(true); - expect(inputModel.properties.has("age")).toBe(true); - expect(inputModel.properties.has("active")).toBe(true); + // Verify AddressInput has correct properties + expect(addressInputModel.properties.size).toBe(2); + expect(addressInputModel.properties.has("street")).toBe(true); + expect(addressInputModel.properties.has("city")).toBe(true); + + // Verify both address properties reference the same AddressInput model + const homeAddressProperty = userInputModel.properties.get("homeAddress")!; + const workAddressProperty = userInputModel.properties.get("workAddress")!; + expect(homeAddressProperty.type).toBe(addressInputModel); + expect(workAddressProperty.type).toBe(addressInputModel); + + // Verify only one AddressInput model was created + const allAddressInputs = Array.from(TestNamespace.models.values()).filter(m => m.name === "AddressInput"); + expect(allAddressInputs.length).toBe(1); }); }); }); From 5e745ddf191856a4c73f082bddef7eb4599384ef Mon Sep 17 00:00:00 2001 From: Fiona Date: Mon, 7 Jul 2025 17:17:32 -0400 Subject: [PATCH 4/4] Use better types --- packages/graphql/src/denormalization.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/packages/graphql/src/denormalization.ts b/packages/graphql/src/denormalization.ts index a6828746a16..1baabc8dd28 100644 --- a/packages/graphql/src/denormalization.ts +++ b/packages/graphql/src/denormalization.ts @@ -1,4 +1,4 @@ -import type { Model, ModelProperty, Namespace } from "@typespec/compiler"; +import type { Model, ModelProperty, Namespace, Type } from "@typespec/compiler"; import { UsageFlags, resolveUsages, type EmitContext, type UsageTracker } from "@typespec/compiler"; import { $ } from "@typespec/compiler/typekit"; @@ -15,9 +15,9 @@ import { $ } from "@typespec/compiler/typekit"; export class GraphQLTSPDenormalizer { private usageTracker: UsageTracker; private namespace: Namespace; - private context: EmitContext; + private context: EmitContext>; - constructor(namespace: Namespace, context: EmitContext) { + constructor(namespace: Namespace, context: EmitContext>) { this.namespace = namespace; this.context = context; this.usageTracker = resolveUsages(namespace); @@ -44,11 +44,11 @@ export class GraphQLTSPDenormalizer { throw new Error(`Model name collision: ${inputName} already exists in namespace.`); } // Recursively transform nested model types to their input variants - const getInputType = (type: any): any => { + const getInputType = (type: Type): Type => { if (type.kind === "Model" && this.usageTracker.isUsedAs(type, UsageFlags.Input)) { const nestedInputName = type.name + "Input"; if (this.namespace.models.has(nestedInputName)) { - return this.namespace.models.get(nestedInputName); + return this.namespace.models.get(nestedInputName)!; } // Create placeholder model first to prevent recursive creation @@ -105,7 +105,7 @@ export class GraphQLTSPDenormalizer { private createInputModelVariant( outputModel: Model, typekit: ReturnType, - getInputType: (type: any) => any, + getInputType: (type: Type) => Type, ): Model { const inputProperties: Record = {}; for (const [name, prop] of outputModel.properties) {