fix: ensure variance of types matches how values are used

[ForbesLindesay]

I've written up detailed reasoning in dotansimha/graphql-typed-document-node#38

I didn't remove the "FIXME" comment, as I'm not sure what you would prefer to do with it. Nominal typing wouldn't actually solve anything here, because the variables and responses are plain objects, not nominal types. What's needed is just accurate structural typing. I really don't view this as a hack because:

1. We're not lying about any of the types that exist at runtime
2. The __apiType property does accurately document/describe the API behavour
3. The type errors TypeScript would produce if you use these TypedQueryDocumentNodes incorrectly matches up with the issues you would see at runtime.

[IvanGoncharov]

I really don't view this as a hack because:

@ForbesLindesay I'm ok with merging it but if we declare a non-void property that is never set it is a clear hack in my book 😄

Nominal typing wouldn't actually solve anything here, because the variables and responses are plain objects, not nominal types.

In Nominal type systems types with different template arguments treated as incompatible types, e.g. C++, Java, etc.

__apiType

The name of the field should reflect its purpose so if I get an error message I don't need to look into graphql-js sources to understand what's the purpose of this field.
I don't think a long name is a problem for a field that you should never use in your code so it is better to have an extremely description field.

[IvanGoncharov]

The type errors TypeScript would produce if you use these TypedQueryDocumentNodes incorrectly matches up with the issues you would see at runtime.

@ForbesLindesay Please add test for this here:
https://github.com/graphql/graphql-js/blob/master/integrationTests/ts/index.ts#L74

[IvanGoncharov]

@dotansimha this bugfix blocks 15.4.0 release since we can't release TypedQueryDocumentNode until we are sure we implemented it correctly.
Can you please work together with @ForbesLindesay to address review comments and also ensure this functionality is compatible with graphql-code-generator. It would great if you prepare matching PR for graphql-code-generator and test it together with other GraphQL packages that it's intended to be used.

[dotansimha]

Hi @IvanGoncharov , sorry for the delay! I will look into this tomorrow :)

[dotansimha]

@ForbesLindesay I think you are right here, I did some checks and it seems like the covariant might cause inconsistency here.
I tested this with your change and it seems to work perfectly :)

@IvanGoncharov From my point of view, I think it's a better way to declare the usage of the generics, and it doesn't really matter since it's unused, same as the existing solution.

[IvanGoncharov]

@dotansimha @ForbesLindesay Can you please rename __apiType to something more descriptive as explained here:
#2786 (comment)
And add test as described here:
#2786 (comment)
I can't merge it without tests since I don't understand what is fixed by this PR.
This issue blocks 15.4.0 release, so if you don't have time to address review comments please ping me so we can figure out the next steps.

[ardatan]

@ForbesLindesay I created a PR for your branch that includes what @IvanGoncharov asked for.
ForbesLindesay#1

[ardatan]

@ForbesLindesay This is another PR for linting errors :)
ForbesLindesay#2

[ardatan]

@IvanGoncharov I think it is ready to merge?

[IvanGoncharov]

@ardatan Thanks a lot 👍
Merged 🎉