I'm wondering if we could still allow fetching this property in cases where it is supported. One option could be to have two different queries for PullRequest - if the first one fails with error about isDraft not existing, fall back to the second that doesn't have it. It's also possible to have a query to see what fields are available (https://developer.github.com/v4/guides/intro-to-graphql/#discovering-the-graphql-api)

interesting, so there is now way to basically request with is draft field, and server can response with something like null if they couldn't provide it?
I mean we basically need to use 2 different query?

yeah, the server will just throw instead of sending back a null or undefined value. It looks like there is a way to conditionally include things in the query: https://graphql.org/learn/queries/#directives

So we could use that, after either querying to see if the 'isDraft' field shows up for the endpoint, or just based on whether this is enterprise or github.com

Hmm this won't be great for future maintainability I'm afraid. I Could imagine if there are 5 version of the API. Each one is adding a new field it's going to get hairy. But I couldn't think of a better solution at the moment

Yeah, graphql is supposed to be "versionless", but since GitHub enterprise will ship with certain versions of each schema, it makes it really hard to work with. (At least this is my understanding, maybe older enterprise versions eventually get the new fields added to them?) I think the most correct fix would be to first run the query to see what the current schema looks like

query {
  __type(name: "PullRequest") {
    fields {
      name
    }
  }
}

and then run the actual pull request query with the subset of fields that we want to fetch that are part of that result. This would be very annoying to have to do before every query though. I think isDraft is the only thing we are likely to have a problem with right now, it's currently marked as a preview feature: https://developer.github.com/v4/object/pullrequest/

Hi @RMacfarlane sorry I didn't not follow up. I was trying something but didn't finish it. Feel free to pick this up. Unfortunately I'm a bit swamp these and the next couple of weeks. Not sure if I can pick it up.

What I did try is to have different schema for github (existing query.gql) and another one (enterprise.gql). And in the credentials.js detect which version this is running (the enterprise return a header with a version). Based on that it will figure the correct schema and store this on the GithHub interface

The benefit of this is that the controller or the view logic is simpler because it doesn't have to do extra or introspection query. and easily support different version of the API in the future. But unfortunately didn't manage to get it to work yet.

Hmm I think i managed to get it to work: https://github.com/yulrizka/vscode-pull-request-github/pull/1/files

what do think @RMacfarlane? or is it maybe too much?

I think it looks pretty good! I'm a little bit worried about potentially having to maintain separate .gql files for different enterprise versions, but I actually like that this makes it very explicit what properties we expect for both cases. Right now I think there is very little API difference between enterprise versions and we could always use the schema of the oldest supported version. I think this change would be reasonable to merge.