stepci/garph

How to handle field level resolvers?

christopher-caldwell opened this issue ยท 21 comments

Say you have a relationship Authors to Books. The Book type has a field author, which could be a field level resolver to avoid the n+1 problem. Defining the schema for this would be like:

const authorType = g.type('Author', {
  name: g.string(),
})

const bookType = g.type('Book', {
  title: g.string(),
  author: g.ref(bookType)
})

const queryType = g.type('Query', {
  books: g.ref(authorType).list(),
})

The resolver would be something like:

const resolvers: InferResolvers<{ Query: typeof queryType }, {}> = {
  Query: {
    async books(parent, args, context, info) {
      const allBooks = db.findAllBooks()
      // does not include author - TS error saying it's missing `author`
      return allBooks
    },
  },
}

Is it possible for this to be setup to know that there's a field level resolver called author somewhere that will satisfy the requirement? As is, this is a TS error, and I have to use a DeepOmit utility type to ignore the need for books.

So far, I have thought about it being optional, but in reality, it's not optional. The schema needs to say this is required, but the resolver needs to be allowed to return without it. This is super challenging for TS to know there's a field resolver somewhere, but at least there could be something like g.fieldResolver() which makes the field optional when using InferResolvers. If someone leaves it off, it will throw a GraphQL error.

If it's helpful, I can make a sample repo showing the issue.

You're welcome to contribute an example!
I'm also considering adding resolvers directly to the fields

Also, could you try to do the same thing using SDL and graphql-codegen and tell me whether the result matches what you expected? eg. Did you still have to use DeepOmit there?

Okay, I will put together an example.

I have the same issue with codegen, and it's the origin of the DeepOmit lol. So it's not something that's lacking, more so trying to figure out the best way to make it work.

If the behaviour is the same as with codegen, then it's "correctly" implemented on our side too. This doesn't mean we can't make improvements though ๐Ÿ˜„

Will check out your example once it's ready and see what we can do
Thanks

@mishushakov https://github.com/christopher-caldwell/garph-field-resolver-demo

If I left anything out, happy to clear it up! Thanks for taking the time to look.

Great, will take a look!

Any ideas?

Will see on the weekend. Thanks for the reminder in any case!

Sorry, I was a bit overwhelmed last weekend. I will you know when I have something ๐Ÿ™

Thank you!

I can try to contribute if you point me in the right direction.

What I believe @christopher-caldwell is talking about is the issue and question I am running into as well.

As an example illustrated below - the profile doesn't exist on the object but is a derived field from the resolver so returning { id: string } doesn't satisfy the expected return type { id: string, profile: Profile }. What should we return here? I'd hope to not have to cast it.

const User = g.type("User", {
  id: g.id(),
  profile: g.ref(Profile)
});

const Profile = g.type("Profile", {
  name: g.string(),
  bio: g.string(),
});

const Query = g.type("Query", {
  viewer: g.ref(User).optional()
});

const resolvers: InferResolvers<{ Query: typeof Query }, { context: Context, User: typeof User }> = {
  Query: {
    viewer: (root, args, context, info) => {
      /*
       TypeScript Error
       Property 'profile' is missing in type 'User' but required in type '{ __typename?: "User" | undefined; id: string; profile: { __typename?: "Profile" | undefined; name: string; bio: string; }; }'
      */
      return context.currentUser ?? null;
    }
  },
  User: {
    profile: async (parent, args, context, info) => {
      return await db.findProfileByUserId(parent.id);
    }
  }
};

@erickreutz I think you are missing the Infer generic.

Taking your snippet, this works for me:

import { g, Infer, InferResolvers } from 'garph'

interface Context {
  currentUser: Infer<typeof User>
}
const Profile = g.type('Profile', {
  name: g.string(),
  bio: g.string(),
})

const User = g.type('User', {
  id: g.id(),
  profile: g.ref(Profile),
})

const Query = g.type('Query', {
  viewer: g.ref(User).optional(),
})

export const resolvers: InferResolvers<{ Query: typeof Query }, { context: Context }> = {
  Query: {
    viewer: (root, args, context, info) => {
      return context.currentUser ?? null
    },
  },
}

I also don't think these issues are related. Again taking your example, my issue is that I'd like to define the User in one file, and the Profile in another. This isn't currently possible, unless you pass a singular reference to g through function arguments in a factory pattern.

Edit: My bad, I was referencing a different issue.

@christopher-caldwell Hmm I'm not sure you are following then. currentUser is not a graphql object. It's a record from the db representing the current user of the request. Illustrating an example that doesn't use the context results in the same issue.

const User = g.type('User', {
  id: g.id(),
  firstName: g.string(),
  lastName: g.string(),
  fullName: g.string(), // this is a resolved field
})

const Query = g.type('Query', {
  viewer: g.ref(User).optional(),
})

export const resolvers: InferResolvers<{ Query: typeof Query, User: typeof User }, {}> = {
  Query: {
    viewer: (root, args, context, info) => {
      // Property 'fullName' is missing in type '{ id: string; firstName: string; lastName: string; }' but required in type '{ __typename?: "User" | undefined; id: string; firstName: string; lastName: string; fullName: string; }
      return {
        id: '1',
        firstName: 'John',
        lastName: 'Doe',
      }
    },
  },
  User: {
    fullName: (root, args, context, info) => {
      return `${root.firstName} ${root.lastName}`;
    }
  }
}

Much like how your author is not part of the Book and is resolved fullName is not part of User but is resolved. As in my first example profile is not part of User but is resolved. We need some way to mark a field as being resolved something like g.resolved(g.string()) or g.string().resolved() or define the resolvers inline with the type.

@erickreutz Ahh okay! Yeah, I understand now. My bad.

Yeah having a way to mark the field as resolved by something else, exactly what you illustrate here, is my issue as well.

In a perfect world, if you marked a field as such (resolved or field level, whatever makes sense) and you did not declare it in the User resolvers block, it should throw a TS error.

const User = g.type('User', {
  id: g.id(),
  firstName: g.string(),
  lastName: g.string(),
  fullName: g.string().fieldLevel() // whatever name makes sense
})

const resolvers: InferResolvers<{ Query: typeof Query, User: typeof User }, {}> = {
  Query: {
    viewer: '...'
  },
  User: {
   // If this was omitted, but declared above as a field level resolver, TS should throw.
    fullName: (root, args, context, info) => {
      return `${root.firstName} ${root.lastName}`;
    }
  }
}

Anything new here?

Hey, just looked into this more deeply

Here are my findings:

  • Currently, you cannot skip resolving a field unless you make it optional
  • Resolving with a function, like in your data-loader example is now a valid pattern (since v0.5.2)
  • Marking fields as "resolved" is a great Idea, I will totally consider this

Sorry for the wait ๐Ÿ˜

Here's a demo of upcoming omitResolver. Once specified you can now skip resolving this value in the root resolver. You can combine it with InferStrict

Screen.Recording.2023-05-24.at.14.12.46.mov

Let me know if you have any feedback?

.omitResolver was added in 0.5.4