dotansimha / graphql-code-generator

A tool for generating code based on a GraphQL schema and GraphQL operations (query/mutation/subscription), with flexible support for custom plugins.
https://the-guild.dev/graphql/codegen/
MIT License
10.85k stars 1.33k forks source link

Types for subscription operations seem to have an extra wrapper #1632

Closed jonaskello closed 5 years ago

jonaskello commented 5 years ago

Describe the bug

Types for subscription operations seem to have an extra wrapper. Maybe this is an effect of #1453 where the wrapper seems to have been removed for the resolver, but it is not removed for the operation types?

To Reproduce Steps to reproduce the behavior:

  1. My GraphQL schema:
type Subscription {
  postAdded: Post
}

type Query {
  posts: [Post]
}

type Post {
  author: String
  comment: String
}
  1. My GraphQL operations:
subscription MyPostAdded {
  postAdded {
    author
  }
}
  1. My codegen.yml config file:
schema:
  - "schema.graphql"
overwrite: true
generates:
  generated.ts:
    documents: "query.graphql"
    plugins:
      - "typescript"
      - "typescript-operations"

Expected behavior

This is generated for the resolvers (which I think is correct according to #1453), there is no wrapper for postAdded:

export type SubscriptionResolvers<Context = any, ParentType = Subscription> = {
  postAdded?: SubscriptionResolver<Maybe<Post>, ParentType, Context>;
};

This is generated for the operations, here we still have a wrapper for postAdded:

export type MyPostAddedSubscription = { __typename?: "Subscription" } & {
  postAdded: Maybe<{ __typename?: "Post" } & Pick<Post, "author">>;
};

I would expect it to instead generate this for the operation (no wrapper, same as the resolver):

export type MyPostAddedSubscription =
  postAdded: Maybe<{ __typename?: "Post" } & Pick<Post, "author">>;

Environment:

Additional context

I think the confusion is that a subscription can only have a single root-field by the specification. This means no wrapper is needed?

jonaskello commented 5 years ago

Actually I don't think this error report is accurate :-).

I am getting some strange behaviour though and I'm not sure the typings are 100% correct.

I have somthing like this resolver:

export const subscriptionResolver: SubscriptionResolvers<Context, RootValue> = {
  postAdded: {
    subscribe: (parent, args, context) => {
      return pubsub.postAddedAsyncIterator();
    },
    resolve: (payload, args, context) => {
      return payload as any;
    },

At run-time it seems there is a difference between the first parameter in subscribe and resolve, that is parent and payload above. For parent i get RootValue, for payload I get the object returned by the iterator. However the typings seem to be generated in a way that assumes they are of the same type, see below:

export interface SubscriptionResolverObject<TResult, TParent, TContext, TArgs> {
  subscribe: SubscriptionSubscribeFn<TResult, TParent, TContext, TArgs>;
  resolve?: SubscriptionResolveFn<TResult, TParent, TContext, TArgs>;
}

This is why I need the return payload as any. Also if I leave off the resolve function it does not work, complaining that I am returning null for the field. I think in this case it should have some default behaviour, like picking the field postAdded but this will not work since payload is not wrapped in something that has a postAdded field.

dotansimha commented 5 years ago

@kamilkisiela can you please take a look?

const86 commented 5 years ago

I believe, operations interface is correct, but resolvers are wrong. At least for the cases when resolve callback is not used, like I do in my app. For the subject schema, the resolver could be

const resolvers: SubscriptionResolvers = {
  Subscription: {
    postAdded: {
      async *subscribe() {
        yield {
          postAdded: { author: "Foo Bar", content: "Lorem ipsum..." }
        }
      }
    }
  }
};

Note the postAdded property in the yielded object. Generated interfaces don't have that so typescript barfs at here. But it is actually needed and without it graphql's subscribe() fails and nullifies the result.

const86 commented 5 years ago

I have put together the complete example to avoid any further ambiguity. Here is my schema.graphql:

type Query {
  foo: Int!
}

type Subscription {
  bar: Int!
}

schema {
  query: Query
  subscription: Subscription
}

and bar-op.graphql:

subscription bar {
  bar
}

Then boring codegen.yml:

schema: schema.graphql
documents: ./*-op.graphql
config:
  avoidOptionals: true
generates:
  ./resolvers.ts:
    plugins:
      - typescript
      - typescript-resolvers
  ./operations.ts:
    plugins:
      - typescript
      - typescript-operations

And finally, the code to bind them all (sorry for some boilerplate at the beginning):

// BOILERPLATE BEGIN

import { parse, subscribe, Source } from "graphql";
import { makeExecutableSchema } from "graphql-tools";
import { readFile } from "fs";
import { Resolvers, SubscriptionResolvers } from "./resolvers";
import { BarSubscription } from "./operations";
import { $$asyncIterator } from "iterall";

(Symbol as any).asyncIterator = Symbol.asyncIterator || $$asyncIterator;

const readFileAsync = (path: string) =>
  new Promise<string>((resolve, reject) =>
    readFile(path, (err, data) =>
      err === null ? resolve(data.toString()) : reject(err)
    )
  );

const loadSource = (path: string) =>
  readFileAsync(path).then(body => parse(new Source(body, path)));

async function* only<T>(value: T) {
  yield value;
}

const asIterable = <T>(value: AsyncIterator<T> | T) => ({
  [Symbol.asyncIterator]: () => ("next" in value ? value : only(value))
});

// BOILERPLATE END

// My favorite option, does not work without nasty `as any`
const bar0: SubscriptionResolvers["bar"] = {
  async *subscribe() {
    yield { bar: 42 };
  }
} as any;

// Dummy resolve callback to make generated interfaces work
const resolve = <TResult, TParent>(parent: TParent) =>
  (parent as unknown) as TResult;

const bar1: SubscriptionResolvers["bar"] = {
  async *subscribe() {
    yield 42;
  },
  resolve
};

// Matches the interface, but simply does not work
const bar2: SubscriptionResolvers["bar"] = () => ({
  async *subscribe() {
    yield 42;
  },
  resolve
});

const resolvers: Resolvers = {
  Query: { foo: () => 42 },
  Subscription: { bar: bar0 }
};

(async function() {
  const typeDefs = await loadSource("schema.graphql");
  const schema = makeExecutableSchema({ typeDefs, resolvers });

  const document = await loadSource("bar-op.graphql");
  const result = await subscribe<BarSubscription>({ schema, document });

  for await (const { data, errors } of asIterable(result)) {
    if (errors) {
      throw errors;
    }

    console.log(data);
  }
})().catch(console.error);

Option 0 is the best, according to me, but it does not match generated interface. I would like to fix that. Option 1 feels like a hack to get something working. If one wants to use resolve callback, generated interface should allow the subscribe to yield arbitrary type which becomes the type of the first argument of the resolve callback. Not sure if such is achievable in Typescript. Option 2 matches the generated interface but does not work. I cannot see how it could work with graphql-js. Is it for some alternative server implementation?

kamilkisiela commented 5 years ago

Schema

type Subscription {
  message: Message!
}

type Message {
  text: String!
}

I would say there are at least three ways of dealing with subscriptions.

1. Any event + resolve function:

// somewhere in code
pubsub.publish('message', 'hi'); // emits a string

// ...

const resolvers = {
  Subscription: {
    message: {
      subscribe() {
        return pubsub.asyncIterable('message') // receives a string
      },
      resolve(text) {
        return { text }; // resolves `Message`
      }
    }
  }
}

2. An event with subscription's name

// somewhere in code
pubsub.publish('message', { message: { text: 'Hi'} }); // emits subscription's name + Message

// ...

const resolvers = {
  Subscription: {
    message: {
      subscribe() {
        return pubsub.asyncIterable('message') // receives `{ message: Message }`
      }
    }
  }
}

3. An event with valid result

// somewhere in code
pubsub.publish('message', { text: 'Hi'}); // emits a Message object

// ...

const resolvers = {
  Subscription: {
    message: {
      subscribe() {
        return pubsub.asyncIterable('message') // receives `Message`
      },
      resolve(payload) { // It's weird that it won't work until we return payload here
        return payload;
      }
    }
  }
}

In order to solve the issue we need to cover those three use cases.

Here's an example repository: https://github.com/kamilkisiela/graphql-subscription-cases

const86 commented 5 years ago
interface SubscriptionResolverObject<TResult, TKey extends string, TParent, TContext, TArgs> {
  subscribe: SubscriptionSubscribeFn<X, TParent, TContext, TArgs>;
  resolve: SubscriptionResolveFn<TResult, X, TContext, TArgs>;
}

I can't figure out how to express this in real Typescript. The X type can be anything, but we don't want it to leak into SubscriptionResolverObject's arguments. (Special case when X extends { [key in TKey]: TResult } make resolve optional. No brainer compared to the first problem.)

kamilkisiela commented 5 years ago

Exactly, I struggled to do it too, tried with infer but we need to know an object in order to use infer and make decisions. One tough thing...

const86 commented 5 years ago

How about this to keep existing code with resolve working, even though without complete type checking?

interface SubscriptionResolverObject0<TResult, TKey extends string, TParent, TContext, TArgs> {
  subscribe: SubscriptionSubscribeFn<{ [key in TKey]: TResult }, TParent, TContext, TArgs>;
  resolve?: never;
}

interface SubscriptionResolverObject1<TResult, TParent, TContext, TArgs> {
  subscribe: SubscriptionSubscribeFn<any, TParent, TContext, TArgs>;
  resolve: SubscriptionResolveFn<TResult, any, TContext, TArgs>;
}

type SubscriptionResolverObject<TResult, TKey extends string, TParent, TContext, TArgs> =
  | SubscriptionResolverObject0<TResult, TKey, TParent, TContext, TArgs>
  | SubscriptionResolverObject1<TResult, TParent, TContext, TArgs>;

Also, what about that option the whole resolve to be a function returning SubscriptionResolverObject? Where is it useful? I believe, graphql's subscribe() fails on such resolvers.

dotansimha commented 5 years ago

Fixed in 1.6.0.