hayes / pothos

Pothos GraphQL is library for creating GraphQL schemas in typescript using a strongly typed code first approach
https://pothos-graphql.dev
ISC License
2.33k stars 160 forks source link

[Prisma plugin] Should selections be available after mutation? #448

Open joshleitzel opened 2 years ago

joshleitzel commented 2 years ago

Thanks for this great library - really enjoying it so far!

I've come across an issue, and I'm not sure if it's a bug or misunderstanding on my part regarding how selections work with the Prisma plugin. I've followed the docs and have the following in my schema:

builder.prismaObject("Option", {
  findUnique: (option) => ({ id: option.id }),
  fields: (t) => ({
    id: t.exposeID("id"),
    text: t.exposeString("text"),
    selected: t.exposeBoolean("selected"),
  }),
});

builder.prismaObject("Question", {
  findUnique: (question) => ({ id: question.id }),
  fields: (t) => ({
    text: t.exposeString("text"),
    options: t.relation("options"),
  }),
});

builder.prismaObject("Survey", {
  findUnique: (survey) => ({ id: survey.id }),
  fields: (t) => ({
    id: t.exposeID("id"),
    order: t.exposeInt("order"),
    status: t.exposeString("status"),
    question: t.relation("question", { nullable: true }),
    preambles: t.relation("preambles"),
    expositions: t.relation("expositions"),
  }),
});

builder.prismaObject("Response", {
  findUnique: (response) => ({ id: response.id }),
  select: {
    id: true,
    option: {
      select: {
        selected: true,
        question: {
          select: {
            text: true,
          },
        },
      },
    },
  },
  fields: (t) => ({
    id: t.exposeID("id"),
    survey: t.relation("survey"),
    option: t.relation("option"),
    text: t.string({
      nullable: true,
      resolve: (response) => {
        // ==> Issue: response.option is present here when returned from query but not from mutation
        if (response.option.selected) {
          return null;
        } else {
          return response.option.question.text;
        }
      },
    }),
  }),
});

builder.queryType({
  fields: (t) => ({
    response: t.prismaField({
      args: {
        id: t.arg.id({ required: true }),
      },
      type: "Response",
      nullable: true,
      resolve: async (query) =>
        prisma.response.findFirst({
          ...query,
        }),
    }),
  }),
});

builder.mutationType({
  fields: (t) => ({
    createResponse: t.prismaField({
      type: "Response",
      args: {
        surveyId: t.arg.id({ required: true }),
        optionId: t.arg.id({ required: true }),
      },

      resolve: async (mutation, root, args, ctx, info) => {
        return prisma.response.create({
          data: {
            surveyId: parseInt(args.surveyId as string),
            optionId: parseInt(args.optionId as string),
          },
        });
      },
    }),
  }),
});

What I noticed is that when I directly query an existing Response record, via the response query, I can access the included relation response.option in the resolver just fine. However, when I submit the createResponse mutation which should return the same kind of Prisma object (Response), the resolver doesn't include response.option — it's undefined. Is this expected behavior? (I'm assuming it isn't, because TypeScript doesn't complain that response.option could be null in the resolver.)

joshleitzel commented 2 years ago

Ah, I think I found my mistake. I forgot to include ...mutation in resolve. Changing to the following works:

resolve: async (mutation, root, args, ctx, info) => {
  return prisma.response.create({
    ...mutation,
    data: {
      surveyId: parseInt(args.surveyId as string),
      optionId: parseInt(args.optionId as string),
    },
  });
},

I do find it strange though that TypeScript doesn't complain, since omitting ...query/...mutation means the selection could be undefined in the resolver. Should this be updated to raise an error when either (1) ...query/...mutation isn't included—or, alternatively, when a resolver doesn't do a null check?

hayes commented 2 years ago

This is one edge case I had a hard time finding a good balance of convenience, performance, and type-safety. If you define the selection for option.question.text at the field level rather than at the type level, Pothos can ensure that the data is loaded correctly. Because it's defined at the type level, pothos doesn't know whuch fields depend on those selections, and can't automatically try to load that data.

On the other side of things, because the mutation is referring to the response type by name, so there is no way in typescript to know what selections you have defined for the response type that need to be loaded. There are 2 potential solutions to this half of the problem. The one that I think makes the most sense would be to write a lint rule that checks for unused first argument of the prisma field resolvers.

The second option would be to have a runtime check after the resolver returns that checks what fields are included in the response. I've thought about trying to do this a prisma plugin, but there are too many issue with that approach. The issue with checking the returned value of the response is that it may be slightly more expense at run time, especially for values that return lists.

I think adding something behind an option that checks that the returned data matches the requested selection would help solve some of these problems, but would need options to disable the check, and more thinking around how to handle nested nullable fields in list responses.