Graphql-js: Strict typing with TS

Created on 20 Sep 2019  路  7Comments  路  Source: graphql/graphql-js

This issue is to track a set of goals for type safety in TypeScript. In experiments, I have confirmed that all of these should be possible. While the increased verboseness has some ergonomic cost, the resulting type safety is overwhelmingly worth it in my opinion.

Object Types

For every field of a GraphQLObjectType, the resolver (including a "default resolver") must return a value compatible with the "source" of the field's GraphQL "type". For example, if ChildType expects { foo: string }, then the result of ParentType's child field cannot be { foo: 123 } or "foo".

Arguments & Input Types

The configuration of all defined arguments and input types must be checked against the types used in a resolver. For example, a resolver expecting args.foo to be a non-null string cannot be used in a field that defines argument foo as optional.

Scalars

The memory type of both custom and built-in scalars must be tracked. For example, a field with an argument configuration describing foo as a GraphQLString cannot have a resolver that expects foo to be a boolean.


Using generics and conditional types, I have been able to demonstrate that each of these is possible. However, as these effect the type signature of all primitive GraphQL components, there are a substantial number of changes across the repo. At one point I had opened a PR to DefinitelyTyped that gave partial support for strict typing of arguments, but the change was breaking (from the perspective of types) and I was too busy to effectively convey the significance before the PR was auto-closed.

As this is a natural time to implement this kind of change (during an existing push to re-type the whole codebase), I'm going to open a PR that converts the entire repo to TS in a way that accomplishes these goals. I'll begin my changes as soon as #2139 gets merged and there's a feature lock for 15.

enhancement

Most helpful comment

As a tiny demonstration of one piece of this, consider this partial TS reimplementation of the graphql-js codebase:

// This is a reimplementation of GraphQLScalarType with generics.
class GraphQLScalarType<TInternal, TExternal> {
  internal: TInternal;
  external: TExternal;
}

// Here are two of our built-in scalars.
const GraphQLString = new GraphQLScalarType<string, string>();
const GraphQLInt = new GraphQLScalarType<number, number>();

// We want to extract the type, using the configuration object in the `type`
// field as our source of truth. If it's set to `GraphQLString`, then we want to
// make sure that the resolver in fact returns a string.
//
// In the real-world, there are many more factors (complex types, nullable
// types, lists, default resolvers, async resolvers, etc) but this is just a
// minimal example.
type FieldConfig<TConfig> = TConfig extends GraphQLScalarType<
  infer TInternal,
  any
>
  ? {
      type: TConfig;
      resolver: () => TInternal;
    }
  : never;

// Our dummy reimplementation of `GraphQLObjectType` can infer the field types
// from the config object, so we don't actually have to pass in this generic
// parameter.
class GraphQLObjectType<
  T extends {
    [K in keyof T]: T[K] extends { type: infer V } ? FieldConfig<V> : never;
  }
> {
  constructor(config: { name: string; fields: T }) {
    // Setup happens here...
  }
}

Using this, it's perfectly legal to create the following Person type:

const Person = new GraphQLObjectType({
  name: "Person",
  fields: {
    name: { type: GraphQLString, resolver: () => "hello" },
    age: {
      type: GraphQLInt,
      resolver: () => 12345
    }
  }
});

However, if we change the age resolver to return a string, our TypeScript will report an error, and prevent us from ever shipping bad code!

image

All 7 comments

As a tiny demonstration of one piece of this, consider this partial TS reimplementation of the graphql-js codebase:

// This is a reimplementation of GraphQLScalarType with generics.
class GraphQLScalarType<TInternal, TExternal> {
  internal: TInternal;
  external: TExternal;
}

// Here are two of our built-in scalars.
const GraphQLString = new GraphQLScalarType<string, string>();
const GraphQLInt = new GraphQLScalarType<number, number>();

// We want to extract the type, using the configuration object in the `type`
// field as our source of truth. If it's set to `GraphQLString`, then we want to
// make sure that the resolver in fact returns a string.
//
// In the real-world, there are many more factors (complex types, nullable
// types, lists, default resolvers, async resolvers, etc) but this is just a
// minimal example.
type FieldConfig<TConfig> = TConfig extends GraphQLScalarType<
  infer TInternal,
  any
>
  ? {
      type: TConfig;
      resolver: () => TInternal;
    }
  : never;

// Our dummy reimplementation of `GraphQLObjectType` can infer the field types
// from the config object, so we don't actually have to pass in this generic
// parameter.
class GraphQLObjectType<
  T extends {
    [K in keyof T]: T[K] extends { type: infer V } ? FieldConfig<V> : never;
  }
> {
  constructor(config: { name: string; fields: T }) {
    // Setup happens here...
  }
}

Using this, it's perfectly legal to create the following Person type:

const Person = new GraphQLObjectType({
  name: "Person",
  fields: {
    name: { type: GraphQLString, resolver: () => "hello" },
    age: {
      type: GraphQLInt,
      resolver: () => 12345
    }
  }
});

However, if we change the age resolver to return a string, our TypeScript will report an error, and prevent us from ever shipping bad code!

image

Make interface DocumentNode generic, like DocumentNode<Result, Variable>, so many community tools can use it.

For example:
@apollo/hooks can just useQuery(gqls.xxx_document) instead of useQuery<Result, Variable>(gqls.xxx_document), and the type of gqls.xxx_document can be declared by graphql-code-generator

How would you reference a type in its own initializer?

image

By giving it a type annotation?

export const BooType: GraphQLObjectType<any, Context, any> = new GraphQLObjectType<any, Context, any>(...)

Does type: () => BooType work?

Great question! @Janpot is correct - in most cases the type parameters can be inferred from the configs, providing type safety without explicitly declaring types. However, it's certainly possible to be explicit with these parameters (which is necessary in certain cases like this). TypeScript will treat any explicit parameters as canon, and will use them to validate matching fields configs.

I recently opened a PR for using type inference in DocumentNode to allow clients to automatically infer types from generics: https://github.com/graphql/graphql-js/pull/2728
I think if we'll go with this one, we can always try to use the inference/generics for similar improvements.

Was this page helpful?
0 / 5 - 0 ratings