redwood icon indicating copy to clipboard operation
redwood copied to clipboard

feat(codegen): generate cells from graphql schema instead of prisma

Open russell-dot-js opened this issue 1 year ago • 8 comments

This PR includes the changes from #8524

Building off of #8524, this PR changes the source of truth for cell generation from the prisma schema to the graphql schema. This has a multitude of benefits, including:

  • Generate cells for custom queries that aren't direct data access
  • Generate cells for external graphql schemas not managed by redwood (introduced in #8524)
  • Generate cells for queries that have inputs other than a single input named id

This also solves the remainder of #8552 not covered by #8556 (excluding scaffolding for now, because it ain't that important)

Examples:

Given the following sdl:

export const schema = gql`
  type Example {
    id: String!
    name: String!
  }

  input ExampleByIdInput {
    id: String!
  }

  input FilterExamplesInput {
    name: String
  }

  type Query {
    examples(filter: FilterExamplesInput): [Example!]! @requireAuth
    example(input: ExampleByIdInput!, nameMustBe: String): Example @requireAuth
    defaultExample: Example @requireAuth
  }
`

yarn rw g cell examples produces:

export const QUERY = gql`
  query ExamplesQuery($filter: FilterExamplesInput) {
    examples: examples(filter: $filter) {
      id
    }
  }
`

yarn rw g cell example produces:

export const QUERY = gql`
  query FindExampleQuery($input: ExampleByIdInput!, $nameMustBe: String) {
    example: example(input: $input, nameMustBe: $nameMustBe) {
      id
    }
  }
`

yarn rw g cell defaultExample produces:

export const QUERY = gql`
  query FindDefaultExampleQuery {
    defaultExample: defaultExample {
      id
    }
  }
`

I am almost 100% positive this is going to break some unit tests, and I'm happy to fix them, but wanted to get feedback first. I kept the "idType" templateVar only when the query has a single argument named id, to avoid breaking projects with a custom cell.tsx.template or cellList.tsx.template.

I also think it would be good to extend the mockIdValues functionality to properly mock more complex input types, but in the spirit of incremental improvement I believe the added functionality alone is worth merging, without the additional sugar.

russell-dot-js avatar Jun 08 '23 01:06 russell-dot-js

@russell-dot-js Thanks for this. I think you are onto something I have wanted for a long time -- my cell should be based on a query operation and the "shape of the data" and not just for Prisma "finds" per se.

This would allow you to have a service resolves and query operation that returns data from an external api and write a cell for that.

I have been deep into GraphQL subscriptions and live queries now, so I will try to review.

But, I like where this is heading -- even if a breaking change ... which we can do for v6.

dthyresson avatar Jun 08 '23 18:06 dthyresson

@russell-dot-js Thanks for this. I think you are onto something I have wanted for a long time -- my cell should be based on a query operation and the "shape of the data" and not just for Prisma "finds" per se.

This would allow you to have a service resolves and query operation that returns data from an external api and write a cell for that.

I have been deep into GraphQL subscriptions and live queries now, so I will try to review.

But, I like where this is heading -- even if a breaking change ... which we can do for v6.

Glad you like it! I don't think it's a breaking change - you'd have to update your custom cell template if you use it, but that isn't breaking because this is net new functionality. So, if you want to use it and have a custom cell template, update the template. Otherwise you're good to go :)

russell-dot-js avatar Jun 08 '23 21:06 russell-dot-js

Hi all, just wanted to check in on the status of this PR. Let me know when / if I can prepare for release.

russell-dot-js avatar Jun 29 '23 01:06 russell-dot-js

pinging @jtoar & @dthyresson

russell-dot-js avatar Jun 29 '23 01:06 russell-dot-js

@russell-dot-js I'll see if I have enough bandwidth to review today / in the next few days. If not I'll circle back next week. This would most likely land in v5.5, or v6 if the v6 RC ends up being shorter than we expect

jtoar avatar Jun 29 '23 19:06 jtoar

Same @russell-dot-js sorry I haven’t had a chance to give this the time it deserves.

I think the first thing I need to get a handle on is if this is a change to current generation or an alternative .

Meaning if the generator could take a target of a Prisma model or a cell based on an option and generate accordingly. But as I said I do need to think it through

dthyresson avatar Jun 29 '23 20:06 dthyresson

Same @russell-dot-js sorry I haven’t had a chance to give this the time it deserves.

I think the first thing I need to get a handle on is if this is a change to current generation or an alternative .

Meaning if the generator could take a target of a Prisma model or a cell based on an option and generate accordingly. But as I said I do need to think it through

Makes sense! FWIW I can't think of any scenario where generating a graphql query off of the prisma schema will work better than generating off of the graphql schema Reminder that this includes the changes from #8524 so it might be better to finish review of that one first

russell-dot-js avatar Jun 29 '23 21:06 russell-dot-js