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.34k stars 162 forks source link

Effect-TS integration #974

Closed vecerek closed 1 year ago

vecerek commented 1 year ago

Hi there πŸ‘‹ I'd like to ask you for an opinion, rather than requesting a feature or reporting a bug. There's this new and sprouting ecosystem in TS called Effect. In their own words:

Effect is a powerful TypeScript library designed to help developers easily create complex, synchronous, and asynchronous programs.

One of the key features that sets Effect apart is how it leverages structured concurrency(opens in a new tab) to provide features such as async cancellation and safe resource management, making it easier to build robust, scalable, and efficient programs.

(source)

I really like Effect and I really like Pothos. There's even an effect plugin made for pothos. What it does is that it exposes an effect configuration option (and a few others) that lets us return an Effect from the resolver rather than a value/Promise. However, I'm also using other plugins like the relay plugin. I use it to define relay-spec-compliant mutations. However, relayMutationField does not have an effect config option. I presume it's because while the plugins are a great solution to interoperate with Pothos itself, they, by nature, don't necessarily interoperate with each other.

So my question to the Pothos author(s) is: how should we go about this strategically? Should the effect plugin for pothos re-implement the relay-mutation abstraction? Or import it and try to extend it and as the relay one evolves, keep maintaining it? Or, should the plugin be built inside the pothos repository itself to ensure interoperability with the rest of the plugins in the future?

I also opened an issue about this in their repo: https://github.com/iamchanii/pothos-plugin-effect/issues/3

hayes commented 1 year ago

What pothos can support automatically for plugin interoperability is very dependent on the use case. In this case, there isn't anything the relay plugin can do to automatically pull in the effect options for a relayMutationField because the effect options defined need their own generic slots in the function signature to be inferred correctly.

I personally never use relayMutationField because this pattern hasn't been part of the relay spec in a long time. For cases where I want a wrapping result type they tend to be very easy to create manually.

vecerek commented 1 year ago

@hayes thanks, that makes total sense! I guess I have some lightweight refactoring work to do now πŸ˜„ If I may ask, how do you decide what plugins should be developed/maintained in this repo? Is it purely demand-based or are there other criteria?

hayes commented 1 year ago

No strict process or rules for that. Mostly just whatever seemed like it would be applicable to a broad audience and encourages good patterns/type safety.

I've generally tried to avoid tying anything too closely to a specific library with a few exceptions. The Prisma plugin was something I needed, and Prisma at the time provided an unrivaled developer experience, and for very well with Pothos. The other one was the validation plugin that uses zod. This plugin was built quickly and was probably a mistake in the long run, and is something I'm looking at refactoring in the future.

Going forward I like the pattern used for the tracing plugin which proves a generic interface with small integrations for specific libraries.

I am very excited about the effect plugin as it's one of the first high quality public plugins that exists outside of Pothos

vecerek commented 1 year ago

I'm also very excited about the plugin πŸ™‚ But now I realize that, more importantly, I also use the connection field from the relay plugin. If I want to use effects, I would have to refactor those into regular query fields as well. If I wanted to keep using the connection interface, would it mean I'd have to bring it over and re-implement it in the effect plugin?

hayes commented 1 year ago

Almost everything that t.connection does can be easily replicated in smaller pieces (either in the effects plugin or in your own code). There are builder methods for creating connection and edge objects, and an arg builder method for creating fields you can see an example here: https://github.com/hayes/pothos/blob/main/packages/plugin-relay/tests/examples/relay/schema/numbers.ts#L220 a long with the query field right below it

That pattern should work with the edge plugin without any issues

hayes commented 1 year ago

Actually, I'm not entirely sure how the connection plugin works in detail, i think these pieces should work, but I'm not 100% sure how the results of effects get resolved, but hopefully this helps

vecerek commented 1 year ago

This is super helpful, indeed! I've had no idea about the connectionObject method. Thanks a million! πŸ™‚ I'll close this issue once I'm done rewriting everything and made the effect plugin work in my project just to avoid re-opening the issue with additional questions.

vecerek commented 1 year ago

What is the correct way of adding extra fields to the connection type with the connectionObject method? I checked and it seems the connectionObject interface and its implementation were intended to handle extra fields. However, the following does not compile:

builder.connectionObject(
  {
    name: "ThingConnection",
    type: "Thing",
    fields: (t) => ({ total: t.exposeInt("total") }),
    //                                    ^^^^^
    // Argument of type 'string' is not assignable to parameter of type 'never'.ts(2345)
  },
  {
    name: "ThingEdge",
  }
)
hayes commented 1 year ago

I think you could probably add a type for t:

fields: (t: ObjectFieldBuilder<
  // Extract SchemaTypes, I usually have this exported alongside the builder in my apps
  typeof builder extends SchemaBuilder<infer T> ? T : never, 
  ThingType
>) => ({...})

On my phone, so not sure if thats quite right. That's one of the limitations of the connectionObject method. t.connectioj infers the return type of the resolver to alw you to add things returned by the resolver. There isn't a good way to tell Pothos what you return from the resolver for this method . Typescript doesn't have a way to partially declare genetics, so you would need to provide all 6 if you wanted to pass it that way.

The above method lets the shape be inferred from the field builder. It's a little backwards but it's type-safe and should work consistently. The only weird thing is extracting the SchemaTypes from builder

vecerek commented 1 year ago

Thanks for the suggestion, @hayes, and I appreciate you taking the time and writing it down while on your phone. I'm struggling a bit with satisfying the compiler. The way I interpreted the following from your suggestion:

// Extract SchemaTypes, I usually have this exported alongside the builder in my apps
typeof builder extends SchemaBuilder<infer T> ? T : never, 

is that it's the type I pass to the SchemaBuilder when I construct the builder const builder = new SchemaBuilder<Types>({...}). I also have that type exported in my apps. However, if I follow your suggestion, I'm getting the following error:

fields: (
//^^^^
// a very long compilation error
  t: ObjectFieldBuilder<
    Types,
//  ^^^^^
//  Type 'Types' does not satisfy the constraint 'SchemaTypes'.
//    Type 'Types' is missing the following properties from type 'SchemaTypes': outputShapes, inputShapes, 
//    DefaultFieldNullability, Root, and 4 more.ts(2344)
    Types["Objects"]["ThingType"]
  >
) => ({ ... }),

My response to this was to intersect Types with SchemaTypes imported from @pothos/core:

fields: (
//^^^^
// a very long compilation error
  t: ObjectFieldBuilder<
    Types & SchemaTypes,
    Types["Objects"]["ThingType"]
  >
) => ({ ... }),

That satisfied the previous error on the first type argument on the ObjectFieldBuilder type. However, I'm still getting the super long error on fields.

Toggle the super long error message ``` Type '(t: ObjectFieldBuilder) => { total: FieldRef; }' is not assignable to type 'ObjectFieldsShape, ConnectionResultShape, ThingType, FieldNullability<[unknown]>, boolean> & ThingType >'. Types of parameters 't' and 't' are incompatible. Type 'ObjectFieldBuilder, ConnectionResultShape, ThingType, FieldNullability<[unknown]>, boolean> & ThingType >' is not assignable to type 'ObjectFieldBuilder'. Types of property 'exposeBoolean' are incompatible. Type ' = false>(name: Name, ...args: NormalizeArgs<[options: Omit, ConnectionResultShape, ThingType, FieldNullability<...>, boolean> & ThingType, "Bo...' is not assignable to type ', ResolveReturnShape, Nullable extends FieldNullability<"Boolean"> = boolean>(name: Name, ...args: NormalizeArgs<...>) => FieldRef<...>'. Types of parameters 'options' and 'options' are incompatible. Type '(Omit, "resolve" | "type" | "nullable"> & { ...; }) | undefined' is not assignable to type '(Omit, ConnectionResultShape, ThingType, FieldNullability<[...]>, boolean> & ThingType, "Boolean", any, {}, any>, "resolve" | ... 1 more ... | "nullable"> & ({ ...; } | { ...; })) | undefined'. Type 'Omit, "resolve" | "type" | "nullable"> & { ...; }' is not assignable to type '(Omit, ConnectionResultShape, ThingType, FieldNullability<[...]>, boolean> & ThingType, "Boolean", any, {}, any>, "resolve" | ... 1 more ... | "nullable"> & ({ ...; } | { ...; })) | undefined'. Type 'Omit, "resolve" | "type" | "nullable"> & { ...; }' is not assignable to type 'Omit, ConnectionResultShape, ThingType, FieldNullability<[unknown]>, boolean> & ThingType, "Boolean", any, {}, any>, "resolve" | ... 1 more ... | "nullable"> & { ...; }'. Type 'Omit, "resolve" | "type" | "nullable"> & { ...; }' is not assignable to type 'Omit, ConnectionResultShape, ThingType, FieldNullability<[unknown]>, boolean> & ThingType, "Boolean", any, {}, any>, "resolve" | ... 1 more ... | "nullable">'. Types of property 'errors' are incompatible. Type '{ types?: (new (...args: any[]) => Error)[] | undefined; directResult?: boolean | undefined; union?: { directives?: Directives | undefined; description?: string | undefined; extensions?: Readonly<...> | undefined; inaccessible?: boolean | undefined; name?: string | undefined; } | undefi...' is not assignable to type '{ types?: (new (...args: any[]) => Error)[] | undefined; directResult?: boolean | undefined; union?: { directives?: Directives, "UNION"> | undefined; description?: string | undefined; extensions?: Readonly<...> | undefined; inaccessible?: boolean | undefined; name?: string | undefined; } | ...'. Type '{ types?: (new (...args: any[]) => Error)[] | undefined; directResult?: boolean | undefined; union?: { directives?: Directives | undefined; description?: string | undefined; extensions?: Readonly<...> | undefined; inaccessible?: boolean | undefined; name?: string | undefined; } | undefi...' is not assignable to type '{ types?: (new (...args: any[]) => Error)[] | undefined; directResult?: boolean | undefined; union?: { directives?: Directives, "UNION"> | undefined; description?: string | undefined; extensions?: Readonly<...> | undefined; inaccessible?: boolean | undefined; name?: string | undefined; } | ...'. Types of property 'result' are incompatible. Type '{ directives?: Directives | undefined; description?: string | undefined; extensions?: Readonly> | undefined; ... 4 more ...; name?: string | undefined; } | undefined' is not assignable to type '{ directives?: Directives, "OBJECT"> | undefined; description?: string | undefined; extensions?: Readonly> | undefined; ... 4 more ...; name?: string | undefined; } | undefined'. Type '{ directives?: Directives | undefined; description?: string | undefined; extensions?: Readonly> | undefined; ... 4 more ...; name?: string | undefined; }' is not assignable to type '{ directives?: Directives, "OBJECT"> | undefined; description?: string | undefined; extensions?: Readonly> | undefined; ... 4 more ...; name?: string | undefined; }'. Types of property 'fields' are incompatible. Type 'ObjectFieldsShape | undefined' is not assignable to type 'ObjectFieldsShape, boolean> | undefined'. Type 'ObjectFieldsShape' is not assignable to type 'ObjectFieldsShape, boolean>'. Types of parameters 't' and 't' are incompatible. Type 'ObjectFieldBuilder, boolean>' is not assignable to type 'ObjectFieldBuilder'. Types of property 'exposeBoolean' are incompatible. Type ', boolean, "Boolean", true>, ResolveReturnShape, Nullable extends FieldNullability<"Boolean"> = false>(name: Name, ...args: NormalizeArgs<...>) => FieldRef<...>' is not assignable to type ' = boolean>(name: Name, ...args: NormalizeArgs<[options: Omit, "resolve" | ... 1 more ... | "nullable"> & ExposeNullability<...>], 0>) => ...'. Types of parameters 'options' and 'options' are incompatible. Type '(Omit, "resolve" | "type" | "nullable"> & { nullable?: any; }) | undefined' is not assignable to type '(Omit, boolean, "Boolean", any, {}, any>, "resolve" | "type" | "nullable"> & ({ ...; } | { ...; })) | undefined'. Type 'Omit, "resolve" | "type" | "nullable"> & { nullable?: any; }' is not assignable to type '(Omit, boolean, "Boolean", any, {}, any>, "resolve" | "type" | "nullable"> & ({ ...; } | { ...; })) | undefined'. Type 'Omit, "resolve" | "type" | "nullable"> & { nullable?: any; }' is not assignable to type 'Omit, boolean, "Boolean", any, {}, any>, "resolve" | "type" | "nullable"> & { ...; }'. Type 'Omit, "resolve" | "type" | "nullable"> & { nullable?: any; }' is not assignable to type '{ nullable: any; }'. Property 'nullable' is optional in type 'Omit, "resolve" | "type" | "nullable"> & { nullable?: any; }' but required in type '{ nullable: any; }'.ts(2322) ```

As a mere user of pothos, I don't know enough about the ObjectFieldBuilder type to make sense of this error message.

hayes commented 1 year ago

It's not quite what you pass into the builder, there is a helper that extends those types with defaults which is why I was extracting the types from the builder in the example. If you hover over builder you can probably see your types wrapped with a utility type that adds defaults that could also be used to replicate the behavior.

I can provide a better example tomorrow when I'm at a computer. This is getting into hacks that a normal user shouldn't ever need. Generally I would suggest if you need to extend the connection object, use t.connection, which doesn't work well for your case (you can also globally extend all connections, which is covered in the relay docs).

The real solution would be for the effects plugin to support relay connections directly, or to provide a helper that can work with other field builder methods (I have some ideas, but no examples of how this might work I can try to prototype some time soon)

vecerek commented 1 year ago

I gave it another go and tried to follow your suggestion more closely. The reason I digressed before was that it didn't type check and I didn't quite understand how to make it work. But now I think I've got it. I need to use typeof SchemaBuilder instead of just SchemaBuilder. Also, I had to add extends Partial<PothosSchemaTypes.UserSchemaTypes> to infer T because TypeScript suggested it as a fix. However, I'm still getting the same long compilation error from above.

fields: (
//^^^^
// but still getting the same long error message from earlier
  t: ObjectFieldBuilder<
    typeof builder extends typeof SchemaBuilder<
      infer T extends Partial<PothosSchemaTypes.UserSchemaTypes>
    >
      ? T
      : never,
    ThingType
  >
) => ({ ... })

I can provide a better example tomorrow when I'm at a computer.

Sweet! Very much appreciated πŸ™‚

This is getting into hacks that a normal user shouldn't ever need. [...] The real solution would be for the effects plugin to support relay connections directly, or to provide a helper that can work with other field builder methods (I have some ideas, but no examples of how this might work I can try to prototype some time soon)

I don't mind temporary hacky workarounds in my code until a proper solution is available. I might look into adding support for relay connections in the effect plugin later, especially once such helpers exist but maybe even sooner.

vecerek commented 1 year ago

you can also globally extend all connections, which is covered in the relay docs

I happened to have a total field on all my connection types. Extending the type globally worked like a charm!

hayes commented 1 year ago

opened https://github.com/iamchanii/pothos-plugin-effect/pull/11 to kickstart the effort to add connection support to the effect plugin

hayes commented 1 year ago

that PR was merged and released, so I'm going to close this out, but let me know if you have more questions

Enalmada commented 9 months ago

@hayes could pothos-plugin-effect be added to the pothos plugins list? Perhaps a section below called "Community Plugins" or something. Without that exposure, the average new user is probably not going to know it is an option.

hayes commented 9 months ago

Yes, definitely! Feel free to open a PR to either add it to https://pothos-graphql.dev/docs/resources, or create a new community plugins page in the Plugins section!