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

codegen graphql schema #5213

Merged
merged 21 commits into from
May 2, 2022
Merged

codegen graphql schema #5213

merged 21 commits into from
May 2, 2022

Conversation

Tobbe
Copy link
Member

@Tobbe Tobbe commented Apr 17, 2022

Use codegen instead of programmatically calling the CLI.

This sets us up to better handle #4762
If/when #4849 goes in we could add a link to that when printing the error message

Easiest way to test this PR is to use the Gitpod link and then paste the Book and Shelf models below into schema.prisma

model Book {
  id      Int    @id @default(autoincrement())
  title   String @unique
  Shelf   Shelf? @relation(fields: [shelfId], references: [id])
  shelfId Int?
}

model Shelf {
  id    Int    @id @default(autoincrement())
  name  String @unique
  books Book[]
}

After that just run yarn rw g sdl Book and you'll see an error message printed

@netlify
Copy link

netlify bot commented Apr 17, 2022

Deploy Preview for redwoodjs-docs canceled.

Name Link
🔨 Latest commit bc6a052
🔍 Latest deploy log https://app.netlify.com/sites/redwoodjs-docs/deploys/62703b5b756fbb0008ffca23

@Tobbe Tobbe added the release:feature This PR introduces a new feature label Apr 17, 2022
@orta
Copy link
Contributor

orta commented Apr 17, 2022

Note that this conflicts pretty heavily with #5216 which right now needs to evaluate code before running

@Tobbe
Copy link
Member Author

Tobbe commented Apr 17, 2022

@orta This PR mainly touches graphqlSchema.ts and yours is mainly in graphqlCodeGen.ts. Can you please help me understand how they conflict? Thanks!

@orta
Copy link
Contributor

orta commented Apr 17, 2022

oh good point - it doesn't!

@Tobbe Tobbe force-pushed the tobbe-graphql-schema-gen branch 2 times, most recently from 48d9fa9 to 63736fd Compare April 18, 2022 04:53
Copy link
Contributor

@dthyresson dthyresson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tobbe I added some language from the docs to help resolve the SDL generator type issue.

I think similar language there and a link to the docs when published can be helpful.

@Tobbe Tobbe force-pushed the tobbe-graphql-schema-gen branch from e019bd2 to 0cc8758 Compare April 24, 2022 14:36
@Tobbe
Copy link
Member Author

Tobbe commented Apr 24, 2022

This is what it looks like now. I think I'll add a couple of newlines
image

@Tobbe Tobbe marked this pull request as ready for review April 24, 2022 15:09
@Tobbe Tobbe force-pushed the tobbe-graphql-schema-gen branch from 99e72cd to fa1bca7 Compare April 24, 2022 15:26
@Tobbe
Copy link
Member Author

Tobbe commented Apr 24, 2022

This is what it looks like now, and I'm happy with that.

image

In my own terminal the link looks better

image

@Tobbe Tobbe requested a review from jtoar April 24, 2022 18:28
@dthyresson
Copy link
Contributor

This is what it looks like now, and I'm happy with that.

image

In my own terminal the link looks better

image

This is great @Tobbe . Leaving to @jtoar to finalize language.

@jtoar jtoar self-assigned this Apr 25, 2022
@jtoar
Copy link
Contributor

jtoar commented Apr 26, 2022

@Tobbe Awesome! This is gonna save us a ton of Discord threads. 😄

I have two suggestions:

First, I'm worried the Maybe you need to generate SDL or scaffolding for Book first part may make users think that they need to start over (by git cleaning, etc), generate the other model (here, Book), only to find another error message saying Maybe you need to generate SDL or scaffolding for Shelf first.

So I propose we change it to:

It looks like you have a Shelf model in your Prisma schema.
If it's part of a relation, you may have to generate SDL or scaffolding for Shelf too.
If you haven't done so yet, ignore the error message here and do so now.

My reasoning is that you have to generate one before the other, so don't tell them to start over, just tell them to generate the other one. You could comment out the relation fields, generate the SDLs/scaffolds, comment the relations fields back in, and force generate the SDLs/scaffolds, but that's a few too many steps for me when all I need to do is run the other SDL/scaffold generator. But thoughts?

Second, I feel like the message should stand out more. Maybe we could style it a bit to make it more eye catching?

image

This is the code I added (along with a chalk import—it's already a dependency of this package) to get that message; happy to commit it if you agree:

        console.error(
          [
            `  ${chalk.bgYellow(` ${chalk.black.bold('Heads up')} `)}`,
            '',
            chalk.yellow(
              `  It looks like you have a ${name} model in your Prisma schema.`
            ),
            chalk.yellow(
              `  If it's part of a relation, you may have to generate SDL or scaffolding for ${name} too.`
            ),
            chalk.yellow(
              `  If you haven't done so yet, ignore the error message here and do so now.`
            ),
            '',
            chalk.yellow(
              `  See the ${terminalLink(
                'Troubleshooting Generators',
                'https://redwoodjs.com/docs/schema-relations#troubleshooting-generators'
              )} section in our docs for more help.`
            ),
            '',
          ].join('\n')
        )

@Tobbe
Copy link
Member Author

Tobbe commented Apr 26, 2022

Yes! I ❤️ this Dom! This is so much better than what I had.

What terminal/shell do you have? Why don't you get the nice looking link I get? (iTerm2 + zsh)

To keep iterating on the text a little bit, what do you think of

It looks like you have a Shelf model in your Prisma schema.
If Book and Shelf form a relation, you may have to generate SDL or scaffolding for Shelf too.
So, if you haven't done that yet, ignore this error message and run that generator now.

@jtoar
Copy link
Contributor

jtoar commented Apr 26, 2022

What terminal/shell do you have? Why don't you get the nice looking link I get? (iTerm2 + zsh)

I use the default mac terminal and fish shell. I think the mac terminal is unsupported. It's ok!

I like the copy! If I could suggest one more thing, I'd change If Book and Shelf form a relation to If Book and Shelf have a relation.

I'll add the commit now. There's one thing I don't know how to do yet: how do you get "Book"?

@jnhooper
Copy link
Contributor

jnhooper commented Apr 27, 2022

Hi! sorry to but into this conversation, this all looks great and i think it'll solve an issue i was butting my head against. I was trying to look through this pr to see what your solution looked like but i couldn't find the successful test fixture, so i was hoping someone here could take a quick peek/ this could be illuminating for anyone else coming across this pr like me before this gets released.

model Item {
  id          Int       @id @default(autoincrement())
  name        String
  quantity    Int
    
  createdBy   User      @relation("createdItems", fields: [createdByUserId], references: [id])
  pickedUpBy  User?     @relation("pickedUpItems", fields: [pickedUpById], references: [id])
  

  createdByUserId Int
  pickedUpById    Int

}

model User {
  id                  Int       @id @default(autoincrement())
  email               String    @unique
  name                String
  itemsCreated      Item[]      @relation("createdItems")
  itemsPickedUp     Item[]      @relation("pickedUpItems")
}

then when scaffolding these i run into the problem Unknown type User and Unknown type Item in the respective sdl files. I solved this by just manually copying the types into each other's sdl files

//item.sdl
export const schema = gql`
# had to copy the below manually
  type User {
    id: Int!
    email: String!
    name: String!
    itemsCreated: [Item]!
    itemsPickedUp: [Item]!
  }

# this was auto generated
  type Item {
    id: Int!
    name: String!
    quantity: Int!
    createdBy: User!
    pickedUpBy: User
    createdByUserId: Int!
    pickedUpById: Int!
  }
`

this works but seems... not ideal. is it possible to import the types from one file into another?
Thanks so much for the guidance, i'm really impressed by everything so far

Copy link
Contributor

@dotansimha dotansimha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. It's better to run codegen from core when you have all the info already, instead of using the cli. You can also control how it's being executed and break the dependency to fs (and reduce a lot of dependencies you might not need...)

@jtoar
Copy link
Contributor

jtoar commented May 2, 2022

@jnhooper you shouldn't have to import types into files, just run the sdl or scaffold generator for one model after the other while expecting the first model that to have type errors. Does that make sense? So it'd be:

yarn rw g scaffold users
# Will report type errors, but still scaffold ok
yarn rw g scaffold items
# No errors

@jtoar jtoar enabled auto-merge (squash) May 2, 2022 18:27
@jtoar jtoar merged commit 34d310a into redwoodjs:main May 2, 2022
@jtoar jtoar added this to the next-release milestone May 2, 2022
@Tobbe Tobbe deleted the tobbe-graphql-schema-gen branch May 2, 2022 20:53
dac09 added a commit to dac09/redwood that referenced this pull request May 3, 2022
…ok-smoke-test

* 'main' of github.com:redwoodjs/redwood:
  Remove extra checkout in RC workflow (redwoodjs#5414)
  chore(deps): update dependency @azure/msal-browser to v2.24.0 (redwoodjs#5412)
  fix(deps): update react monorepo (redwoodjs#5406)
  cli upgrade: Always search from the start for semvers (redwoodjs#5368)
  codegen graphql schema (redwoodjs#5213)
  fix(deps): update dependency core-js to v3.22.4 (redwoodjs#5409)
  fix(deps): update typescript-eslint monorepo to v5.22.0 (redwoodjs#5410)
  Add graphql-scalars to graphql-server (redwoodjs#5408)
  Update yarn.lock
  v1.2.1
  fix(deps): update dependency cross-undici-fetch to v0.3.6 (redwoodjs#5402)
  fix(deps): update dependency cross-undici-fetch to v0.3.5 (redwoodjs#5398)
  fix(deps): update dependency cross-undici-fetch to v0.3.3 (redwoodjs#5378)
  chore(story📗): extract MSW logic into a loader (redwoodjs#4919)
@jtoar jtoar modified the milestones: next-release, v1.4.0 May 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

7 participants