Skip to content
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

Repeated fragment in auto-generated query. #270

Open
Zimmi48 opened this issue Sep 13, 2021 · 11 comments
Open

Repeated fragment in auto-generated query. #270

Zimmi48 opened this issue Sep 13, 2021 · 11 comments

Comments

@Zimmi48
Copy link

Zimmi48 commented Sep 13, 2021

I've written a GraphQL request which uses several times the same fragment through different fragments (see the code below and https://github.com/coq/bot/blob/bc65314888df8e8c6c8bad55624f9492a36a1e78/bot-components/GitHub_GraphQL.ml#L30-L47 for the full context).

  fragment Column on ProjectColumn {
    id @ppxCustom(module: "ID")
    databaseId
  }

  fragment Project on Project {
    columns(first:50) {
      nodes { ... Column }
    }
  }

  fragment ProjectCards on ProjectCardConnection {
    nodes {
      id @ppxCustom(module: "ID")
      column { ... Column }
      project { ... Project }
    }
  }

The auto-generated query contains two copies of the Column fragment, this makes the GraphQL query ill-formed.

This is all the more annoying that, in this case, GitHub's GraphQL API doesn't answer with a clear error message but with:

{
  "data": null
}

(separately reported to GitHub)

@jfrolich
Copy link
Collaborator

Ah interesting! How would that work in JS?

@Zimmi48
Copy link
Author

Zimmi48 commented Sep 13, 2021

I'm an OCaml dev, so I'm not sure I can answer that. But what I did to debug the problem (because I didn't understand why GitHub was answering with an empty data field) was to print the request it was sending, then I confirmed with curl that it was not working:

curl https://api.github.com/graphql -X POST -H "Authorization: bearer $GH_TOKEN" -d '{"query":"query GetMilestoneMergedPullRequests($owner: String!, $repo: String!, $number: Int!)  {\nrepository(owner: $owner, name: $repo)  {\nmilestone(number: $number)  {\npullRequests(first: 100, states: [MERGED])  {\nnodes  {\n...PullRequestWithMilestoneAndCards   \n}\n\n}\n\n}\n\n}\n\n}\nfragment PullRequestWithMilestoneAndCards on PullRequest   {\nid  \ndatabaseId  \nnumber  \nmilestone  {\n...Milestone   \n}\n\nprojectCards(first: 20)  {\n...ProjectCards   \n}\n\n}\nfragment Milestone on Milestone   {\nid  \nnumber  \ntitle  \ndescription  \n}\nfragment ProjectCards on ProjectCardConnection   {\nnodes  {\nid  \ncolumn  {\n...Column   \n}\n\nproject  {\n...Project   \n}\n\n}\n\n}\nfragment Column on ProjectColumn   {\nid  \ndatabaseId  \n}\nfragment Project on Project   {\ncolumns(first: 50)  {\nnodes  {\n...Column   \n}\n\n}\n\n}\nfragment Column on ProjectColumn   {\nid  \ndatabaseId  \n}\n","variables":{"owner":"coq","repo":"coq","number":37}}'

Then I tested the auto-generated query in GitHub's GraphQL explorer:

query GetMilestoneMergedPullRequests($owner: String!, $repo: String!, $number: Int!)  {
  repository(owner: $owner, name: $repo)  {
    milestone(number: $number)  {
      pullRequests(first: 100, states: [MERGED])  {
        nodes  {
          ...PullRequestWithMilestoneAndCards   
        }
        
      }
      
    }
    
  }
  
}
fragment PullRequestWithMilestoneAndCards on PullRequest   {
  id  
  databaseId  
  number  
  milestone  {
    ...Milestone   
  }
  
  projectCards(first: 20)  {
    ...ProjectCards   
  }

}
fragment Milestone on Milestone   {
  id  
  number  
  title  
  description  
}
fragment ProjectCards on ProjectCardConnection   {
  nodes  {
    id  
    column  {
      ...Column   
    }
    
    project  {
      ...Project   
    }
  
  }

}
fragment Column on ProjectColumn   {
  id  
  databaseId  
}

fragment Project on Project   {
  columns(first: 50)  {
    nodes  {
      ...Column   
    }
  
  }

}
fragment Column on ProjectColumn   {
  id  
  databaseId  
}

and it highlighted the first Column fragment with the message There can be only one fragment named "Column".

@Zimmi48
Copy link
Author

Zimmi48 commented Sep 13, 2021

Another simpler (and complete) example where it triggered this issue:

  fragment Milestone on Milestone {
    id
  }

  fragment PullRequest on PullRequest {
    id
    milestone { ... Milestone }
  }

  query issueMilestone($owner: String!, $repo: String!, $number: Int!) {
    repository(owner:$owner, name:$repo) {
      issue(number:$number) {
        id
        milestone { ... Milestone }
        timelineItems(itemTypes:[CLOSED_EVENT],last:1) {
          nodes {
            ... on ClosedEvent {
              closer {
                ... on PullRequest { ... PullRequest }
                ... on Commit {
                  associatedPullRequests(first: 2) {
                    nodes { ... PullRequest }
                  }
                }
              }
            }
          }
        }
      }
    }
  }

@jfrolich
Copy link
Collaborator

I think some production users haven't hit this issue because they use the template tag option (default for rescript-apollo-client). What client are you using? I think this should be fixed regardless btw.

@Zimmi48
Copy link
Author

Zimmi48 commented Sep 13, 2021

I am calling graphql-ppx directly (I am not aware of any available client for native OCaml).

@jfrolich
Copy link
Collaborator

This issue didn't happen with lower versions? (because handling of this shouldn't have changed too much I think)

@Zimmi48
Copy link
Author

Zimmi48 commented Sep 13, 2021

Before upgrading to 1.2.0, I didn't use fragments as much because the object encoding didn't make them as essential. So I don't think it's a regression but rather that I had never had an occasion of encountering this issue before.

@jfrolich
Copy link
Collaborator

Interesting. I think JS GraphQL clients might do some deduplication. Need to look into this a little deeper.

@Zimmi48
Copy link
Author

Zimmi48 commented Nov 4, 2021

Hey @jfrolich, do you think this issue is reasonably easy to fix? I tried to look at the code to see if I could help, but I don't understand the code enough to manage to do that.

@Zimmi48
Copy link
Author

Zimmi48 commented Nov 5, 2021

BTW, you were right that (at least some) JS GraphQL clients do some deduplication: apollographql/graphql-tag#27

@vknez
Copy link

vknez commented Jun 24, 2022

I encountered the same issue while trying to implement a data-layer using GraphQL PPX. It generated a duplicate fragment, which yields in server error.

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

No branches or pull requests

3 participants