Allow to pass generics to DocumentNode to allow TS type inference by GraphQL clients

[dotansimha]

Resolves #2727

[dotansimha]

Hi @IvanGoncharov , @leebyron how can we progress with this one? thanks!

[IvanGoncharov]

I'm totally for adding wrapping type for DocumentNode.
But It should be a separate type since the template argument is never used in DocumentNode or any other node. Plus DocumentNode can also contain SDL definitions.
So it should be separate type inherited from DocumentNode e.g. TypedQueryDocumentNode or something similar.

But I'm fully against changes to graphql/execute:

1. We shouldn't provide typings for something we can't guarantee. For example, we plan to merge @defer that will change payload and it would be different from type generated by graphql-codegen. The same goes for other custom directives that change payload or custom scalars that have representation other that string.
2. What are production use cases for typing the result of execute/graphql? Client queries are sent over the wire and we don't have type info on the server. I understand how typed queries are helpful on the client but I struggle to understand why it's needed on the server.

Summary: I understand how useful it would be to bind type info to query and share it between other tools so I'm fully for adding this type. However, I strongly disagree with reference implementation providing typings for execute/graphql that it can't guarantee to be in sync with what is actually returned.

[dotansimha]

Thank you @IvanGoncharov for the review!

@ForbesLindesay suggested (dotansimha/graphql-typed-document-node#34) to add internal types to DocumentNode to make sure TypeScript treats it as structural type and not removing the generics because they are unused.

Regarding execute/graphql functions:

1. Type inference won't work without adding the generics to the entire chain of calls. If execute won't have the generics, it won't get inferred later based on DocumentNode passed to it.
2. I do understand how @defer could effect it, but just like @skip and @include - this effect types only, and probably just make it optional.
3. graphql-codegen will handle the @defer when it will be available in graphql-js and will allow developers to reflect the effect of it on generated types. But please note that graphql-codegen isn't the only way to use generate types. Developers can manually specify the types.
4. No one expects GraphQL to be responsible for the result and match the generic types. If you allow to pass generics - developers knows that he's in charge of those types, and it's not part of the package itself.
5. We don't have to change execute / graphql at all if you wish, but then it will mean that only tools that uses DocumentNode will benefit from the inferred types. Also, graphql-js is used by many projects locally, and execute sometimes is being executed directly with an operation that has been declared locally.

@IvanGoncharov what do you think about the following: I can remove all the changes from execute, graphql and even from Source and limit those changes only for DocumentNode to allow generics in it. These will be defaulted to any / record value in order to make sure it's not breaking anything.
That means that types could still be inferred while DocumentNode is being used by other libraries / graphql clients.

I don't think adding a new type called TypedDocumentNode will be the best solution, because then it means we do need to change execute and others to allow DocumentNode | TypedDocumentNode.

[dotansimha]

@IvanGoncharov I updated the PR with the minimal changes possible, in order to support that. What do you think?

[IvanGoncharov]

@dotansimha Thanks 👍
As I wrote above can we add separate TypedQueryDocumentNode type with template parameters inherited from DocumentNode?

[dotansimha]

Thanks @IvanGoncharov .
As It wrote before, I think making it a new variable might make it more difficult to adopt in GraphQL client libraries. Since it means they need to modify the codebase to use TypedDocumentNode instead of the original DocumentNode.

Are you open to keep it as part of the original DocumentNode? it doesn't break anything, and it fully compatible.

[IvanGoncharov]

As It wrote before, I think making it a new variable might make it more difficult to adopt in GraphQL client libraries.

@dotansimha Sorry missed that in you previous comment.

I don't think adding a new type called TypedDocumentNode will be the best solution, because then it means we do need to change execute and others to allow DocumentNode | TypedDocumentNode.

Here is a snippet that execute should work perfectly fine with a new type and it also shows how to implement wrapper function around execute that utilize full potential of this new type:

interface DocumentNode {
  someValue: number
}


interface TypedDocumentNode<TArg> extends DocumentNode {
}

function execute(document: DocumentNode): unknown { return null; }

const stringTypedDocume...

Playground Link