-
Notifications
You must be signed in to change notification settings - Fork 1
Add Input/Output model splitting to GraphQL mutation engine #60
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: fionabronwen/emitter-integration
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,35 @@ | ||
| import { UsageFlags } from "@typespec/compiler"; | ||
| import { SimpleMutationOptions } from "@typespec/mutator-framework"; | ||
|
|
||
| /** | ||
| * GraphQL-specific mutation options. | ||
| * | ||
| * Currently a simple wrapper around SimpleMutationOptions. | ||
| * Can be extended in the future to support additional GraphQL-specific options. | ||
| * Extends SimpleMutationOptions with usage-aware mutation key support, | ||
| * enabling separate mutations for input vs output type variants. | ||
| */ | ||
| export class GraphQLMutationOptions extends SimpleMutationOptions {} | ||
| export class GraphQLMutationOptions extends SimpleMutationOptions { | ||
| /** | ||
| * The usage flag indicating whether this mutation is for input or output usage. | ||
| * Used to generate separate mutations for the same type when used in both contexts. | ||
| */ | ||
| readonly usageFlag: UsageFlags; | ||
|
|
||
| constructor(usageFlag: UsageFlags = UsageFlags.None) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's valid for me to do new GraphQLMutationOptions(usageFlag: UsageFlags.Input | UsageFlags.Output);or any other combination of the flags. So we either need to account for this in |
||
| super(); | ||
| this.usageFlag = usageFlag; | ||
| } | ||
|
|
||
| /** | ||
| * Override mutationKey to include usage flag. | ||
| * This ensures the mutation engine caches separate mutations for input vs output variants. | ||
| */ | ||
| override get mutationKey(): string { | ||
| const baseKey = super.mutationKey; | ||
| if (this.usageFlag === UsageFlags.Input) { | ||
| return `${baseKey}:input`; | ||
| } else if (this.usageFlag === UsageFlags.Output) { | ||
| return `${baseKey}:output`; | ||
| } | ||
| return baseKey; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,8 @@ import { | |
| GraphQLString, | ||
| type GraphQLFieldConfigMap, | ||
| type GraphQLInputFieldConfigMap, | ||
| type GraphQLInputType, | ||
| type GraphQLOutputType, | ||
| } from "graphql"; | ||
| import { TypeMap, type TSPContext, type TypeKey } from "../type-maps.js"; | ||
|
|
||
|
|
@@ -32,42 +34,55 @@ export class ModelTypeMap extends TypeMap<Model, GraphQLObjectType | GraphQLInpu | |
|
|
||
| // Create InputObjectType for input usage, ObjectType for output | ||
| if (context.usageFlag === UsageFlags.Input) { | ||
| return this.materializeInputType(name, tspModel); | ||
| return this.materializeInputType(tspModel, name); | ||
| } | ||
| return this.materializeOutputType(name, tspModel); | ||
| return this.materializeOutputType(tspModel, name); | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need to pass the |
||
|
|
||
| private materializeOutputType(name: string, tspModel: Model): GraphQLObjectType { | ||
| /** | ||
| * Materialize as a GraphQLObjectType (output type). | ||
| */ | ||
| private materializeOutputType(tspModel: Model, name: string): GraphQLObjectType { | ||
| const fields: GraphQLFieldConfigMap<unknown, unknown> = {}; | ||
|
|
||
| for (const [propName, prop] of tspModel.properties) { | ||
| fields[propName] = { | ||
| type: this.mapPropertyType(prop.type), | ||
| // TODO: Add description from doc comments | ||
| type: this.mapOutputType(prop.type), | ||
| }; | ||
| } | ||
|
|
||
| return new GraphQLObjectType({ name, fields }); | ||
| } | ||
|
|
||
| private materializeInputType(name: string, tspModel: Model): GraphQLInputObjectType { | ||
| /** | ||
| * Materialize as a GraphQLInputObjectType (input type). | ||
| */ | ||
| private materializeInputType(tspModel: Model, name: string): GraphQLInputObjectType { | ||
| const fields: GraphQLInputFieldConfigMap = {}; | ||
|
|
||
| for (const [propName, prop] of tspModel.properties) { | ||
| fields[propName] = { | ||
| type: this.mapPropertyType(prop.type), | ||
| // TODO: Add description from doc comments | ||
| type: this.mapInputType(prop.type), | ||
| }; | ||
| } | ||
|
|
||
| return new GraphQLInputObjectType({ name, fields }); | ||
| } | ||
|
|
||
| /** | ||
| * Map a TypeSpec property type to a GraphQL String type. | ||
| * Map a TypeSpec property type to a GraphQL output type. | ||
| * TODO: Implement full type mapping with references to other registered types. | ||
| */ | ||
| private mapOutputType(_type: unknown): GraphQLOutputType { | ||
| // Placeholder - will need to resolve references to other types | ||
| return GraphQLString; | ||
| } | ||
|
|
||
| /** | ||
| * Map a TypeSpec property type to a GraphQL input type. | ||
| * TODO: Implement full type mapping with references to other registered types. | ||
| */ | ||
| private mapPropertyType(_type: unknown): typeof GraphQLString { | ||
| private mapInputType(_type: unknown): GraphQLInputType { | ||
| // Placeholder - will need to resolve references to other types | ||
| return GraphQLString; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,4 +24,7 @@ namespace MyLibrary { | |
| Mystery, | ||
| Fantasy, | ||
| } | ||
|
|
||
| op getBooks(): Book[]; | ||
| op createBook(book: Book): Book; | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is an expensive operation so we probably don't want to do it in the constructor.
Expensive calls can be made lazily; i.e. if we ask for usage information (in
getUsage) and we have not yet resolved usages, we do so (and store the result). Callers should also have the flexibility to trigger the resolve explicitly to optimize their call flow.