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

Relation support #316

Open
gfpacheco opened this issue Mar 22, 2020 · 21 comments
Open

Relation support #316

gfpacheco opened this issue Mar 22, 2020 · 21 comments

Comments

@gfpacheco
Copy link

@gfpacheco gfpacheco commented Mar 22, 2020

Hi, I just tried Redwood for a new personal project.

At first I thought this could be the one boilerplate to rule them all, lots of great stuff, very organized.
I went through the entire tutorial, it's great btw.

But the illusion ended when I tried to create my first relation.

I wrote my prisma schema like this:

model Teacher {
  id Int @id @default(autoincrement())
  name String
  courses Course[]
}

model Course {
  id Int @id @default(autoincrement())
  name String
  teacher Teacher
}

And after running yarn rw db save I saw that Prisma generates the db field without the Id suffix, but that's just Prisma's fault, I guess I can live with that.

Then I ran yarn rw g scaffold for both teacher and course models and realized the generated GraphQL schema was like this:

  type Teacher {
    id: Int!
    name: String!
    courses: Course
  }

  type Course {
    id: Int!
    name: String!
    teacher: Teacher
  }

Not only teacher's courses field wasn't an array, both teacher's courses and course's teacher fields didn't have ! as expected since they didn't have ? on Prisma schema. So I went and fixed them (together with their inputs).

Then I went to the GraphQL Playground and created a teacher, so far so good, until I tried to create a course and got a really weird error, very hard to read (because it was coming from prisma), but I managed to identify: the createCourse function created by the scaffold was like this:

export const createCourse = ({ input }) => {
  return db.course.create({
    data: input,
  })
}

But needed to be like this:

export const createCourse = ({ input: { teacher, ...input } }) => {
  return db.course.create({
    data: { ...input, teacher: { connect: { id: teacher } } },
  })
}

My next disappointment was with the course query, of course querying the teacher field from course wouldn't work since the resolver only gets the course from the database and there's no resolver for the teacher field (the same happens with the courses field on Teacher).

PS: I still don't know how to create a field resolver, but I'll ask somewhere else.

Not to mention the web part, where teacher queries all have the courses fields but without any subfields (the same for Course's teacher field) and the forms have a Int field for each relation field.

I'm sorry it might seem I'm just saying bad things about a open source project, I really appreciate your efforts, I'm gonna keep using Redwood and try helping as much as I can. I just hoped there was anything in the docs saying relations are a work in progress or something.

PS: I have a few years of Node and GraphQL experience (not so much with tooling though), would be happy to tell how I (as a developer) would expect things to work, and maybe help with the implementation.

@cannikin

This comment has been minimized.

Copy link
Contributor

@cannikin cannikin commented Mar 22, 2020

We got Redwood to the point of being able to run through the tutorial as written, anything else (like automatically creating these relations) is being worked on by contributors like you! 😀 Want to help us out?

Although scaffolds will probably never be built out to the point of adding forms for relationships, the SDL and service generators should be able to introspect and build them when possible.

@gfpacheco

This comment has been minimized.

Copy link
Author

@gfpacheco gfpacheco commented Mar 23, 2020

I imagined that, but yes, I wanna help. I don't have much free time, but I'll try!

@weaversam8

This comment has been minimized.

Copy link

@weaversam8 weaversam8 commented Mar 23, 2020

@gfpacheco @cannikin I've been stumbling through my own schema and dealing with issues with relations, and was planning on narrowing the problems down into a few issues. (Planning on posting some tonight.) Maybe this issue can serve as an "EPIC" for relation support as a whole.

@RobertBroersma

This comment has been minimized.

Copy link
Contributor

@RobertBroersma RobertBroersma commented Mar 24, 2020

I've found that exporting Model from a service file allows you to define field resolvers, since redwood will automatically pick them up just like Query and Mutation resolvers.

I was just playing a bit, defining Model resolvers in their matching service, resolving all relation fields. E.g:

// api/src/services/user.js
export const User = {
  posts: (_obj, { root }) =>
    db.user.findOne({ where: { id: root.id } }).posts(),
}

// api/src/services/post.js
export const Post = {
  owner: (_obj, { root }) =>
    db.post.findOne({ where: { id: root.id } }).owner(),
}

The pattern is simple! Something like this:

// api/src/services/<modelName>.js
export const <ModelName> = {
  <relationField>: db.<modelName>.findOne({ where: { id: root.id } }).<relationField>(),
}

What do you think?

@cannikin

This comment has been minimized.

Copy link
Contributor

@cannikin cannikin commented Mar 24, 2020

@RobertBroersma so you're saying with the syntax above this query would work?

query FIND_POST_BY_ID($id: Int!) {
  post: post(id: $id) {
    id
    title
    owner {
      name
    }
  }
}

@peterp Is this possible? These constants User and Post are being automatically imported and everything would magically work?

@weaversam8

This comment has been minimized.

Copy link

@weaversam8 weaversam8 commented Mar 24, 2020

Nice find @RobertBroersma!

@cannikin - I haven't tested this locally, but it makes sense that this would work. I kinda like this as a preferred solution to #324 compared to what I proposed.

Edit: I prefer this as a solution because this should actually lazy load relations, instead of ham-handedly fetching all relations when fetching a single model instance, like I proposed in this comment.

@RobertBroersma

This comment has been minimized.

Copy link
Contributor

@RobertBroersma RobertBroersma commented Mar 24, 2020

@cannikin Haven't tested that query, but yes I think that should work! I have tested this query:

query {
  posts {
    title
    owner {
      name
    }
  }
}

Which works! Of course atm you'd need to manually adjust the geneated .sdl.js file to include the relations!

@thedavidprice

This comment has been minimized.

Copy link
Contributor

@thedavidprice thedavidprice commented Mar 24, 2020

Hey, all you geniuses! (...maybe including @cannikin 😉)

When/if this is ironed out, it would be above and beyond helpful to have a topic about this (with an example) in the forum "Troubleshooting & How To".

Pretty please 🙏

@RobertBroersma

This comment has been minimized.

Copy link
Contributor

@RobertBroersma RobertBroersma commented Mar 25, 2020

I was playing around a bit more and had another thought. You can just re-use the service already defined:

// api/services/post/post.js
export const post = ({ id }, { context }) => {
  return db.post.findOne({
    where: { id },
  })
}

export const Post = {
  tags: (args, { root, ...rest}) => post(root, rest).tags(args),
  owner: (args, { root, ...rest}) => post(root, rest).owner(args),
}

I'm not sure on that arg forwarding syntax... but the added benefit is that you can do Authorization stuff in one place (post()). Does that make sense?

I'm quite new to Prisma; calling .findOne() multiple times doesn't cause any extra database work until it's resolved, either by itself or with a sub query, right?

@JohnMilazzo

This comment has been minimized.

@thedavidprice

This comment has been minimized.

Copy link
Contributor

@thedavidprice thedavidprice commented Mar 25, 2020

Thanks @JohnMilazzo I did link from that topic to here as you saw.

Once this discussion resolves to a few good solutions/patterns/examles, I’d like to create a forum Topic in the “How To” that leads with the examples as a starting place. Make sense?

@weaversam8

This comment has been minimized.

Copy link

@weaversam8 weaversam8 commented Mar 25, 2020

@JohnMilazzo @thedavidprice making a "How To" topic seems wise, but I'd also really like to close #324 with a PR for this in the near term, depending on my bandwidth and what we end up deciding re @RobertBroersma's suggestions.


@RobertBroersma - I'm not sure if Prisma caches the calls to findOne(...) or not... that's an open question.

@cannikin

This comment has been minimized.

Copy link
Contributor

@cannikin cannikin commented Mar 25, 2020

Now, who wants to update the SDL/services generator to do this automatically? :)

@weaversam8

This comment has been minimized.

Copy link

@weaversam8 weaversam8 commented Mar 25, 2020

@cannikin that what I meant when I said above that I'd like to close #324 with a PR for this :)

Assuming I have the bandwidth and can figure it out I'm happy to take this one

@cannikin

This comment has been minimized.

Copy link
Contributor

@cannikin cannikin commented Mar 26, 2020

@weaversam8 YES PLEASE!! 😀😀😀

@cannikin

This comment has been minimized.

Copy link
Contributor

@cannikin cannikin commented Mar 28, 2020

If anyone is still looking at this, we got a note from the Prisma team:

Hey folks 👋 another heads-up: A few more upcoming major breaking changes:

Renaming the prisma2 CLI command to prisma

The command that invokes the Prisma 2 CLI will be renamed to just prisma. The recommended installation and usage is:

npm install @prisma/cli --save-dev
npx prisma

New relation syntax

Previously relations could be declared with two relation fields where one was "virtual", the other one "represented the foreign key", e.g.:

model Post {
  id        Int     @id @default(autoincrement())
  title     String
  content   String?
  published Boolean @default(false)
  author    User?
}

model User {
  id    Int     @id @default(autoincrement())
  email String  @unique
  name  String?
  posts Post[]
}

After the next release, you will need to annotate author with the @relation attribute. In the @relation attribute you must specify a "link field" on the same model which references a key on another model (data modeling gets closer to SQL now):

model Post {
  id        Int     @id @default(autoincrement())
  title     String
  content   String?
  published Boolean @default(false)
  author    User?   @relation(fields:  [authorId], references: [id]) // physical relation field
  authorId  Int?    // link field
}

model User {
  id    Int     @id @default(autoincrement())
  email String  @unique
  name  String?
  posts Post[]  // virtual relation field
}

We already updated our new docs to incorporate these changes, you can take a look here: https://prisma2.netlify.com/reference/tools-and-interfaces/prisma-schema/relations

@weaversam8

This comment has been minimized.

Copy link

@weaversam8 weaversam8 commented Mar 28, 2020

Thanks for the heads up @cannikin! Any idea if this beta version is available for testing anywhere?

@cannikin

This comment has been minimized.

Copy link
Contributor

@cannikin cannikin commented Mar 28, 2020

@thedavidprice

This comment has been minimized.

Copy link
Contributor

@thedavidprice thedavidprice commented Mar 30, 2020

@weaversam8 Just to clarify since there was some confusion on our end:

  • Prisma2 preview025 was released last week and has been merged into Redwood per #350. However, we have not yet published a new version of Redwood including this change.
  • Prisma2 beta will be coming next with the breaking changes that Rob mentioned above. We do know now about a release version available now for testing. (I'm assuming the Prisma2 master is what will be released as beta.)

On our end, we'll be taking our time with testing, fixes, and meeting requirements before merging into Redwood for release. As Rob mentioned, here are the Prisma2 docs for Relations in the beta:
https://prisma2.netlify.com/reference/tools-and-interfaces/prisma-schema/relations

@thedavidprice

This comment has been minimized.

Copy link
Contributor

@thedavidprice thedavidprice commented Mar 31, 2020

Prisma 2 Beta was released today #365

There’s a lot of specific updates and changes regarding Relations. @weaversam8 are you still interested in taking a look at helping with this? We’d appreciate it if possible!

@weaversam8

This comment has been minimized.

Copy link

@weaversam8 weaversam8 commented Mar 31, 2020

I might be able to help out with some documentation / issue writing / testing, but I think my bandwidth for doing an actual implementation PR for the generation stuff has been shifted... I'm being pulled onto some COVID-related projects, and understandably that has to take priority right now. Apologies 😢

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.