feat(codegen): support sdl generation for model with compound id

[russell-dot-js]

partial resolution of #8552

with this prisma schema:

model Temp {
  tenantId  String
  id        String   @default(uuid())
  name String
  @@id([tenantId, id])
}

running yarn rw g sdl temp now generates:

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

  type Query {
    temps: [Temp!]! @requireAuth
    temp(id: TempIdInput!): Temp @requireAuth
  }

  input CreateTempInput {
    tenantId: String!
    name: String!
  }

  input TempIdInput {
    tenantId: String!
    id: String!
  }

  input UpdateTempInput {
    tenantId: String
    name: String
  }

  type Mutation {
    createTemp(input: CreateTempInput!): Temp! @requireAuth
    updateTemp(id: TempIdInput!, input: UpdateTempInput!): Temp! @requireAuth
    deleteTemp(id: TempIdInput!): Temp! @requireAuth
  }
`

It's not perfect for the following reasons:

1. Generated service still expects a single { id } argument (but this may or may not be correct depending on where you get your tenantId from)
2. CreateTempInput is expecting part of the primary key in the request
3. UpdateTempInput duplicates the tenantId property from the TempIdInput

#⁠2 and #⁠3 above are easily fixable. In my opinion #⁠3 above makes sense to fix, but for both #⁠1 and #⁠2, we cannot draw assumptions about what the developer intends to take as an input when using a compound primary key. Sometimes it might be part of the input, sometimes (in the case of tenantId) it might come from the authorized user and be invisible to the API consumers.

The important thing is, now you can generate your SDL's and modify from there. Previously, redwood threw an error and you couldn't use codegen.

This may be considered a breaking change because users who supply their own sdl.{j,t}s.template should now update to support idInput. However, since the feature didn't exist before, I personally don't consider this breaking.

[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]

pinging @jtoar & @dthyresson

[Tobbe]

@russell-dot-js Sorry this PR got stuck. I'll try to get you a decision on some next steps here.
I did find an old PR by @dthyresson that tries to solve the same problem: #6051

As noted in his PR we did decide to hold off on implementing this feature. But with community help (like yours!) maybe we can move forward. I'll discuss with the team.

I do think his implementation looks a bit cleaner. But yours is newer and therefore probably easier to keep working on. Could you please compare your two's PRs and combine the best of both into this one? Either get started right now, or wait until the team has decided if this is something we're ready for now or not.

[Tobbe]

Hey @russell-dot-js it would be awesome if you still wanted to work on this.

The team wants to see this feature, but we don't have time to implement it ourselves now or in the close future. But would be happy to review your code and generally help you along if you're up for it.

Some extra thought will probably have to be given to the Prisma part of the templates, and also the scaffold generator. Especially the forms the scaffolded stuff uses.

Totally understand if you lost the motivation for this. If I don't hear back from you I'll just close this PR for now.

For anyone else reading this, if we don't hear back from Russel you're more than welcome to pick up where this PR left off 🙂

[thedavidprice]

I'm marking this one for close — the work here by @russell-dot-js is great so want to discuss it internally to see if we can first see if others in the community want to pick it up.

[russell-dot-js]

@thedavidprice just saw your comments... a bit demotivating when my 3 pr's were ignored for about 6 months :(

see also:
#8557
#8524

why can't this be merged as-is? there's already the option to use your own template for the generators based on how your app is structured, and it's non-breaking for apps that aren't already using compound id's (and it was previously unusable for compound id's, so in practice this PR, in its current state, has no downside)

[thedavidprice]

@thedavidprice just saw your comments... a bit demotivating when my 3 pr's were ignored for about 6 months :(

Completely understood @russell-dot-js We haven't supported you well across these three PRs. Given that we'd dropped the ball and hadn't heard back from you, I assumed you'd given up on us.

Are you interested in picking this (and possibly the other two) up again? Let me know if so and I'll review with the team to see when we can offer you support. We are a small team and devoting 80% of our focus to Bighorn development right now. But we just talked today about resourcing some work on GQL and codegen, so the timing couldn't be better.

[russell-dot-js]

@thedavidprice just saw your comments... a bit demotivating when my 3 pr's were ignored for about 6 months :(

Completely understood @russell-dot-js We haven't supported you well across these three PRs. Given that we'd dropped the ball and hadn't heard back from you, I assumed you'd given up on us.

Are you interested in picking this (and possibly the other two) up again? Let me know if so and I'll review with the team to see when we can offer you support. We are a small team and devoting 80% of our focus to Bighorn development right now. But we just talked today about resourcing some work on GQL and codegen, so the timing couldn't be better.

I'm happy to get the three PR's across the finish line. It wasn't clear to me what's lacking in any of them from our convos thus far. If you can provide feedback I can find some time to make the changes

[thedavidprice]

I'm happy to get the three PR's across the finish line. It wasn't clear to me what's lacking in any of them from our convos thus far. If you can provide feedback I can find some time to make the changes

Sounds good @russell-dot-js I'll follow up with the team so we can prioritize and get back to you about next steps. Regardless, we'll get you feedback + clarity.

Appreciate your time and involvement. Truly!

[orta]

This hits me roughly once a month, including today - so I'd like to see this get in.

I don't know if you've used this in a real project, but IMO, you'll probably also need to drop the check which throws in the CLI for the scaffolds ( look for missingIdConsoleMessage) otherwise it won't get here (maybe it was added since you started this PR)

[thedavidprice]

@russell-dot-js handing this off to @orta (see his reply). You're in good hands!

[russell-dot-js]

This hits me roughly once a month, including today - so I'd like to see this get in.

I don't know if you've used this in a real project, but IMO, you'll probably also need to drop the check which throws in the CLI for the scaffolds ( look for missingIdConsoleMessage) otherwise it won't get here (maybe it was added since you started this PR)

To be honest, I stopped using redwood after my PR's went stale because I didn't feel confident the framework could keep up with our needs, so I haven't used it in a "real" project, but I POC'd a project I later moved to Nest with this feature, and it was working great at the time. My experience was that >50% of scaffolded code will get deleted or refactored anyway, so just having it work and create the files as an entrypoint was the time saver I was looking for.

Can you please elaborate the issue with missingIdConsoleMessage? This PR still requires you to have an id field, but extends @@id to allow an array or a string.

[Tobbe]

To be honest, I stopped using redwood after my PR's went stale because I didn't feel confident the framework could keep up with our needs, so I haven't used it in a "real" project, but I POC'd a project I later moved to Nest

Thank you so much for telling us this @russell-dot-js 🙏
I really wish more users would tell us both why they stopped using RW and what they went with instead.
This is super valuable to us! Thanks again.

[orta]

Cool, yeah, this should have just gotten merged ages ago then - at some point I'll run the linter over this PR and fix the conflicts and we'll get it in

[orta]

This is now rebased, and passes lints etc, I'm going to hand off to someone else to wrap up getting it merged