objection.js icon indicating copy to clipboard operation
objection.js copied to clipboard

ModelObject type maps optional properties as required

Open onderonur opened this issue 5 years ago • 8 comments

Greetings, I'm using [email protected] and [email protected]. Let's say that I have a simple class like this:

class PersonModel extends Model {
  id: string;
  name: string;
  phone?: number;
}

When I use ModelObject<PersonModel> to create a POJO typing of this class, it gives me this result:

type PersonPojo = {
    id: string;
    name: string;
    phone: number | undefined;
}

As you can see, phone is an optional property in PersonModel class. ModelObject maps it to undefined too, but makes it an required property. I'm using graphql-codegen and to map the typings of the generated response types to my classes, I use ModelObject. Because of it maps these kind of properties as required, if you have lots of relationships under PersonModel, it wants you to set all of those properties (as undefined or its own value type).

To not miss which properties are optional, I've modified ModelObject like this:

type ModelObject2<T extends Model> = Pick<
  T,
  Exclude<NonFunctionPropertyNames<T>, 'QueryBuilderType'>
>;

And when I use ModelObject2<PersonModel>, it protects the optional properties like this:

type PersonPojo = {
    id: string;
    name: string;
    phone?: number | undefined;
}

I'm not sure if this is an expected behavior of ModelObject but wanted to give a heads up about this. Mine can be an edge case which may require a custom solution like ModelObject2 of course. But other people may bump into this kind of an issue too.

Thanks for this great package!

onderonur avatar Sep 01 '20 01:09 onderonur

Didn't optional using ? use to mean exactly the same as T | undefined? Is this distinction something that changed in typescript 4?

koskimas avatar Sep 01 '20 16:09 koskimas

I mean there IS a difference, whether a field is undefined or not set at all, but didn't typescript use to treat them as the same thing?

koskimas avatar Sep 01 '20 18:09 koskimas

Sorry, maybe I didn't explain what is happening with this kind of a transformation. I have a model class like this;

class PersonModel extends Model {
  id: string;
  name: string;
  phone?: number;
  pets?: AnimalModel[]
}

When you use GraphQL, you can return your main record with a single resolver and return its relations with multiple resolvers, as you know. So, you would just call PersonModel.findById(123) to get your main Person record, without its pets. You wouldn't return undefined for it. You would not add this property to your object at all. After that, you would create a pets resolver and call a query for that person's pets.

When you use TypeScript + GraphQL (I use Apollo Server), you need to specify the return type of each resolver. You can use tools like graphql-codegen. It just wants some kind of a interface or class to generate your typings.

So, when we say that user resolver should return PersonModelModel, it needs you to return model methods too. (query, knexQuery etc). When you use ModelObject<PersonModel>, it removes the function properties, and gets a POJO of course. But TypeScript says that the returned object doesn't have pets property. It can be undefined. But can not be omitted, even if you use ?. One way is using Omit<ModelObject<PersonModel>| "pets">, but if you have a lot of relationships, this makes a little bit too much hassle.

Sorry about going into details like GraphQL or codegen, but I wanted to point the use case a little detailed. I hope I could be able to specify the problem and difference here. 🙂

onderonur avatar Sep 02 '20 13:09 onderonur

I have same problem.

nenadfilipovic avatar Dec 29 '20 20:12 nenadfilipovic

Same problem here. Thanks for the workaround @onderonur !

chrishuman0923 avatar Aug 06 '21 20:08 chrishuman0923

This is still a problem! Workaround fixes the typing error

hespinola avatar Oct 21 '21 06:10 hespinola

Tested w/ Typescript 4.5.5

import { NonFunctionPropertyNames } from 'objection'
type NonFunctionProperties<T> = Pick<T, NonFunctionPropertyNames<T>>
type ModelObject<T> = Omit<NonFunctionProperties<T>, 'QueryBuilderType'>
type PersonModelShape = ModelObject<PersonModel>

knicola avatar Feb 13 '22 01:02 knicola

Is the solution to define our own ModelObject as outlined by @knicola ? is the internal type not going to be updated?

ssilve1989 avatar Feb 09 '23 20:02 ssilve1989