Redwood: SDL generator ignores relations

Created on 24 Mar 2020  路  6Comments  路  Source: redwoodjs/redwood

This issue is for improving Redwood's relation support (see #316).


Summary

When running yarn rw generate sdl ModelName, if a model include relations, the generated service files will not return values that satisfy the contract enforced by the GraphQL model.

Example

The following Prisma schema (schema.prisma):

// datasource + generator statements omitted
// [...]

model IceCream {
  id       String @id @default(cuid())
  flavor   String
  toppings Topping[]
}

model Topping {
  id   String @id @default(cuid())
  name String
}

should generate the following SDL (combined from a generated icecreams.sdl.js and toppings.sdl.js) when yarn rw generate sdl IceCream and yarn rw generate sdl Topping are run.

export const schema = gql`
  type IceCream {
    id: String!
    flavor: String!
    toppings: [Topping]
  }

  type Topping {
    id: String!
    name: String!
  }

  input IceCreamInput { ... }
  input ToppingInput { ... }

  type Query {
    // ...
    iceCream(id: String!): IceCream
  }

  type Mutation {
    createIceCream(input: IceCreamInput!): IceCream
    // ...
  }
`

The SDL generated is valid, but the services generated don't satisfy the contract that this SDL promises. The service generated in api/services/icecreams/icecreams.js would include this implementation for the iceCream(id: String!) query:

export const iceCream = ({ id }) => {
  return db.iceCream.findOne({
    where: { id },
  })
}

This is not valid, because the value returned from the iceCream(...) function would look something like this:

{
  id: 'ck852f2jz000101mofix935b0',
  flavor: 'chocolate',
  toppings: [ 'ck852fr9f000301mo1p909v3i' ]
}

In reality, Apollo expects that the iceCream(...) function returns a value that looks like this:

{
  id: 'ck852f2jz000101mofix935b0',
  flavor: 'chocolate',
  toppings: [
    {
      id: 'ck852fr9f000301mo1p909v3i'
      name: 'sprinkles'
    }
  ]
}

This is so that the user is allowed to write a GraphQL query that looks like this:
(this query would generate the above result)

query {
  iceCream(id: 'ck852f2jz000101mofix935b0') {
    id
    flavor
    toppings {
      id
      name
    }
  }
}

Solution

Prisma 2 includes methods in the generated Client API for fetching relations along with the initial query. For the services generated to satisfy the SDL, we'd likely want to use the include keyword, for eager loading. The difficulty is: I don't think that the client API has a method to automatically fetch all relations, so we'd need to do them one by one. This isn't trivial, but shouldn't be too hard.

In the above example, the service code for the iceCream(...) function would need to be rewritten to be:

export const iceCream = ({ id }) => {
  return db.iceCream.findOne({
    where: { id },
    include: {
      toppings: true
    }
  })
}
kinimprovement generators prisma

All 6 comments

For reference, in case anyone is stumbling on this from Google search results, this an example of the error that the GraphQL server will surface in a situation like this (since the resolver function within the service isn't returning values that meet the contract.)

{
  "errors": [
    {
      "message": "Cannot return null for non-nullable field IceCream.toppings.",
      "locations": [
        {
          "line": 5,
          "column": 5
        }
      ],
      "path": [
        "iceCream",
        0,
        "toppings"
      ],
      "extensions": {
        "code": "INTERNAL_SERVER_ERROR",
        "exception": {
          "stacktrace": [
            "Error: Cannot return null for non-nullable field IceCream.toppings.",
            "    at completeValue (C:\\Users\\weave\\Documents\\rw-app\\node_modules\\graphql\\execution\\execute.js:560:13)",
            "    at completeValueCatchingError (C:\\Users\\weave\\Documents\\rw-app\\node_modules\\graphql\\execution\\execute.js:495:19)",
            "    at resolveField (C:\\Users\\weave\\Documents\\rw-app\\node_modules\\graphql\\execution\\execute.js:435:10)",
            "    at executeFields (C:\\Users\\weave\\Documents\\rw-app\\node_modules\\graphql\\execution\\execute.js:275:18)",
            "    at collectAndExecuteSubfields (C:\\Users\\weave\\Documents\\rw-app\\node_modules\\graphql\\execution\\execute.js:713:10)",
            "    at completeObjectValue (C:\\Users\\weave\\Documents\\rw-app\\node_modules\\graphql\\execution\\execute.js:703:10)",
            "    at completeValue (C:\\Users\\weave\\Documents\\rw-app\\node_modules\\graphql\\execution\\execute.js:591:12)",
            "    at completeValueCatchingError (C:\\Users\\weave\\Documents\\rw-app\\node_modules\\graphql\\execution\\execute.js:495:19)",
            "    at C:\\Users\\weave\\Documents\\rw-app\\node_modules\\graphql\\execution\\execute.js:618:25",
            "    at Array.forEach (<anonymous>)"
          ]
        }
      }
    }
  ],
  "data": {
    "iceCream": [
      null
    ]
  }
}

IMHO: I would expect the generated schema to be:

  type IceCream {
    id: String!
    flavor: String!
    toppings: [Topping!]!
  }

I don't see why the list or its elements could be null.

What about plain vanilla? No toppings! Or would that just return an empty array? If the array is required, and the element inside is required, that seems like it wouldn鈥檛 allow the array to be empty? I鈥檓 not the GraphQL expert on the team, though.

Yes, that's the case. The list cannot be null, its elements cannot be null, but the list itself can be empty.

That allows the client to write less checking code:

if (iceCream.toppings.length > 0)

Instead of:

if (iceCream.toppings && iceCream.toppings.length > 0)

I love it, I always make my lists required in GraphQL (except in specific cases where it being null actually means something different than being empty).

(This is unrelated to the discussion about GraphQL lists)

FYI for implementation detail for this issue, @RobertBroersma suggested a much better way to do this than my original suggestions above. I wrote a comment about why I think that's a better fit in #316.

Closed by #412

Was this page helpful?
0 / 5 - 0 ratings

Related issues

Tobbe picture Tobbe  路  4Comments

freddydumont picture freddydumont  路  3Comments

weaversam8 picture weaversam8  路  4Comments

cannikin picture cannikin  路  3Comments

josteph picture josteph  路  3Comments