feat: emit TypedDocumentNode alias in generated code

[hallettj]

This change adds an exported type alias from each generated module based on the TypedDocumentNode type from @graphql-typed-document-node/core. Apollo Client and Villus are able to consume this type and propagate result data and variables types correctly. It looks like support for this type is also coming to Apollo Angular and to GraphQL.js itself. For example:

import { gql, useQuery } from '@apollo/client';

const query = gql`
  query GitHubQuery($first: Int!) {
    viewer {
      repositories(first: $first) {
        nodes {
          id
          description
        }
      }
    }
  }
` as import("./__generated__/git-hub-query.ts").GithubQueryDocument;
//   ^
//   type assertion is co-located with query

export default () => {
  const { data } = useQuery(query, { variables: { first: 100 } });
  //      ^                                     ^
  //      inferred type is `GithubQuery`        |
  //                                            |
  //                                 inferred type is `GithubQueryVariables`

  /* ... */
};

As a result types can be hooked up with one type assertion that is applied directly to the query expression. If this PR is merged I'd like to update @originate/eslint-plugin-ts-graphql to apply that type assertion automatically when running eslint --fix. I think this pattern is helpful because it allows generated types to be hooked up automatically, and in cases where a query is used in multiple call sites setting up types once on the query expression is more robust than setting up types at every call site.

With the change generated files look like this:

/* eslint-disable */
/* This is an autogenerated file. Do not edit this file directly! */
import { TypedDocumentNode } from "@graphql-typed-document-node/core";
export type GitHubQuery = {
    viewer: {
        repositories: {
            nodes: (({
                id: string;
                description: string | null;
            }) | null)[] | null;
        };
    };
};
export type GitHubQueryVariables = {
    first: number;
};
export type GitHubQueryDocument = TypedDocumentNode<GitHubQuery, GitHubQueryVariables>;

The change adds a dependency on @graphql-typed-document-node/core. I know that your contributing guidelines say not to add dependencies - but this is a teeny tiny dependency that I think adds significant value.

It is possible to avoid the new dependency by writing the type like this:

import { DocumentNode } from "graphql"

export interface GithubQueryDocument extends DocumentNode {
    __resultType?: GitHubQuery;
    __variablesType?: GitHubQueryVariables;
}

That interface is compatible with the current version of TypedDocumentNode. But from what I see in this commit TypedDocumentNode may change in an upcoming release so that it is no longer be compatible with the above interface. If you depend on and import TypedDocumentNode then you don't have to worry about the change.

By the way I was not able to run the e2e tests, so I don't know whether I've broken them.

[codecov-io]

Codecov Report

Merging #230 (58a2276) into master (3076b33) will increase coverage by 0.08%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #230      +/-   ##
==========================================
+ Coverage   91.09%   91.18%   +0.08%     
==========================================
  Files          47       47              
  Lines        3696     3732      +36     
  Branches      451      457       +6     
==========================================
+ Hits         3367     3403      +36     
  Misses        329      329              

Impacted Files	Coverage Δ
src/typegen/type-gen-visitor.ts	90.27% <100.00%> (+0.73%)	⬆️

[Quramy]

Hi. @hallettj thanks for PR.

I see your comment, #117 (comment) .

I've not seen TypedDocumentNode approach and I have few questions about this type.

import { DocumentNode } from 'graphql';
export interface TypedDocumentNode<Result = {
    [key: string]: any;
}, Variables = {
    [key: string]: any;
}> extends DocumentNode {
    __resultType?: Result;
    __variablesType?: Variables;
}

1. Who did decide the __resultType and __variableTypes fields' name ? Is this Apollo client's convention ?
2. DocumentNode is AST type (It should stands for query syntax tree structure) . But Result / Variables types represent some values. Is the extending interface really correct ? It seems that gql tag function never provides __resultType nor __variableType properties at runtime.

I understand this feature may be useful for AC users and I like your ESLint plugin fixer.

But I can't merge this to my type generator because:

• this approach seems experimental and hacky
• I want to keep our my generator generic (e.g. for https://github.com/octokit/graphql.js/ users)

I think my type-generator should expose some functions to customize. Then we can provide this TypedDocumentNode generation option to AC users such as:

/* tsconfig */
{
  plugins:[
    {
       name: "ts-graphql-plugin",
       schema: "schema.gql",
       typegen: {
         addons: ["./typed-document-node"]
       }
     }
}

/* typed-document-node.ts */
export default function addon(source: ts.SourceFile, generationContext) {
  const importStatement = ts.factory.createImport(...);
  const exportTypedDocumentNodeStatement = ts.factoryCreateExport(...) // create with generationContext
  return ts.factory.updateSourceFileNode(source, ...); // push the statements
}

[hallettj]

Thanks for the quick reply! I'll try to address your questions, and to make my case.

I have more detailed explanations below, but briefly:

1. The __resultType and __variableTypes names were chosen by the GraphQL Code Generator team (IIUC) as the authors of @graphql-typed-document-node/core. The names don't have anything to do with Apollo specifically. They should be considered to be internal details of the TypedDocumentNode implementation - they're not really part of the public API. You only have to think about them if you want to create your own type that is assignable to TypedDocumentNode.
2. You're right, __resultType and variableTypes are not defined at runtime. It's necessary to include them in the type to make the phantom type pattern work (details below). They're marked as optional, so it's ok that they are not set at runtime.

I respect your preference to avoid adding a special feature to support one library. But I don't think that's the case with this PR. TypedDocumentNode is not specific to Apollo Client. It comes from the GraphQL Code Generator team as a proposed standard solution for communicating query metadata between libraries. It happens that Apollo is one of the early adopters. The TypedDocumentNode readme has instructions wrapping or patching a number of other libraries so that they can also benefit from type inference via TypedDocumentNode - I'm sure it's possible to do the same with @octokit/graphql. We need some evangelizing to encourage more libraries to adopt this type to make it a proper standard (which is what I'm trying to do here 😉). If you adopt this pattern it would be immediately useful to people using Apollo or Villus, or to anyone using a patched version of another library, and it would help to encourage more library authors to adopt it too.

I disagree that the solution is "hacky". Well ok, it is a little hacky. But phantom types are a proven pattern with legitimate uses (again, more details below). It's not an idea that is specific to TypedDocumentNode or to Apollo. They work reliably regardless of how weird the pattern looks. And I think they add substantial value in this case.

The TypedQueryDocumentNode option from graphql

I read in the @graphql-typed-document-node readme that TypedDocumentNode would be used in the main graphql package in v16. But it looks like graphql has created its own version, TypedQueryDocumentNode. Being in graphql makes it seem official to me, and you already have a peer dependency on graphql. So maybe that is the type to work with instead of TypedDocumentNode. That version would be almost the same:

/* eslint-disable */
/* This is an autogenerated file. Do not edit this file directly! */
import { TypedQueryDocumentNode } from "graphql";
export type GitHubQuery = {
    viewer: {
        repositories: {
            nodes: (({
                id: string;
                description: string | null;
            }) | null)[] | null;
        };
    };
};
export type GitHubQueryVariables = {
    first: number;
};
export type GitHubQueryDocument = TypedQueryDocumentNode<GitHubQuery, GitHubQueryVariables>;

Edit: The downside is that TypedQueryDocumentNode was only added in graphql v15.4.0 which has only been out for a few weeks.

Phantom Types

Both TypedDocumentNode and TypedQueryDocumentNode use the phantom type pattern. This is an established pattern for preserving information in types. By placing information in type arguments, another piece of type-level code can extract that information. For example if Apollo Client did not support TypedDocumentNode it would be possible to propagate types like this:

type ResponseData<Doc> = Doc extends TypedDocumentNode<infer Result, unknown> ? Result : never;
type Variables<Doc> = Doc extends TypedDocumentNode<unknown, infer Variables> ? Variables : never;

// assuming `query` is declared with an `as GitHubQueryDocument` or similar type assertion
const { data } = useQuery<ResponseData<typeof query>, Variables<typeof query>>(
  query,
  { variables: { first: 100 } }
)

And it's pretty easy to make a reusable wrapper that abstracts away the type inference:

// The types `QueryHookOptions` and `QueryResult` come from `@apollo/client`
function useQueryTyped<ResponseData, Variables>(
  query: TypedDocumentNode<ResponseData, Variables>,
  options: QueryHookOptions<ResponseData, Variables>
): QueryResult<ResponseData, Variables> {
  return useQuery(query, options)
}

const { data } = useQueryTyped(query, { variables: { first: 100 } })

Nothing ever accesses __resultType, __variablesType, or __ensureTypesOfVariablesAndResultMatching (from TypedQueryDocumentNode). Those properties are unimportant, and in fact will never exist at runtime. It is necessary to include them in the type definition because TypeScript is structurally typed so TypeScript would throw away the type arguments if they were not applied to some property in the type - even if it is a purely hypothetical property. And it is necessary to be consistent with the property names in case two libraries use distinct but structurally-compatible types. Notice that all three properties are optional so even though they do not exist at runtime the type does technically accurately reflect the runtime document node value.

Promise analogy

Consider the Promise type. It takes a type parameter to communicate the type of the resolved value, even though the resolved value might not be expressed in the runtime representation of the promise at least while it is in the pending state (for example if the result is JSON data from a network response). It is valuable to express that metadata in the Promise type regardless.

It's true that Promise has a then method which uses the type parameter, and then does exist at runtime. If DocumentNode had a built-in method to execute a query it would be logical to include type parameters, and to express them in that execute method like this:

interface DocumentNode<Data, Variables> {
  /* ... */
  execute(variables: Variables): Promise<Data>;
}

Instead of having a built-in execute method DocumentNode delegates that functionality to other types, which makes a lot of sense. The idea behind TypedDocumentNode is that we don't need to give up on valuable type inference just because query execution is not implemented directly in the DocumentNode type.

Holding on to relevant metadata

It is important to extend the DocumentNode interface the value of a gql`...` expression is a DocumentNode AST. TypedDocumentNode accurately describes the runtime AST - it just attaches some extra metadata at the type level. An AST does encode the shape of response data and variables even if it does not directly encode the type of each field and variable. That information does not have to be thrown away! It can be preserved using phantom type parameters.

String queries

I looked into @octokit/graphql and from what I can tell it does not accept DocumentNode query values, only string queries. Is that correct? If so that does make it problematic to use TypedDocumentNode with that specific library because a TypedDocumentNode is not a string. It is possible to use a similar trick to attach type-level metadata to a string type:

import { RequestParameters } from "@octokit/graphql/dist-types/types"

type TypedQueryString<ResponseData, Variables> = string & {
  __ensureTypesOfVariablesAndResultMatching?: (
    variables: Variables
  ) => ResponseData
}

// Wrapper around @octokit/graphql that hooks up type inference.
function typedGraphql<ResponseData, Variables>(
  query: TypedQueryString<ResponseData, Variables>,
  parameters: Variables & RequestParameters
): Promise<ResponseData> {
  return graphql(query, parameters)
}

const data = await typedGraphql(query, { first: 100 })
//    ^                                ^
//    type is `GitHubQuery`            |
//                                     |
//                           variables are correctly type-checked

A value of type TypedQueryString is a string for all intents and purposes, and its runtime representation is a plain string. But it carries some extra metadata at type-checking time.

But this is probably a different issue. I don't know of an existing candidate for a standardized TypedQueryString type.

Conclusion

The plugin option looks useful. I can write my own plugin if you don't want to merge TypedDocumentNode or TypedQueryDocumentNode` integration. But I do think that the pattern is broadly useful, and I urge you to reconsider.

What would you think if I modified this PR to get rid of the @graphql-typed-document-node/core dependency, and to use the TypedQueryDocumentNode type from graphql instead? Would that be an impediment to users who do not have a need for that type?

Edit: My proposal would require users to install graphql v15.4.0 which has only been out for a few weeks. I can see how that is problematic. On the other hand the @graphql-typed-document-node option doesn't have the problem of requiring users to upgrade graphql.

Edit: What about a configuration option to allow users to opt into using TypedQueryDocumentNode?

Thanks for the clean types!

While I'm here I want to mention that the most important reason I've chosen to use ts-graphql-plugin over other query type generation solutions is that the types produced by ts-graphql-plugin lead to error messages that are easy to understand. Previously I've used GraphQL Code Generator which produces complex types which lead to inscrutable error messages. The first time I introduced a team to type-checked GraphQL they gave me feedback that they found it difficult to work with error messages they could not understand. So thank you for the clean type generation!

[hallettj]

I'm sorry, I dumped a lot of text on you yesterday. After writing all that I see how the typed document node idea is rather experimental. I have one more idea, which is that the typegen command could check the version of graphql that is installed, and emit a TypedQueryDocumentNode only if the latest version of graphql is present. That would avoid the problem of requiring users to upgrade graphql, but allow users who do have the new version to take advantage of the new feature. But for now I'll look into the addon / plugin option.

[Quramy]

What would you think if I modified this PR to get rid of the @graphql-typed-document-node/core dependency, and to use the TypedQueryDocumentNode type from graphql instead? Would that be an impediment to users who do not have a need for that type?

export interface TypedQueryDocumentNode<
  TResponseData = Record<string, any>,
  TRequestVariables = Record<string, any>
> extends DocumentNode {
  readonly definitions: ReadonlyArray<ExecutableDefinitionNode>;
  // FIXME: remove once TS implements proper way to enforce nominal typing
  /**
   * This type is used to ensure that the variables you pass in to the query are assignable to Variables
   * and that the Result is assignable to whatever you pass your result to. The method is never actually
   * implemented, but the type is valid because we list it as optional
   */
  __ensureTypesOfVariablesAndResultMatching?: (
    variables: TRequestVariables,
  ) => TResponseData;
}

It seems also experimental because the above FIXME comment. (And I think there're a little differences of purpose between nominal typings and result/variables type inference 🤔 )

But I like graphql-js's TypedQueryDocumentNode rather than @graphql-typed-document-node/core's one. I want to keep my plugin's dependencies small and I think graphql-js should be responsible to provide this type.

If graphql-js v16 be released, it may be nice to generate like the following:

/* __generated/my-query.ts */

import { TypedQueryDocumentNode } from 'graphql'

export type MyQueryResult = ...
export type MyQueryVariables = ...

export type MyQueryDocumentNode = TypedQueryDocumentNode<MyQueryResult, MyQueryVariables>;

In any case, we should to provide a way to opt-in ( or opt-out ) outputting MyQueryDocumentNode.
I think it's better opt-in while my plugin is depending on graphql-js v15.

[hallettj]

It seems also experimental because the above FIXME comment. (And I think there're a little differences of purpose between nominal typings and result/variables type inference 🤔 )

I don't think that FIXME serves much purpose. As you say nominal types are a different idea. It just happens that both nominal types and phantom types are implemented with hypothetical properties in TypeScript. And I don't think either pattern is going to change because it would mean a substantial change to TypeScript's philosophy of structural typing. If the internal details of the type do change in the future it shouldn't make a difference for libraries that use it. The public API is just this part:

interface TypedQueryDocumentNode<
  TResponseData = Record<string, any>,
  TRequestVariables = Record<string, any>
> extends DocumentNode

I'll put in an opt-in configuration option, and switch to the TypedQueryDocumentNode type, and you can tell me what you think.

[Quramy]

@hallettj Thanks for refactoring and writing docs !

I'm writing prototype of functions to hook the type generator #232 . For add-on detail, see https://github.com/Quramy/ts-graphql-plugin/blob/typegen-addon/project-fixtures/typegen-addon-prj/addon.ts

And I intend to implement your TypedQueryDocumentNode feature as a type generator extension.

I plan users can make this feature enabled via the following option:

    "plugins": [
      {
        "name": "ts-graphql-plugin",
        "tag": "gql",
        "schema": "schema.graphql",
        "typegen": {
          "addons": ["ts-graphql-plugin/addons/typed-query-node"]
        }
      }
    ]

( Of course, after graphql-js v16 release, I'll change this typed-query-node add-on enabled by default )

How about it and can you wait a little until #232 is merged ?

[hallettj]

Cool, that works for me. Thank you for all of your time and effort!

[Quramy]

@hallettj .
I implemented a typegen extension to use TypedQueryDocumentNode and published it as v2.1.0. See https://github.com/Quramy/ts-graphql-plugin#typed-query-document .

[hallettj]

That's awesome - thanks so much!

…

On Sat, Nov 21, 2020, 10:41 Yosuke Kurami ***@***.***> wrote: @hallettj <https://github.com/hallettj> . I implemented a typegen extension to use TypedQueryDocumentNode and published it as v2.1.0. See https://github.com/Quramy/ts-graphql-plugin#typed-query-document . — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#230 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAACLFXPTO3VSTQFT4LWDCLSQ7NSFANCNFSM4TVJXM6Q> .