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

[Documentation] Simple object question #948

Closed
MarceloPrado opened this issue Jun 30, 2023 · 8 comments
Closed

[Documentation] Simple object question #948

MarceloPrado opened this issue Jun 30, 2023 · 8 comments

Comments

@MarceloPrado
Copy link

MarceloPrado commented Jun 30, 2023

Hi @hayes!
I couldn't understand from the Simple Object docs how to achieve two things:

Make an object extend the .node interface

My first thought was to manually create a simpleInterface and use it in the simpleObject definition:

const NodeInterface = builder.simpleInterface("Node", {
  fields: (t) => ({
    id: t.id({
      nullable: false,
    }),
  }),
});

However, since I'm using the relay plugin, I get the PothosSchemaError: Duplicate typename: Another type with name Node already exists. error.

Make a simple object recursive

While it's explained in the docs, I couldn't find an example with Simple Object plugin. Because my interface is generated by .simpleObject, I can't find a way to add new fields to it.

This is what I've got so far. The output schema looks right, but the casting feels wrong:

const Condition = builder.simpleObject("Condition", {
  fields: (t) => ({
    field: t.field({
      description: "The field that the condition should evaluate.",
      nullable: false,
      type: ConditionField,
    }),
  }),
});

builder.objectField(Condition, "additionalConditions", (t) =>
  t.field({
    type: [Condition],
    resolve: (condition) =>
      (
        condition as unknown as typeof condition & {
          additionalConditions?: Array<typeof condition>;
        }
      ).additionalConditions ?? [],
  })
);

Unsolicited feedback

Finally, if you allow me one piece of feedback as a consumer of pothos. The more I know and use the library, the more I'm surprised by how large the API surface is. The many different ways of accomplishing one thing are quite overwhelming IMO - I'm frequently rethinking if I'm doing the right thing. I understand the library has to do A LOT of things, but at the same time, I wonder if there's a better way of encapsulating functionality. Today, plugins can register themselves in the global builder scope, which leads to a bloated API:
CleanShot 2023-06-29 at 22 36 05@2x

What I started doing quite often is inspecting the generated graphql schema to ensure the output looks correct. I'm curious if you have any thoughts on this. In your opinion, is this a bug or a feature?

@hayes
Copy link
Owner

hayes commented Jun 30, 2023

You can use builder.nodeInterfaceRef() to get a reference to the node type. This is generally not recommended though, since the node won't be resolvable if you don't prove a load method.

Simple objects shouldn't be recursive, and as the name implies they are really only meant for very simple use cases. There are a lot of limitations and downsides to using the simple objects API, while it is convenient, it's almost always better to just use one of the core methods for defining objects. You can make it work as you have above, but requires casting and won't be type-safe, and resolvers returning your type won't be expected to provide the additionalConditons.

The relay docs are some of the oldest docs in Pothos, and really need some cleanup.

If you want to create a recursive node, it would look something like:

type ConditionType = {
  field: ConditionField
  additionalConditions?: ConditionType[]
}
const Condition = builder.objectRef<{{
  field: 
}}>("Condition");

builder.node(Condition, {
  loadOne: id => loadConditonByID(...),
  id: { ... },
  fields: (t) => ({
    field: t.field({
      description: "The field that the condition should evaluate.",
      nullable: false,
      type: ConditionField,
    }),
    additionalConditins: t.field({
    type: [Condition],
    resolve: (condition) => condition.additionalConditions ?? [],
  })
  }),
});

@MarceloPrado
Copy link
Author

MarceloPrado commented Jul 2, 2023

That makes sense. Let me know if you want some help.
I've been thinking about contributing with some plugins. I'm having a hard time with Prisma due to performance issues, and thought on giving drizzle-orm a test. Getting the same Prisma type safety for other ORMs could be a great addition to the ecosystem.

While I haven't dived into the source yet, one thing I'm afraid is regarding how much each plugin knows about each other. Similar to my feedback above about polluting the global scope, I wonder if one plugin can access others. For example, the relay plugin makes assumptions about the prisma one

@hayes
Copy link
Owner

hayes commented Jul 2, 2023

Definitely open to PRs to improve documentation!

Regarding other ORMs, it's definitely something I'd like, but requires a lot of work to get it right. The Prisma plugin is by far the most complex plugin so far. Getting type-level integration to allow defining object types is pretty easy, the hard part is building the right abstractions to make it easy to make your queries efficient.

I worked on prototyping a drizzle plugin, and have reached out to the drizzle team a couple times about what I'd need to make it work, but haven't really gotten much traction there. I've got the types mostly working for drizzle, but there is no way to actually make the queries work correctly with the existing APIs.

Regarding plugins knowledge of each other, there are some places it's required, but it's generally pretty limited. The relay plugin doesn't know anything about the prisma plugin, but the prisma plugin does need to know a decent amount abut the relay plugin because it needs to be able to create nodes and connections. The knowledge for the most part is about the public APIs the other plugins add (builder.node, t.connection, etc) so the plugin can call those methods, but there are other ways plugins interoperate: shared global types, so that one plugin can extend options for another, or a plugin can use a type from another plugin without fully re-implementing it. Common "extensions" attached to types or fields for specific runtime behaviors, and in a few rare instances, certain behavior is re-implemented in multiple plugins.

None of this is ideal, but is generally not required for most plugins, it's really when 2 plugins need fairy tight integration (3 or 4 plugins do this, almost all other plugins have 0 knowledge of any other plugins), and building a consistent API without knowledge of other plugins isn't really possible.

You mentioned there being a lot of different ways to do something, this is definitely true, but I don't see it as a bad thing (outside of documentation). The reason Pothos works so well is because it doesn't make a lot of assumptions about how you want to build your API, and will work with just about anything.

A lot of things are split off into plugins (simple objects, dataloaders, prisma integration) so while there are a lot of ways to do something, the core API isn't that large (there are 3 ways to define objects in core, (classes, refs, types defined in the builder) but that is mostly for legacy reasons) almost everything in core is pretty close to a 1:1 mapping of GraphQL concepts. Plugins introduce a lot of new options, but they are opt-in, and are all intended for fairly specific use cases. I've never had issues with all the plugins extending the core builder, and in my experience quickly being able to see all the methods in one place rather than having to think about where to pull things in from is a lot easier. Having everything added as extensions to the core classes also makes plugin interoperability a lot easier. Eg, the prisma plugin can use builder.node to define a node without knowing anything about how nodes are build, the prisma field builder doesn't know anything about t.authField from the scope-auth plugin, but this method will just work becuase it's added a a very generic way to the core graphql field builder, etc...

I am open to suggestions on how this could be improved, but I've generally found that the Pothos plugins work really well together, and haven't heard about any big issues caused by most things extending the core classes.

@hayes
Copy link
Owner

hayes commented Jul 2, 2023

If you are curious about drizzle, you can see the API I built for it here: https://github.com/hayes/pothos/blob/drizzle/packages/plugin-drizzle/tests/example/schema/index.ts

This is 100% type-safe, but there is one small missing piece that makes this impossible to implement the runtime properly, I think drizzle could make this work pretty easily if they exposed a way to alias relations (allow querying the same relation multiple times with different filters) in their relation API

@MarceloPrado
Copy link
Author

MarceloPrado commented Jul 4, 2023

I really appreciate your in-depth answer Michael! Lots of great discussion points in there.

Regarding plugin interoperability/accessing each other not being a problem, I understand your point about not wanting to re-implement things like Node and auth that every plugin might need. While interoperability adds its own challenges (plugin order matters, for instance), seems like a trade-off worth making vs. the added complexity of making every plugin "pure".

I'll take a look at the pointed reference. One of the things that made me think about drizzle is their recently released relational queries, an API closely inspired by Prisma. This ignited the idea of "what if we translated potho's prisma plugin to drizzle?". It seems you also had the same thought though 😂

quickly being able to see all the methods in one place rather than having to think about where to pull things in from is a lot easier

I believe this to be the main point. As someone who's fairly new to Pothos API, when I'm building a new GraphQL type, seeing 3 different ways to define objects causes overhead. I have to remind myself what are the differences between each, and fairly often re-read their docs.

However, I can also understand how it could be annoying to "search" through namespaced types to find the type you already knew in the first place. If the surface area was smaller, it could be more beginner-friendly, but annoying to experienced engineers.

Seems like we're discussing a "docs issue". While what Pothos has is pretty extensive and great, having something that more clearly exposes these differences and guides users could be decent improvement. Ideas like #930 could also enhance the e2e DX.

I hate being the "complain but don't do anything" type of person though. I'm happy to help if you think that's a direction worth pursing 🙂

@hayes
Copy link
Owner

hayes commented Jul 6, 2023

Docs improvements are more than welcome (and definitely needed). If you have specific ideas about how to improve the APIs or modularity around plugins I am also happy to hear them. The existing API is meant to provide the best developer experience possible while considering what is possible in typescript. There are a lot of tradeoffs that need to be made to ensure that plugins can work together, ensure that types are always accurate/safe, and to try to keep the API consistent across plugins whenever possible.

Balancing these tradoffs does sometimes mean that specific use cases are not entirely optimal, because the benefits don't outweigh the downsides for other aspects, etc.

Type safety is one of the most important pieces of the API, and this is part of why there are a number of ways to do a lot of things. Type information can come from a lot of sources, and creating a single API that does everything will generally not provide the best experience from a types perspective.

@MarceloPrado
Copy link
Author

MarceloPrado commented Jul 15, 2023

One thing I did last week that I wish I had done sooner: add the generated graphql schema to source control.

Previously, I had it on my .gitignore. I was generating it, but not leveraging git diff to inspect what actually changed. Once I moved the schema.graphql file to source control, it was really easy to see how each pothos method impacted the final schema.

I could write something around this idea, and also make improvements to the relay docs, since you've mentioned it's one of the oldest ones. Does that make sense to you?

@hayes
Copy link
Owner

hayes commented Jul 15, 2023

100% agree, I always have a generated schema checked into source for my projects. It makes things a lot easier. Docs around this would be a good idea. I've avoided it because there are so many different (opinionated) ways to do it, but I think having something easy to follow would probably help.

Could also be added to all the examples (this helps both show how to set things up, but also makes comparing code to the generated schema easier for the examples without actually running them.

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