pothos icon indicating copy to clipboard operation
pothos copied to clipboard

Combination of "expose" and GraphQL JIT can cause a resolve failure

Open fabulator opened this issue 2 years ago • 1 comments

Hello,

I have found a problem when using "expose" and GraphQL JIT. I have following example, that is correctly compiled by typescript:

import { createYoga } from 'graphql-yoga';
import { createServer } from 'node:http';
import SchemaBuilder from '@pothos/core';
import { useGraphQlJit } from '@envelop/graphql-jit'

const builder = new SchemaBuilder({ });

class User {
    name: Promise<string>;
    constructor() {
        this.name = Promise.resolve('name');
    }
}

builder.objectType(User, {
    name: 'User',
    fields: (t) => ({
        name: t.exposeString('name'),
    }),
})

builder.queryType({
    fields: (t) => ({
        user: t.field({
            type: User,
            resolve: () => new User(),
        }),
    }),
});

const yoga = createYoga({
    schema: builder.toSchema(),
    plugins: [useGraphQlJit()],
});

const server = createServer(yoga);

server.listen(3000, () => {
    console.log('Visit http://localhost:3000/graphql');
});

I'm trying query

query {
  user {
    name
  }
}

When JIT is not in plugins, I get correct response. When I use JIT, I see error String cannot represent value: {}. Because the name is Promise.

In Readme of JIT there is: "The primary limitation is that all computed properties must have a resolver and only these can return a Promise." Which is probably the problem. Fix is simple. I just put a resolver to property and it's working:

builder.objectType(User, {
    name: 'User',
    fields: (t) => ({
        name: t.string({ resolve: (user) => user.name }),
    }),
})

I was just wondering if there could be some settings for the builder, that could enforce that exposeString would required for property to be type of string and not promise. It could be usefull for JIT users and it would make usage with JIT more type safe.

fabulator avatar Dec 10 '23 18:12 fabulator

I am open to suggestions on how this could be handled, but there isn't anything existing in the type system to support these kinds of changes. I recognize the problem but not sure if this is something that belongs in core.

I think a potential solution would be 2 changes:

  • Add a forceResolver: boolean option to expose fields
  • Add an extendable interface for expose fields that could be extended by a plugin
    • This would enable a plugin to make the forceResolver required to be true if the exposed field is a promise.

I'm not sure how complex this would be, or if this is the best path forward, but that's the best thing that comes to mind immediately

hayes avatar Dec 11 '23 20:12 hayes