Issue when returning Union (Abstract type)

[nadirBenSaid]

So, I am using type-graphql with express-graphql and graphql-jit.

Here is how I use graphql-jit within express-graphql :

function resolveGraphqlHTTPOptions(schema: GraphQLSchema): Options {
  const cache: any = {};
  return (req: Request, res: unknown, params: any) => {
    const { query } = params;
    if (query && !(query in cache)) {
      const document = parse(query);
      cache[query] = compileQuery(schema, document);
    }
    return {
      schema,
      graphiql: !isProduction,
      customFormatErrorFn,
      customExecuteFn: (args: ExecutionArgs) => {
        const { rootValue, contextValue, variableValues } = args;
        return cache[query].query(rootValue, contextValue, variableValues);
      },
    };
  };
}

At first everything works fine before I add a query that returns a Union type but the moment I use a query that returns a Union, I get the error below:

Abstract type <Name>Union must resolve to an Object type at runtime for field Mutation.<Mutation_Name>. Either the <Name>Union type should provide a \"resolveType\" function or each possible types should provide an \"isTypeOf\" function.

This error only manifests when a mutation/query returns a Union. Other than that every mutation/query works just fine. when I disable graphql-jit (by commenting the customExecuteFn), everything works fine.

BTW, thanks for this awesome lib ❤️

[ruiaraujo]

I believe this to be a duplicate of #69. While resolving the type a promise is returned and this is not handled cleanly. As a starter we can return a proper error message instead of this confusing message.

[nadirBenSaid]

Hello @ruiaraujo, thanks for your reply. I agree with you, a proper error message would definitely be more helpful!
Earlier, I updated to the latest version of type-graphql (v1.0.0) in which they have updated their TypeResolver interface to match with GraphQLTypeResolver from graphql-js, unfortunately, it doesn't solve the issue, I also considered using a fork of type-graphql as in #69 but decided not to after reading this thread.
So currently I'm wondering if there are any plans to patch how graphql-jit handles this case?

[ruiaraujo]

The type has changed, but the behaviour is most likely the same.

[maapteh]

Same issue as above. I used https://github.com/zalando-incubator/graphql-jit/blob/master/examples/blog-apollo-server/src/executor.ts and saw our union types not working anymore.

Maybe worthwile to add it to the docs that the async usecase is also not being supported with async TypeResolvers? Since the example and readme made me think i shouldnt expect problems because im not using @skip or @include yet. But when its not usable when having Union types that's a totally different story ...

Im using graphql-modules and see __resolveType actually being called but then the above error is given. Im using a dataloader for the data and the type resolver is just simple one. But as you can see below the result of the resolveType is indeed async one being generated in my setup.

I will debug little further, since it doesnt happen in the example when i make stuff async and introduce Union type there https://github.com/maapteh/graphql-jit/tree/bug/union-types/examples/blog-apollo-server

branches:

 maapteh:bug/union-types => bare reproduction no issues
 maapteh:bug/union-types-tools => on top of bare using graphql-tools and no issues
 maapteh:bug/union-types-modules => on top of tools using graphql-modules and having the issue

So in my case its graphql-modules.

But GraphQLTypeResolver can be Value or Promise according to graphql-js specs so the main issue should not happen in my opinion. Do you consider fixing this? Btw i like the effort to make Apollo faster, in our case it was 2x but still Apollo seems to be the worst performant one so every addition to make it faster is nice to have. So your library is very promising.

[ex3ndr]

We are also facing this issue and unfortunately it is a blocker for us( we are not using type-graphql.

[StarpTech]

But GraphQLTypeResolver can be Value or Promise according to graphql-js specs so the main issue should not happen in my opinion

Hi, it would be great to address this issue if it is spec-compliant. Would you accept a PR?