Skip to content

Add TSP Model -> GraphQLObject type translation #31

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

Open
wants to merge 2 commits into
base: feature/graphql
Choose a base branch
from

Conversation

FionaBronwen
Copy link

Summary

This PR adds:

  • Basic handling for emitting GraphQLObjectTypes from TSP Models using the TypeMap abstract class
  • Mapping function TSP -> GraphQL built-in scalars
  • Tests to check for name collisions
  • Updates to use JavaScript's #private properties

Coming Soon in a Followup PR Near You

  • TSP Models -> GraphQLInputTypes

@FionaBronwen FionaBronwen marked this pull request as ready for review June 10, 2025 22:06
/**
* Map TypeSpec scalar types to GraphQL Built-in types
*/
export function mapScalarToGraphQL(

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like https://github.com/microsoft/typespec/blob/ff28640117f7f58166c9455c4812acd7fc044636/packages/http-client-js/src/components/transforms/scalar-transform.tsx establishes a good pattern to follow here.

Parts of that code (like getScalarTests) should arguably part of a shared library — only the scalarTransformerMap is really emitter-specific. Might want to check with @joheredi on that.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah I think scalar-transform really belongs in packages/emitter-framework. There is some type-transform in there but I think that is probably not used much and might want to replace with the transforms from http-client-js.

@maorleger - fyi

Comment on lines +56 to +57
// Type maps for different GraphQL types
#objectTypes: ObjectTypeMap;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we'll want a construct that can return the mapping for any type?
e.g.

#getTypeMap(tspType: Type): TypeMap<Type, Any> {};

* Register a TSP Model
*/
addModel(model: Model): void {
const model_context: TSPContext<Model> = {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the expected style is to use pascalCase.
Python habits are hard to break 🙂

}

const tspEnum = context.tspType as Enum;
// Create a thunk for the property type that will be resolved later
const typeThunk = (): GraphQLOutputType | GraphQLInputType => {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think GraphQLOutputType | GraphQLInputType == GraphQLType

/**
* Maps a TypeSpec type to a GraphQL type
*/
#mapTypeSpecToGraphQL(type: Type): GraphQLOutputType | GraphQLInputType {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

enum: (node: Enum) => {
this.registry.addEnum(node);
namespace: (namespace: Namespace) => {
if (namespace.name === "TypeSpec" || namespace.name === "Reflection") {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if (namespace.name === "TypeSpec" || namespace.name === "Reflection") {
if (["TypeSpec", "Reflection"].includes(namespace.name)) {

@@ -101,3 +159,143 @@ export abstract class TypeMap<T extends Type, G extends GraphQLType> {
*/
protected abstract materialize(context: TSPContext<T>): G;
}

/**
* TypeMap for GraphQL Object types (output types)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there some shared functionality here between output object and input object types that warrants a common parent class?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants