API suggestions

[miloofcroton]

I have a hunch that the choices made, which I'm about to question, came with tradeoffs regarding type inference and brevity (a "less brevity here, more brevity there" type of thing).

callback pattern on fields

I'm curious why you went with the callback pattern. Either to cut down on imports or to allow something dynamic on the value of t, I'm guessing.

For instance, using an objectRef definition as example:

export const CommentObject = builder
  .objectRef<Comment>('Comment')
  .implement({
    fields: (t) => ({
      id: t.exposeID('id'),
    )}
  )}

rather than a plain object pattern:

export const CommentObject = builder
  .objectRef<Comment>('Comment')
  .implement({
    fields: {
      id: exposeID('id'),
    },
  )}

This is not a huge deal, but it makes the code more cluttered in my opinion.

This is more noticeable when developing resolvers as independent variables:

export const getPost = (t: PothosT) => t.field({
  type: PostObject,
  nullable: true,
  args: {
    id: t.arg.id({ required: true }),
  },
  resolve: (root, args) => [...Posts.values()]
    .find(post => post.id === String(args.id)),
})

which have to be imported and called like so:

import { getPost, getPosts } from '../posts/resolvers';
import { getUser, getUsers } from '../users/resolvers';

builder.queryType({
  fields: (t) => ({
    getPost: getPost(t),
    getPosts: getPosts(t),
    getUser: getUser(t),
    getUsers: getUsers(t),
  }),
});

It would be nice to be able to just do this:

import * as postsQueries from '../posts/queries';
import * as usersQueries from '../users/queries';

builder.queryType({
  fields: ({
    ...postsQueries,
    ...usersQueries,
  }),
});

sidenote: Nexus uses a callback pattern too, but their API is more indirect, making this necessary. In the below example, you don't even have keys on the definition (in pothos, this would be fields essentially), so you have to call t.string and other t methods just to register a field with a key. Not a fan of how they do this at all.

const Book = objectType({
  name: "Book",
  definition(t) {
    t.string("title", { nullable: true })
    t.string("author", { nullable: true })
  },
})

extending builder vs importing to builder

I'm curious what the pros and cons are here. To me, extending builder via dot methods like objectRef

export const builder = new SchemaBuilder({
  plugins: [...]
});

import { builder } from './schema'

export const UserObject = builder
  .objectRef<User>('User')
  .implement({
    fields: (t) => ({
      id: t.exposeID('id'),
    )}
  )}

is less clean than importing to a builder function.

export const UserObject = objectRef<User>('User')
  .implement({
    fields: (t) => ({
      id: t.exposeID('id'),
    )}
  )}

import * as userObjects from './users/objects'
import * as postObjects from './posts/objects'

import * as userQueries from './users/queries'
import * as postQueries from './posts/queries'

// note, you just have the schema without having to call `toSchema` because there's no necessary mutability on `builder`.
export const schema = new SchemaBuilder({
  plugins: [...],
  objects: {
    ...userObjects,
    ...postObjects,
  },
  query: {
    ...postsQueries,
    ...usersQueries,
  },
});

I'm guessing this suggestion might make certain TypeScript things harder to do (maybe impossible), but that's without digging too deeply. Curious to hear your thoughts.

[hayes]

Yes, the changes you are suggesting look a little cleaner for some use cases, but make a lot of common, but slightly more advanced patterns impossible. Some of this comes down to type-safety/inference, some of it comes down to how plugins work. There is also a lot of complexity hidden in how pothos lazy-loads the field functions that helps avoid common problems with circular references, circular imports and other nasty side effects of trying to define a graph with circular relations.

Some more concrete examples:

export const CommentObject = builder
  .objectRef<Comment>('Comment')
  .implement({
    fields: {
      id: exposeID('id'),
    },
  )}

In this example, exposeID has no way to know anything about Comment, which means it can't enforce that id is a field that exists on Comment. We could do something in the fields to make this specific case work, but the expose methods are just convenience wrappers around t.field. t.field needs to know the type of Comment to be able to safely type the resolver (and many other plugin options available on fields). It is definitely a little more verbose, but getting a good/type-safe DX without the wrapping function is not possible here.

export const getPost = (t: PothosT) => t.field({
  type: PostObject,
  nullable: true,
  args: {
    id: t.arg.id({ required: true }),
  },
  resolve: (root, args) => [...Posts.values()]
    .find(post => post.id === String(args.id)),
})

The better alternative here is to do something like:

builder.objectField(PostObject, 'getPost', t => t.field({
  type: PostObject,
  nullable: true,
  args: {
    id: t.arg.id({ required: true }),
  },
  resolve: (root, args) => [...Posts.values()]
    .find(post => post.id === String(args.id)),
}))

Builder pattern vs composition

import * as userObjects from './users/objects'
import * as postObjects from './posts/objects'

import * as userQueries from './users/queries'
import * as postQueries from './posts/queries'

// note, you just have the schema without having to call `toSchema` because there's no necessary mutability on `builder`.
export const schema = new SchemaBuilder({
  plugins: [...],
  objects: {
    ...userObjects,
    ...postObjects,
  },
  query: {
    ...postsQueries,
    ...usersQueries,
  },
});

This is closer to how the original versions of Pothos worked. This works okay at small scales, but for large graphs this becomes a pain to manage. I also know from experience remembering to import and add the right types to the right places is something many skilled developers still struggled with, and * imports also cause a lot of issues with people accidentally exporting something other than builder types.

Whether or not a builder pattern vs a composition pattern is better is probably a large question, and might get into very opinionated territory. More practically speaking, there are a lot of good use cases for taking the same schema definitions and building them with different options. There are a few plugins that take advantage of this (mocks, sub-graph, federation, auth). Using a builder pattern allows having all the definitions created once, and then creating schemas with different transforms, features, or filters applied. This can also be achieved in other ways, but over all, based in the feedback I got on early versions of Pothos using the current pattern of creating objects through the builder, and removing the need to import all schema types was a very posative change.

Another big reason the pattern suggested above would not work is that the API, and features of the builder change drastically based on options passed in when creating the builder. The builder carries type information for all kinds of things from types for custom scalars, to plugin features like authorization, to resolver contexts, etc. All of this type information, and all the other settings provided to the builder would need to somehow be provided to all the other files that define schema types.

[miloofcroton]

Thank you for the detailed explanation. This is helping me think a new way, regarding the builder pattern.

I've been using builder.objectRef for each graphql object's creation and exporting callback functions (with t) for import into a single builder.queryType call that adds each resolver as a field manually (as explained above). I didn't know that I could call builder.queryFields (and do so multiple times) instead, and I didn't know I could call builder.objectType instead of builder.objectRef for the object creation. I admittedly got into this situation by trying to refactor a really simple example.

So, this raises some questions. What exactly are the best practices for a scalable solution?

1. Is there any way to reference object types without saving them as variables (ie not saving the return of builder.objectRef and importing it into another object manually)? I was stuck in composition thinking, but now I'm thinking builder style. Best practices for creating GraphQL objects?

2. Having a single builder.queryFields is nicer than calling builder.objectField for every single resolver because you have to say something like the first example below, instead of the second example below, which uses the name as a key instead of a string param in a generator function, and it calls UserObject once instead of twice. All that being said, are there any best practices or tips I should be considering for resolvers?

builder.objectField(
  UserObject,
  'getUser',
  t => t.field({
    type: UserObject,
    nullable: true,
    args: {
      id: t.arg.id({ required: true }),
    },
    resolve: (root, args) => Users
      .filter(user => user.id === String(args.id)),
  })
)

builder.queryFields((t) => ({
  getUser: t.field({
    type: UserObject,
    nullable: true,
    args: {
      id: t.arg.id({ required: true }),
    },
    resolve: (root, args) => Users
      .filter(user => user.id === String(args.id)),
    })
  }),

  // you can add more resolvers here without calling `builder.queryFields` again and setting up another callback!  
}))

Edit: Perhaps I should explain something, regarding what I'm trying to do with app design and why I have tried to refactor example boilerplate in this way.

Some people have just one layer of organization, which is the technology type, in their app. In other words, a file for controllers, a file for resolvers, a file for the server, etc. This is only really suitable for a hello world app.

Some people have two layers of organization, which is a folder for controllers/resolvers/components/etc and then a file for each entity name in there (a users.ts file under /controllers, a users.ts file under /resolvers, a posts.ts file under /controllers, etc).

I prefer something else. For starters, an app-level organization at the top that isn't based on technology but based on area of concern. So, my full stack apps often have folders for "resources", "utils", "app", "components", etc. Under resources (and sometimes "components"), I have a folder for each entity, such as /users, /posts, etc. Then within those files, there's objects.ts, resolvers.ts, mocks.ts, etc. There's no strict number of files (sometimes I combine them or separate them), but the goal is domain organization before technology organization.

So with that in mind, I prefer a certain way of importing/exporting things, and I'm not sure how well the builder pattern jives with this. I'm trying to figure out if I have to import every single file that calls builder at a root point if I want to avoid importing resolvers manually the way I was originally.

[hayes]

Is there any way to reference object types without saving them as variables (ie not saving the return of builder.objectRef and importing it into another object manually)? I was stuck in composition thinking, but now I'm thinking builder style. Best practices for creating GraphQL objects?

There are 3 main ways to reference Object types:

1. You can create an object ref (const MyObject = t.objectRef('SomeName')), refs are also returned by any other method that creates an object type.
2. You can use classes as refs. You can just pass in a class anywhere that accepts an object ref (both for defining types, and for defining the return type of a field)
3. You can use strings (just the name of the type). This can be nice if you don't want to import things from other files. To do this, you need to map each type name to a type when you create the builder. new SchemaBuilder<{ Objects: { MyType: SomeType}>({...})

Having a single builder.queryFields is nicer than calling builder.objectField for every single resolver because you have to say something like the first example below, instead of the second example below, which uses the name as a key instead of a string param in a generator function, and it calls UserObject once instead of twice. All that being said, are there any best practices or tips I should be considering for resolvers?

Not really, this is all just personal preference. I would agree that when you are defining multiple fields queryFields is cleaner. If you are just defining one field, then maybe queryField makes more sense. There are similar methods for mutations, subscriptions, objects, and interfaces as well (eg. t.objectFields(SomeObject, t => ({...}))

So with that in mind, I prefer a certain way of importing/exporting things, and I'm not sure how well the builder pattern jives with this. I'm trying to figure out if I have to import every single file that calls builder at a root point if I want to avoid importing resolvers manually the way I was originally.

Pothos tries to be unopinionated about how you structure your apps, but I have some personal best practices I tend to use. Some basic patterns are described here: https://pothos-graphql.dev/docs/guide/app-layout and then some more examples for specific types of issues described here: https://pothos-graphql.dev/docs/guide/circular-references

These are things I have done in some of my apps, and are not necessarily required to use pothos:

• A builder.ts that defines the builder and DOES NOT IMPORT any other files that reference any other parts of the schema. This should be as self contained as possible

• A directory of files with type definitions, this can be one file per type, or even have multiple files per type nested in another folder like you described.

  • I'll often put a type, along with queries/mutations for that type, and related input types/enums all into one file if its not too big.
  • I like co-locating queries with types rather than having all the queries/mutations in one big file
  • Sometimes I'll use t.objectField to add fields to other objects for the current type (eg User.ts might defined the author field on a post, if that resolver is complicated, or if I need to attach a similar field to a bunch of types and want them all together).

• schema.ts should import all files that define any parts of a schema.

• imports should be of the form import './path/to/file' and should not import named or default exports

• Don't use index.ts file to re-export values. You can have index files that use imports like import './path/to/file', but do not use export * from './path/to/file. This is just to make it easier to to import all the files from schema (eg you could have schema.ts import objects.ts, interfaces.ts, etc, then objects.ts import all files that implement objects.

• All imported references to other types should be imported directly from the file that defines them, not from some aggregate or index file

• I generally only export type definitions type refs that I expect to use in other files (rather than defaulting to exporting everything)

• I often create separate typescript definitions

• I try to use import type { Foo } from ... when possible

• I try to keep my context objects smaller (eg things like database clients, and anything not specific to the request I usually import from a file rather than accessing it through the context object.

Hope some of these help. Not everything above is super important, and it definitely does not apply to all situations, but just some quick general patterns I've tended towards in my own apps.