Make brandWithType return the branded object

[XiNiHa]

This lets users to do something like return brandWithType(await prisma.user.findUnique({ ... }), User)

[changeset-bot]

🦋 Changeset detected

Latest commit: 68c0858

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages

Name	Type
@pothos/core	Minor
@pothos-examples/envelope-helix-fastify	Patch
@pothos-examples/graphql-shield	Patch
@pothos-examples/helix	Patch
@pothos-examples/nextjs	Patch
@pothos-examples/open-telemetry	Patch
@pothos-examples/prisma-federation	Patch
@pothos-examples/prisma	Patch
@pothos-examples/relay-windowed-pagination	Patch
@pothos-examples/simple-classes	Patch
@pothos-examples/simple-interfaces	Patch
@pothos/website	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated
pothos	✅ Ready (Inspect)	Visit Preview	Nov 14, 2022 at 5:53AM (UTC)

[hayes]

Overall I think returning the passed in value could be a decent change, but I probably wouldn't include the brand field as part of the type, because it shouldn't be accessed outside of Pothos.

[hayes]

I'm not sure adding the brand field to the type here makes sense, it's intended as an invisible side effect, I would probably just return T directly

[XiNiHa]

But in that case the type error still persists (since the field resolver expects the returned value to have the key attached)

[hayes]

Do you have an example of how you are trying to use this?

[XiNiHa]

Something like this:

author: t.field({
  type: UserOrTeam,
  resolve: async (root) => {
    if (root.authorType === 'USER')
      return brandWithType(
        await prisma.user.findUnique({ where: { id: root.authorId } }),
        User
      )
    else
      return brandWithType(
        await prisma.team.findUnique({ where: { id: root.authorId } }),
        Team
      )
  }
})

[hayes]

The pothos type system doesn't have any concept of typeBrandKey. Adding it to the return type here, should not be affecting how this field is type-checked. I am fairly sure that if you update brandWithType to just return T (which allows you to get rid of the Types parameter as well) should still allow this to work correctly without type errors.

[hayes]

there is an existing WithBrand type in the prisma plugin you could copy or move instead. I think the type used above is still a bit over-complicated

[XiNiHa]

Ooooh actually .addBrand was the one I was looking for. Maybe I should close the PR then 🤔

[saevarb]

Hello. After updating Pothos I started running into this error, and I found the solution here. Previously, I had no errors. This is happening when dealing with a union type.

I was under the impression that this sort of thing was handled by isTypeOf and friends, and obviously until this update, things worked. What is the reason that this is needed all of a sudden, and why is there not a single mention of this in the documentation here: https://pothos-graphql.dev/docs/plugins/prisma ? The word brand doesn't occur once.

[hayes]

yes, this definitely needs some docs. Sorry about that.

So with isTypeOf checks, this technically isn't needed. The issue is that writing good isTypeOf checks for Prisma objects is almost impossible without adding a column to your DB with the type name.

The way the brands work currently is roughly like this:

When you create a union, if the union doesn't have a resolveType resolver, it will check to see if any of the types declared a custom "abstract shape", which is basically a different type to use when the type is returned as part of a union. Pothos default resolveType checks for a brands on objects that are part of a union as a way to identify them automatically.

This was always how the node/nodes interfaces worked for prisma objects, but the more recent change was intended to make it easier to build your own interfaces/unions that used prisma objects. Unfortunately I forgot to document them.

If you have working isTypeOf checks, I can see why this change is pretty inconvenient. The issue is that pothos can optimize selections to only load the fields that were queried, and consistently identifying objects in an isTypeOf check can be tricky because sometimes the only thing selected is the id.

If you have a resolveType check the expected type should not require branding, but for everything else, it should be easy to add, you basically just need to wrap your prisma calls like: YourType.addBrand(prisma.yourTyoe.findMany(...)). Which should also allow you to remove the isTypeOf checks. If you have a good way of writing isTypeOf checks let me know, this is something I have never found a good way to do

[saevarb]

Thanks a lot for your detailed reply!

Yes, that's actually a brilliant idea. I've run into the exact same problems you describe with isTypeOf checks, and in a couple of places I'm doing some funky stuff to get it working.

Thanks for all your hard work as well -- pothos is absolutely great, and I really appreciate the thought and engineering that goes into it.