Sequelize-typescript: Create methods $getOne and $getMany

[max10rogerio]

Versions

• sequelize: 5.19.8
• sequelize-typescript: 1.0.0
• typescript: 3.6.4

I'm submitting a ...
[ ] bug report
[x] feature request

Using the $get( ) method, the return can be an array or not.
This can causes some confusions.
Example:
Using graphql with typescript, I need return exact type for it, but the $get( ) method returns
Promise<R | R[ ]>, and to work you need force cast it.

I suggest creating two methods:

• $getOne( ): where returns one object.
• $getMany( ) where returns a list.

This visually simplifies the code and avoids casting and future problems.

Example:

GraphQL Example

type User {
  id: ID!
  name: String!
  roles: [Role]
}

type Role {
  id: ID!
  name: String
}

Using $get( )

export const User: UserResolvers = {
  // note: parent is an instance of User (Model)
  roles: async parent => {
    return (await parent.$get('roles')) as Roles[]
  }
}

Example of possible error/confusion

export const User: UserResolvers = {
  // note: parent is an instance of User (Model)
  // note: role need only one object of Role for this example
  role: async parent => {
    const role = (await parent.$get('role')) as Role[]
    return role[0] // this returns NULL
  }
}

If it existed $getMany( )

export const User: UserResolvers = {
  // note: parent is an instance of User (Model)
  roles: parent => parent.$getMany('roles')
  }
}

This is more explicit, easy to read and avoids returns array where an object should be returned and vice versa

I don't know if this will work:

  /**
   * Returns related instance (specified by propertyKey) of source instance
   */
  $getOne<R extends Model<R>>(propertyKey: keyof this, options?: AssociationGetOptions): Promise<R> {
    return this['get' + capitalize(propertyKey as string)](options);
  }

  /**
   * Returns related instance (specified by propertyKey) of source instance
   */
  $getMany<R extends Model<R>>(propertyKey: keyof this, options?: AssociationGetOptions): Promise<R[]> {
    return this['get' + capitalize(propertyKey as string)](options);
  }

Related Issue: #389

[RobinBuschmann]

Hey @max10rogerio, thanks for bringing this up. I just noticed an issue with the current types of $get(...): Actually it should rather be Promise<R | null | R[ ]> than Promise<R | R[ ]>. So that $getOne would be Promise<R | null>, which isn't an exact type either. So type checking or type casting need to be done anyway.

Besides fixing the types, we could implement some type inference instead. So that one doesn't need to distinguish between R | null and R[] anymore:

type GetType<T> = T extends any[] ? T : (T | null);

$get<K extends keyof this>(propertyKey: keyof this, options?: AssociationGetOptions): Promise<GetType<NonNullable<this[K]>>>;

(typescriptlang.org/play)

This ensures that if the type of User['roles'] is an array, the return type of $get is Promise<Role[]>. And if you would have a property role: Role on User, $get('role') would return Promise<Role | null>.

What do you think?

[max10rogerio]

I'm new in typescript, so I'm not really the best one to say if its good or not. But It does seen to solve my problem, thanks!

[JasonEtco]

I came here looking for this issue! The above solution looks like a great implementation. I'd want to continue using the $get method, with smarter typing inferred from the model properties rather than additional methods.

[RobinBuschmann]

@JasonEtco @max10rogerio Here we go: [email protected] or just sequelize-typescript@next

[JasonEtco]

Thanks for the quick action @RobinBuschmann, works like a charm! The only thing is, I'm now able to remove things like:

const relation = await instance.$get('single_relation') as Relation

But that hack was also preventing it from returning <Relation | null>. Is there an obvious way to say that $get() will always return a record, and never null? Sorry if that's out of scope of this issue, but it came up while removing the type overrides that #697 solves.

[RobinBuschmann]

Your welcome

If you are 100% sure that you always get a result, casting is still your best option. From sequelize point of view Relation | null, makes perfectly sense. Since the related data doesn’t necessarily exist on database table.

[max10rogerio]

@RobinBuschmann Thanks!

[JasonEtco]

That makes sense, thanks for the info @RobinBuschmann ✨

[RobinBuschmann]

Closing this, since it is solved