Zaid-Ajaj / Snowflaqe

A dotnet CLI to generate type-safe GraphQL clients for F# and Fable with automatic deserialization, static query verification and type checking
MIT License
154 stars 25 forks source link

Exception when using custom scalars inside .graphql schema #72

Closed MargaretKrutikova closed 2 years ago

MargaretKrutikova commented 2 years ago

Type generation fails with Unable to resolve reference to type MyCustomScalar if the schema contains custom scalars. This happens only if using .graphql schema when calling Introspection.fromSchemaDefinition. The reason for this is that when using schema first in graphql-dotnet you have to register your custom scalars directly after parsing the schema, here is the docs and here is how it looks like:

var schema = Schema.For(...);
schema.RegisterType(new MyBooleanGraphType());

I was working on a fix for this and my idea was to parse the schema to get the custom scalars out of it and register the types, which would look like this (pseudocode):

let graphqlServer = GraphQL.Types.Schema.For(definition, configureSchemaBuilder)
let types = GraphQLParser.Parser.Parse(definition) // parse the schema into a graphql document to filter scalars

getCustomScalars types |> buildGraphTypes |> |> Seq.iter graphqlServer.RegisterType
let schemaJson = graphqlServer.ExecuteAsync(...)

This seems to work, however it does seem pretty inefficient since the schema is parsed many times - first graphq-dotnet parses it in GraphQL.Types.Schema.For, then I parse it again to get the scalars, then Snowflaqe parses it once more to convert from json to its internal representation.

So I started thinking that maybe we could parse the result of GraphQLParser.Parser.Parse(definition) directly into the internal data structures Snowflaqe uses for type generation? This of course would only be necessary for schemas defined in .graphql. Any thoughts on this, @Zaid-Ajaj ?

FYI, @Yakimych @danielstarck 🙂

Zaid-Ajaj commented 2 years ago

I think I like the first approach better and a PR would be appreciated 👍 less fond of the second approach because then we two ways of inferring types from a schema and I would like to not depend on the parser of graphql-dotnet

MargaretKrutikova commented 2 years ago

Understood, I will prepare a PR with the first approach. Any particular reason you don't want to depend on the parser from graphql-dotnet? Just wondering for educational purpose 🙂

Zaid-Ajaj commented 2 years ago

Any particular reason you don't want to depend on the parser from graphql-dotnet?

I would rather rely on deriving the types from the result of the standard introspection query because the output of it is predictable and won't change based on different versions of a specific parser, graphql-dotnet in this case

MargaretKrutikova commented 2 years ago

It got me thinking, can we avoid depending on graphql-dotnet all together and use some other tool to convert from .graphql to .json? Or in the worst case write it ourselves in F# 😆 It seems like we are tweaking graphql-dotnet to make it become a valid graphql server and the only thing we want from it is the introspection query.

Zaid-Ajaj commented 2 years ago

I would rather not have to write and maintain my own parser if possible to replace the one from graphql-dotnet. Tweeking it is a lot easier than rebuilding it, although I agree it is not ideal.