redwood
redwood copied to clipboard
[Bug?]: SDL-only fields required in generated resolver types
What's not working?
I have SDL-only fields defined on graphql types that resolve primarily to Prisma-defined models. The generated types, e.g. QueryResolvers['posts']
and QueryResolvers['post']
from the example below, have the SDL-only fields required in the return type of the query resolvers. At best, the SDL-only fields should be optional, or even better, the return type of all queries and mutations should just be the type corresponding to the Prisma model since its the Prisma model that serves as the parent type of the Post
type resolver.
Prisma model in api/db/schema.prisma
model Post {
id Int @id @default(autoincrement())
title String
body String
createdAt DateTime @default(now())
}
GQL type and queries in api/src/graphql/posts.sdl.ts
Note dayOfWeek
is NOT a field in the Prisma model.
type Post {
id: Int!
title: String!
body: String!
createdAt: DateTime!
dayOfWeek: String!
}
type Query {
posts: [Post!]! @requireAuth
post(id: Int!): Post @requireAuth
}
Resolvers in api/src/services/posts/posts.ts
export const posts: QueryResolvers['posts'] = () => {
return db.post.findMany()
}
export const post: QueryResolvers['post'] = ({ id }) => {
return db.post.findUnique({
where: { id },
})
}
export const Post = {
dayOfWeek: (_args, { root }) =>
root.createdAt.toLocaleDateString('en-US', {
timeZone: 'UTC',
weekday: 'short',
}),
}
The typescript error reported for the posts
function. A similar error is reported for the post
query resolver.
Type '() => PrismaPromise<Post[]>' is not assignable to type 'OptArgsResolverFn<ResolverTypeWrapper<{ __typename?: "Post"; dayOfWeek: string; body: string; createdAt: Date; id: number; title: string; }>[], {}, RedwoodGraphQLContext, {}>'.
Type 'PrismaPromise<Post[]>' is not assignable to type 'ResolverTypeWrapper<{ __typename?: "Post"; dayOfWeek: string; body: string; createdAt: Date; id: number; title: string; }>[] | Promise<ResolverTypeWrapper<{ __typename?: "Post"; dayOfWeek: string; body: string; createdAt: Date; id: number; title: string; }>[]>'.
Type 'PrismaPromise<Post[]>' is not assignable to type 'Promise<ResolverTypeWrapper<{ __typename?: "Post"; dayOfWeek: string; body: string; createdAt: Date; id: number; title: string; }>[]>'.
Types of property 'then' are incompatible.
Type '<TResult1 = Post[], TResult2 = never>(onfulfilled?: (value: Post[]) => TResult1 | PromiseLike<TResult1>, onrejected?: (reason: any) => TResult2 | PromiseLike<TResult2>) => Promise<...>' is not assignable to type '<TResult1 = ResolverTypeWrapper<{ __typename?: "Post"; dayOfWeek: string; body: string; createdAt: Date; id: number; title: string; }>[], TResult2 = never>(onfulfilled?: (value: ResolverTypeWrapper<{ __typename?: "Post"; dayOfWeek: string; body: string; createdAt: Date; id: number; title: string; }>[]) => TResult1 |...'.
Types of parameters 'onfulfilled' and 'onfulfilled' are incompatible.
Types of parameters 'value' and 'value' are incompatible.
Type 'Post[]' is not assignable to type 'ResolverTypeWrapper<{ __typename?: "Post"; dayOfWeek: string; body: string; createdAt: Date; id: number; title: string; }>[]'.
Type 'Post' is not assignable to type 'ResolverTypeWrapper<{ __typename?: "Post"; dayOfWeek: string; body: string; createdAt: Date; id: number; title: string; }>'.
Property 'dayOfWeek' is missing in type 'Post' but required in type '{ __typename?: "Post"; dayOfWeek: string; body: string; createdAt: Date; id: number; title: string; }'
How do we reproduce the bug?
I set up a minimal proof-of-concept repo: https://gitlab.com/mattlong/redwoodjs-sdl-typescript-issue.
After making sure types have been generated with yarn rw g types
, TS errors are shown on all query and mutation resolvers in api/src/services/posts/posts.ts
.

Despite the typescript errors, the Post service behaves correctly and without error; i.e. the SDL-only field resolver dayOfWeek
can be requested and displayed:

What's your environment? (If it applies)
System:
OS: macOS 12.6.1
Shell: 5.2.2 - /opt/homebrew/bin/bash
Binaries:
Node: 16.17.0 - /private/var/folders/t1/rkgj66nd4fzd_h586hdty40h0000gn/T/xfs-9117000b/node
Yarn: 3.2.3 - /private/var/folders/t1/rkgj66nd4fzd_h586hdty40h0000gn/T/xfs-9117000b/yarn
Databases:
SQLite: 3.37.0 - /usr/bin/sqlite3
Browsers:
Firefox: 105.0.3
Safari: 16.1
npmPackages:
@redwoodjs/core: 3.2.0 => 3.2.0
Are you interested in working on this?
- [X] I'm interested in working on this
Hey @mattlong - thank you for raising a well described issue!
So currently, the Redwood resolver type mapper will only make Prisma relations optional. If the extra property in your object isn't a Prisma relation, it will respect the fact that you said dayOfWeek
is a required field. You could always set it to optional to prevent the TS error.
Is this not expected behaviour for you?
My reasoning - would love to hear your thoughts One of the challenges here is that we want to let you know that you're not returning a required field from your query resolver - but at the same time, with a field resolver you can always "override" how specific fields are resolved. There's clearly a bit of a disconnect here - because you could define any and all the fields in the field/relation resolver rather than the query resolver.
In the case of Prisma relations, we encourage you to use a RelationResolver (usually), because the querying relations can be expensive and unnecessary if your query doesn't need it.
@dac09 Thanks for the reply and your contributions to redwoodjs!
I think the disconnect is that these SDL-only fields might not always be requested by the client and therefore always computing them in query resolvers could be needlessly expensive. e.g. instead of my toy dayOfWeek
example, a more realistic scenario could be an aptly-named expensiveToCompute
field which requires a lot of CPU computation, reading from disk, making an API request to a third party service, etc. In this scenario, you'd ideally only compute expensiveToCompute
when it is actually requested by the client:
export const QUERY = gql`
query FindPostById($id: Int!) {
post: post(id: $id) {
id
title
expensiveToCompute
}
}
`
/* expensiveToCompute is computed */
But when the client does NOT request expensiveToCompute
, you'd skip the expensive computation altogether:
export const QUERY = gql`
query FindPostById($id: Int!) {
post: post(id: $id) {
id
title
}
}
`
/* expensiveToCompute is not computed */
Marking the expensiveToCompute
field as optional would not be appropriate if the desired behavior is for it to always have a non-null value when requested or propagate an error for the query if e.g., reading from disk or making an API request were to fail while computing it.
Hey Matt, good points, and that's exactly what the relation resolvers are meant for - so you can put expensive stuff in there which only gets executed if its been requested. I'm super happy that you picked up on this - and wish there was a good way of documenting this behaviour.
I'm not sure how to solve the problem you're describing here though... I'll try and explain:
The PostRelationResolvers
type is generated by graphql codegen - and includes all the fields in your model - whether you have defined them in your file in the relation resolver or not. This means that I can't pick the defined fields and make them optional (because they're all listed).
But if I remove the PostRelationResolvers
type def from the service, in order to pick out only the ones that are defined in the code (to make them optional in the type)
- export const Post: PostRelationResolvers = {
+ export const Post = {
author: (_obj, { root }) => {
then you lose type safety when writing these relation resolvers.
I hope that makes sense.... bit of a catch 22. Any suggestions?
I would also be very interested in this, as this and another issue leads to our service files basically always being red all over the place.
Also interested in this for the same reasons as @razzeee. We have hundreds of @ts-ignore
s for resolver functions in our services that we'd love to be able to remove.