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
157 stars 26 forks source link

Similar to your comment about unions not be implemented yet, what about interfaces? #12

Closed mattsonlyattack closed 4 years ago

mattsonlyattack commented 4 years ago

This works when querying github, though snowflaq cannot validate it w/"unknown field author"

{
    "name": "author",
    "description": "The actor who authored the comment.",
    "args": [],
    "type": {
        "kind": "INTERFACE",
        "name": "Actor",
        "ofType": null
    },
    "isDeprecated": false,
    "deprecationReason": null
},
query GetPullRequests($org: String!) {
  organization(login: $org) {
    name
    url
    repositories(first: 100) {
      nodes {
        name
        pullRequests(first: 100, states: OPEN) {
          nodes {
            number
            title
            url
            body
            author {
              login
            }
            reviews(last: 10, states: APPROVED) {
              nodes {
                author {
                  login
                }
              }
            }
          }
        }
      }
    }
  }
}

** this could just be me not knowing much about graphql...

mattsonlyattack commented 4 years ago

this should be close but probably not exact to our github enterprise version - https://raw.githubusercontent.com/octokit/graphql-schema/master/schema.json

Zaid-Ajaj commented 4 years ago

This works when querying github, though snowflaqe cannot validate it

Hi @mattsonlyattack, it is correct that GraphQL interfaces aren't supported. After having checked out the query and the schema, it looks like it "should" be doable to generate correct types from this particular query when you are asking for common fields on the interface itself. However, once you start using concrete type query fragments it becomes as complicated as with the GraphQL unions. For example:

query GetPullRequests($org: String!) {
  organization(login: $org) {
    name
    url
    repositories(first: 100) {
      nodes {
        name
        pullRequests(first: 100, states: OPEN) {
          nodes {
            number
            title
            url
            body
            author {
              login
            }
            reviews(last: 10, states: APPROVED) {
              nodes {
                author {
                  ... on Bot {
                     botField1
                     botField2
                     // etc.
                   }

                   ... on User { 
                       userSpecificField1
                       userSpecificField2
                    }
                }
              }
            }
          }
        }
      }
    }
  }
}

This makes the return type complicated because basically I end up having to implement the GraphQL unions anyways which isn't straightforward, especially because the JSON deserialization (currently using SimpleJson) doesn't understand unions that are encoded as objects.

In any case, I will take a look at it soon to see if it is doable. Thanks a lot for sharing the schema and query! It would be great if I also have a test query that returns fields from just Author but also from another type which implements Author like Bot or User. A bonus is a sample JSON returned from the query to help me figure out how I should change the way JSON deserialization to make it work for this case. You can obfuscate the actual content of the entities returned, I only care about the shape of the data returned.

mattsonlyattack commented 4 years ago

Hope this helps.

query:

query GetPullRequests($org: String!) {
  organization(login: $org) {
    name
    url
    repositories(first: 1) {
      nodes {
        name
        pullRequests(first: 1, states: MERGED) {
          nodes {
            number
            title
            url
            body
            author {
              login
              url
            }
            mergedBy {
              login
              resourcePath
            }
            reviews(last: 10, states: APPROVED) {
              nodes {
                author {
                  avatarUrl
                  login
                  ... on Bot {
                    login
                    id
                  }
                  ... on User {
                    bio
                    id
                  }
                }
              }
            }
          }
        }
      }
    }
  }
}

response:

{
  "data": {
    "organization": {
      "name": "my-org",
      "url": "https://example.com/my-org",
      "repositories": {
        "nodes": [
          {
            "name": "some-repo",
            "pullRequests": {
              "nodes": [
                {
                  "number": 2,
                  "title": "add engineering agreements to readme",
                  "url": "https://example.com/my-org/some-repo/pull/2",
                  "body": "probably a giphy",
                  "author": {
                    "login": "APerson",
                    "url": "https://example.com/APerson"
                  },
                  "mergedBy": {
                    "login": "BPerson",
                    "resourcePath": "/BPerson"
                  },
                  "reviews": {
                    "nodes": [
                      {
                        "author": {
                          "avatarUrl": "https://example.com/avatars/u/1665",
                          "login": "CPerson",
                          "bio": "i do things",
                          "id": "ASDFIJ="
                        }
                      },
                      {
                        "author": {
                          "avatarUrl": "https://example.com/avatars/u/1892",
                          "login": "BPerson",
                          "bio": i dont do things,
                          "id": "BASODF="
                        }
                      }
                    ]
                  }
                }
              ]
            }
          }
        ]
      }
    }
  }
}
Zaid-Ajaj commented 4 years ago

@mattsonlyattack Thanks a lot for the sample queries! I think I know now where to look to find a solution. I am afraid however that it will be quite the challenge to implement this feature. I can't promise a timeline but I will definitely look into it soon

Zaid-Ajaj commented 4 years ago

Alright, fixed the first part of the problem with JSON deserialization in Fable Implement deserialization heuristic with union of records

Now I need to do the same for the dotnet deserializer, tracked in this issue Extend Fable.Remoting.Json with heuristics for unions of records

Then I could come back to Snowflaqe and generate the types for interfaces. I think because the deserialization issue is resolved, it will be relatively straightforward to implement GraphQL unions as well

Zaid-Ajaj commented 4 years ago

Hi @mattsonlyattack,

It has been a long and exciting weekend. I think I have got this implemented like it should 😁 Snowflaqe now understands both GraphQL interfaces and unions, generates the corresponding F# union types and verifies the used fields just like it does with objects. The only requirement is that the subtypes both on interfaces and unions must now add a __typename field and will otherwise throw a validation error. This field is used for deserialization.

It would be great if you could put this version to the real test and run it against the Github schema that you working with. You have to modify your query slightly into this:

query GetPullRequests($org: String!) {
  organization(login: $org) {
    name
    url
    repositories(first: 1) {
      nodes {
        name
        pullRequests(first: 1, states: MERGED) {
          nodes {
            number
            title
            url
            body
            author {
              login
              url
            }
            mergedBy {
              login
              resourcePath
            }
            reviews(last: 10, states: APPROVED) {
              nodes {
                author {
                  avatarUrl
                  login
                  ... on Bot {
                    __typename
                    login
                    id
                  }
                  ... on User {
                     __typename
                    bio
                    id
                  }
                }
              }
            }
          }
        }
      }
    }
  }
}

Please let me know how it goes and whether everything works for you :smile:

mattsonlyattack commented 4 years ago

Wow, very productive weekend!

The results from the server exactly match my query results in postwoman, and I'm able to inspect the query results and so far the data is looking great. So cool, thank you!

I did start w/an invalid query by accident, and my query result matched on Ok, wondering if it should have been an Error instead? Maybe a useful test case, or maybe I'm thinking wrong.

Here is my query screw-up where I put the type spread in the wrong spot (that made it through snowflaqe validation and generation):

query GetPullRequests($org: String!) {
  organization(login: $org) {
    name
    url
    repositories(first: 100) {
      nodes {
        name
        pullRequests(first: 100, states: OPEN) {
          nodes {
            number
            title
            url
            body
            author {
              login
            }
            reviews(last: 10, states: APPROVED) {
              nodes {
                author {
                  login
                }
                ... on User {
                    __typename
                    bio
                    id
                  }
              }
            }
          }
        }
      }
    }
  }
}

And here is the error message from graphql: {"errors":[{"path":["query GetPullRequests","organization","repositories","nodes","pullRequests","nodes","reviews","nodes","... on User"],"extensions":{"code":"cannotSpreadFragment","typeName":"User","fragmentName":"unknown","parentName":"PullRequestReview"},"locations":[{"line":45,"column":33}],"message":"Fragment on User can't be spread inside PullRequestReview"}]}

Zaid-Ajaj commented 4 years ago

Hi @mattsonlyattack,

I have created an issue to validate the incorrect query. As for the interfaces, it seems like I make a small mistake by only including the types to generated F# union case which are used in the inline fragments as opposed to adding all subtypes of the interface to the generated union (which also means the __typename will now be required on interface field selections as well)

Zaid-Ajaj commented 4 years ago

Alright, I fixed the issue with the subtyping of interfaces and pushed a new package as of v1.7.0 🚀 can you please update and try it out?