Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
fabulator opened this issue Dec 10, 2023 · 1 comment
Open

Comments

@fabulator
Copy link

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.

@hayes
Copy link
Owner

hayes commented Dec 11, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants