Make `@ResolveField` generic, offering type safety
Is there an existing issue that is already proposing this?
- [x] I have searched the existing issues
Is your feature request related to a problem? Please describe it
Consider the GraphQL entity Person:
@ObjectType()
class Person {
@Field()
id: string;
@Field()
name: string;
@Field(() => [Friend])
Friends: Friend[] // This is a relation we need to resolve
}
When writing the resolver for Person, our @ResolveField usage looks something like
//...
@ResolveField(() => [Friend], { name: "may-not-exist" })
findFriends(@Parent() parent: Person){
//...
}
//...
Here the name for the resolver is an arbitrary string, but in reality it must be specified on the entity for person. Hence the @ResolveField usage is coupled to the Person entities fields. However, we can write whatever we want in the name field, creating room for error.
Describe the solution you'd like
A relatively cost-free solution to this could be to use a generic @ResolveField function, like:
import {
ResolveField,
ResolveFieldOptions,
ReturnTypeFunc,
} from "@nestjs/graphql";
export const ResolveFieldTyped = <T extends { [key: string]: any }>(
typeFunc?: ReturnTypeFunc,
options?: Omit<ResolveFieldOptions, "name"> & { name: keyof T & string },
) => {
return ResolveField(typeFunc, options);
};
Here, the name field is, optionally, typed to the entity you're aiming to resolve. This means our field resolvers are statically checked to be valid before needing to run our schema, and perhaps run into silently incorrect code.
Teachability, documentation, adoption, migration strategy
It is backward compatible, so would not need any of this other than demonstrating the use of a generic
What is the motivation / use case for changing the behavior?
Additional type safety, enforcing safer code
Would you like to create a PR for this issue?
@kamilmysliwiec Yep, I'll submit one in the next couple days
🏓 🙌
I add a bit of my thoughts. Actually, you don't need to have your calculated fields (@ResolveField() in resolver) in your model. The framework will merge declarations from your entity + all @ResolveField() found in a project.
And I personally encourage developers to not put computed fields in the model definitions and below i will explain why.
It's actually bring more typesafety than proposed in this issue changes.
@ObjectType()
class Person {
@Field()
id: string;
@Field()
name: string;
@Field(() => [Friend])
Friends: Friend[]; // This is a relation we need to resolve
}
// person.resolver.ts
@Resolver(() => Person)
class PersonResolver {
@Query(() => Person)
async getOnePerson(
id: string,
): Promise<Person> {
const dbPerson = await personLoader.load(id);
return {
id: dbPerson.id,
name: dbPerson.name,
Friends: [], // <--- here i should return some fake value for field. Or mark return type as Partial<Person>
};
}
@ResolveField(() => [Friend], { name: 'may-not-exist' })
findFriends(@Parent() parent: Person) {
// In child resolvers typescript will not help future developers to understand
// which fields really passed from parent and which are computed on the fly
parent.Friends; // <-- no error, no help from compiler.
}
}
It is much better when resolved fields not specified in the entity:
@ObjectType()
class Person {
@Field()
id: string;
@Field()
name: string;
}
// person.resolver.ts
@Resolver(() => Person)
class PersonResolver {
@Query(() => Person)
async getOnePerson(
id: string,
): Promise<Person> {
const dbPerson = await personLoader.load(id);
// return object which exactly followed Typescript type, no Partial types involved
return {
id: dbPerson.id,
name: dbPerson.name,
// no need to return empty values or null (for possibly non nullable fields)
// Friends: []
};
}
@ResolveField(() => [Friend], { name: 'may-not-exist' })
findFriends(@Parent() parent: Person) {
parent.Friends; // <-- would be error. Typescript types in child resolvers actually reflects runtime objects
}
}