-
-
Notifications
You must be signed in to change notification settings - Fork 161
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
wip - Pothos EdgeDB Plugin #539
base: main
Are you sure you want to change the base?
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
: never | ||
: never; | ||
|
||
export type EdgeDBModelShape< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
EdgeDB's Query Builder contains a default which includes all db schema's with their properties
and relations
, or named as links
in edgeDB, of the schemas and can be used to recreate the PrismaTypes
structure for type safety.
Example of EdgeDBTypes
export interface types {
"std": {
...
};
"schema": {
...
};
"cfg": {
...
};
"default": {
"Comment": Comment;
"Follow": Follow;
"Media": Media;
"Post": Post;
"PostMedia": PostMedia;
"Profile": Profile;
"User": User;
};
"sys": {
....
};
}
export interface Post extends std.$Object {
"big_int_id": bigint;
"content"?: string | null;
"created_at": Date;
"published": boolean;
"title": string;
"updated_at": Date;
"author"?: User | null;
}
This looks like an awesome start! Getting the type system working is often one of the hardest parts, and it looks like you are making great progress! I'll try to find some time today to dive a little deeper. Initial skim looks like there are lots of pieces that are unused or copied from the prisma plugin that don't quite make sense yet, so I'll just ignore those for now and try to look specifically at types for defining objects and links |
packages/plugin-edgedb/CHANGELOG.md
Outdated
@@ -0,0 +1,340 @@ | |||
# Change Log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably don't need this
"author": "Michael Hayes", | ||
"license": "ISC", | ||
"keywords": [ | ||
"giraphql", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
probably don't need giraphql in keywords anymore, I think pothos had gotten enough traction that there shouldn't be many people still looking for giraphql
| ({ [ModelKey in keyof Model]: Model[ModelKey] extends infer U ? U : never } & { | ||
Fields: string | never; | ||
Links: { | ||
[Key in Model['Fields']]: { Shape: Model['Links'][Key] }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
curious about this shape referencing itself here, what is that used for?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had parent args, Fields and Links in one type which end up really nested. Will need to rethink that and construct it in a clean way, will probably refer more or less to PrismaTypes
's structure. This Shape referencing isnt actually useful there indeed.
pothos/packages/plugin-prisma/src/types.ts
Lines 37 to 53 in 1f9bec0
export interface PrismaModelTypes { | |
Name: string; | |
Shape: {}; | |
Include: unknown; | |
Select: unknown; | |
Where: {}; | |
Fields: string; | |
ListRelations: string; | |
RelationName: string; | |
Relations: Record< | |
string, | |
{ | |
Shape: unknown; | |
Types: PrismaModelTypes; | |
} | |
>; | |
} |
Curios is there a difference between Fields
and RelationName
?
Looks like they contain the same values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope... That seems to be an oversight. I'll remove Fields
from the prisma plugin
// description: getFieldDescription(this.model, this.builder, name, description), | ||
extensions: { | ||
...extensions, | ||
pothosEdgeDBFallback: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the fallback resolver stuff in pothos is one of my least favorite parts. having a resolver option that only sometimes gets called is really confusing for users, so if this can be avoided for edgeDB that would be cool. I haven't seen if you have started working on any entity-reloading logic yet.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't really looked into it, just copy left from the prisma plugin. Will take a look into it, we could avoid it. No entity-reloading logic existing yet.
) => EdgeDBObjectRef<Types, Model>; | ||
} | ||
|
||
export interface EdgeDBObjectFieldBuilder< |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I noticed that there is something broken about this in the Prisma plugin. I tried referencing this global type to create a function that accepts a PrismaFieldBuilder, but it wasn't actually compatible with the PrismaFieldBuilder type. Not sure if this will have the same issue, but something that might be worth checking on. Not sure if you need this at all if you are not supporting extending EdgeDBObjectFieldBuilder from other plugins initially.
packages/plugin-edgedb/src/index.ts
Outdated
}; | ||
}) | ||
| never, | ||
Shape extends object = Model, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shape is generally used as the shape of the parent
arg
packages/plugin-edgedb/src/types.ts
Outdated
range = 'range', | ||
} | ||
export type tupleOf<T> = [T, ...T[]] | []; | ||
export type cardinalityAssignable<C extends Cardinality> = C extends Cardinality.Empty |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this could probably be modeled in a way thats more readable:
type CardinalityAssignable<C extends Cardinality> = {
Empty: Cardinality.Empty
...
AtLeastOne: Cardinality.One | Cardinality.AtLeastOne | Cardinality.Many
}[C]
Curious about your goals here, and what you are looking for. Is this something you want to get merged in the not too distant future, or more just sharing/collecting feedback while you iterate? Long term, is this something you want have live in the main repo and have maintained with the rest of the plugins, or something you want to own in your own repo. I have been meaning to create some docs pages for linking out to community maintained projects. It might also be interesting do have a model similar to https://github.com/opentracing-contrib where there is an official place for people to add their own plugins/packages/examples etc in a way that is discoverable. Open to any ideas and suggestions here, and don't feel too strongly about it. I would say that the bar for what I would expect in this repo would likely be a lot higher, and include things like making sure that this works well with all the other plugins, thorough documentation, test coverage, and solutions for things like aliasing fields, or defining multiple fields that use the same Regardless of how you want to work on this, and where you would like it to live, I am happy to help out when I have time, and give feedback/reviews or answer any questions you have about how the internals of pothos work. Looks like you are already making a lot of progress and have figured out a lot of the weird patterns needed to make something like this work. |
Agreed, this is right now just not enough for a plugin imo. Long term I would love to see it in the main repo as a plugin. But also looking for more features, supporting other plugins and enabling integrations, like relay nodes and so on. A lot of work needs to be done, not finished at all. I am not very familiar with the architecture and type system of Pothos, still learning the patterns and experimenting with the types :) I was porting a lot of the prisma plugins system into this plugin therefore still unused and unnecessary code / type definitions around here. Will clean that up. |
Will need some feedback on this topic. Edgedb queries return type depends on the select shape. Even Example// EdgeDB Generated QueryBuilder
import e from '../../client';
builder.queryType({
fields: (t) => ({
users: t.edgeDBField({
type: ['User'],
nullable: true,
resolve: async (_query, _parent, _args, ctx) => {
const users = await e
.select(e.User, (user) => ({
id: true,
email: true,
name: true,
}))
.run(db);
return users;
// ^? { id: string; email:string; name: string | null; }[]
// This would not match with Users Model["Shape"], comments | posts ... are missing
},
}),
}),
});
interface EdgeDBModelTypes {
User: {
Shape: {
"comments": Comment[];
"posts": Post[];
"email": string;
"name"?: string | null;
"followers": Follow[];
"following": Follow[];
"media": Media[];
"profile"?: Profile | null;
}
}
} |
Added pothos/packages/plugin-edgedb/src/types.ts Lines 80 to 86 in f853b76
pothos/packages/plugin-edgedb/src/global-types.ts Lines 102 to 104 in f853b76
|
Actually supposed to be a deep partial type, updated version: |
I think the technically correct approach here would be to model this after the "select" mode from the prisma plugin. Basically the prisma plugin supports having a default set of selections defined on the "prismaObject", when you do this, the "parent" type for all resolvers is limited to that selection. You can still "expose" fields that are not part of the selection (which will automatically select them when the expsosed field is queried) . You can then also define selections on individual fields, which will extend the parent type in the resolver for that field. You can see an example of this here: https://github.com/hayes/pothos/blob/main/packages/plugin-prisma/tests/example/schema/index.ts#L38-L59 The implementation for this gets a lot more complicated, so not sure if you need to explore this in the initial version. Took me a few iterations of the prisma plugin before I figured out how to make that work properly. |
WIP EdgeDB Plugin
Related Issue: #534
edgeDBObject
Naming Conventions
t.link()
: Model property which is a relation to another model. (like prisma pluginst.relation()
)Could be one or many relation to model.
Open for any feedback and reviews!