fix: improved typing for ModelObject<T>

[flodlc]

The current ModelObject does not preserve ? optional properties.

Example:

export class User extends Model {
  id: string;
  name: string;
  parentId?: User
}

function insertUser(user: Omit<ModelObject<User>, "id">) {...}

// Typescript will throw an error here. Missing property parentId
insertUser({
    name: "john"
});

// Unfortunately, I have to manually set parentId: undefined
insertUser({
    name: "john",
    parentId: undefined
});

It's not surprising since with this declaration we lose the ? optional syntax

type ModelObject<T extends Model> = {
    [K in DataPropertyNames<T>]: T[K];
};

Typescript has a native Pick<Type, Keys> utility Type for this which preserves optional properties and allows a shorter syntax.

Not sure if it can be used also for PartialModelObject
What do you think ?

[koskimas]

This is the implementation of Pick

type Pick<T, K extends keyof T> = {
    [P in K]: T[P];
};

which is 100% the same as what we have.

[flodlc]

This is the implementation of Pick

type Pick<T, K extends keyof T> = {
    [P in K]: T[P];
};

which is 100% the same as what we have.

@koskimas
It's not the same.
The difference is that the original Pick extends keyof T so it keeps all the keys information of the given keys instead of only iterating on the given key names.

type Pick<T, K extends keyof T> = {
    [P in K]: T[P];
}
// Pick keys extends keyof T so it keeps `?`

Some examples with a simple User

[posgarou]

@koskimas Can this be reopened and considered? @flodlc's solution fixes some type issues I've run into.

The snippet below fails to compile with the current definition of ModelObject, but passes with the proposal from @flodlc.

class Cat extends Model {
  age?: number;
}

type CatObject = ModelObject<Cat>;

const cat = Cat.fromJson({})
const catObject: CatObject = cat;
// Error: Type 'Cat' is not assignable to type 'CatObject'.
// Property 'age' is optional in type 'Cat' but required in type 'CatObject'

[lehni]

I'm going with @falkenhawk's suggestion here and go ahead and merge this: #2299 (comment)