-
Notifications
You must be signed in to change notification settings - Fork 1
Add GraphQL mutation engine for TSP type transformations #56
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/type-utils
Are you sure you want to change the base?
Conversation
65f55e7 to
0bc74f1
Compare
17e8959 to
e651f96
Compare
| * GraphQL mutation engine that applies GraphQL-specific transformations | ||
| * to TypeSpec types, such as name sanitization. | ||
| */ | ||
| export class GraphQLMutationEngine { |
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.
As I understand the implementation of the mutation framework, a MutationEngine should capture the mutations for one type of transformation; i.e., we should have a specific GraphQLNamingMutationEngine, and other engines that capture the various different transformations we want to run, rather than a global "GraphQL" engine.
I'm using the HttpCanonicalizer engine as the reference point here — it implements a very specific type of transformation rather than "HTTP" transformations broadly.
It also seems that SimpleMutationEngine might be sufficient for what we need here.
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.
We discussed a bit offline, but just to share in context-- I think that the GraphQL mutations should all share an engine for a couple of reasons:
- We've broken the GraphQL mutations out into Individual "transformations" which is helpful for conceptualizing all of the ways the TSP schema must be altered to match GraphQL conventions. However, individual transformations aren't useful in isolation, they all need to be applied to give us the correct resulting GraphQL schema.
- When we make multiple transformations in the same engine we have the benefit of reusing a shared cache and performing a single traversal of the TSP graph.
- Using one mutation engine will simplify the consumer API when using the GraphQL Mutation Engine from within emitters.
If we find that this approach becomes unwieldy, then we could look into splitting into multiple engines. For now, I think this is the right approach! Open to discussing further if you feel very strongly!
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.
I'm 👍 on moving forward and seeing how it works out. I think it will be important that we set and keep guardrails about what does and doesn't go into the "GraphQL" mutation engine — e.g. only things that are backed up by some part of the GraphQL spec.
8dbe441 to
f5fca8d
Compare
e651f96 to
04ae5e7
Compare
04ae5e7 to
83f1ddc
Compare
| this.#mutationNode.whenMutated((member) => { | ||
| if (member) { | ||
| member.name = sanitizeNameForGraphQL(member.name); | ||
| } | ||
| }); |
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.
Help me understand why we're taking this approach for enum members and model properties, but not any of the other types.
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.
I'm taking this approach so that we can hook into whenMutated at construction time of these child nodes and ensure their names are sanitized before the parent edge callbacks need them!
Summary
This PR introduces a GraphQL mutation engine that leverages the
@typespec/mutator-frameworkto transform TypeSpec types for GraphQL schema generation. This PR establishes the foundation with name sanitization, but the architecture will support additional transformations (e.g., input/output type splitting, visibility filtering) in future PRs.What it does
The mutation engine wraps the
mutator-framework'sMutationEnginewith GraphQL-specific mutation classes. These classes intercept type graph traversal to apply GraphQL-specific transformations.Current transformation: Name sanitization ensures all type, field, and member names are valid GraphQL identifiers (e.g.,
$Invalid$→_Invalid_,my-field→my_field).Future transformations (not in this PR):
Architecture
GraphQLMutationEngineMutationEngineGraphQLEnumMutationGraphQLEnumMemberMutationGraphQLModelMutationGraphQLModelPropertyMutationGraphQLOperationMutationGraphQLScalarMutationChanges
New Files:
src/mutation-engine/engine.ts- Core engine wrappingMutationEnginesrc/mutation-engine/options.ts- GraphQL-specific mutation optionssrc/mutation-engine/mutations/*.ts- 6 mutation classes for different type kindstest/mutation-engine/graphql-mutation-engine.test.ts- unit testsUsage