Generics for the model attributes? #125

Activity

1. felixfbecker commented on Sep 5, 2017

   

   felixfbecker

   on Sep 5, 2017

   Collaborator

   More actions

   Why do you think is it more accurate than this?

   class TypedModel extends Model {
     public id: number;
     public optionalData?: object;
   }

   The attributes are attributes of the class, so imo they should be declared there.
   What conveniences will it bring exactly?
2. rokoroku commented on Sep 5, 2017

   

   rokoroku

   on Sep 5, 2017

   Last edited by rokoroku

   ContributorAuthor

   More actions

   when we define custom model method

   class TypedModel extends Model {
     public id: number;
     public optionalData?: object;
     aMethod() {
       ...
     }
     anotherMethod() {
       ...
     }
   }

   aMethod and anotherMethod can be treated as model's attributes in the <K extends keyof this> scenario.

   Furthermore, the model prototype's own method (such as get, set, toJSON, ... ) also treated as their attributes, so it makes the intellisense (quick suggestions) bulky with unnecessary values.
3. SimonSchick commented on Sep 11, 2017

   

   SimonSchick

   on Sep 11, 2017

   Contributor

   More actions

   Making the attributes a model would also properly restrict attributes: [...], and allow .get() to return correct objects.
4. felixfbecker commented on Sep 11, 2017

   

   felixfbecker

   on Sep 11, 2017

   Collaborator

   More actions

   I don't think it's that big of an issue that you can also pass the methods. I don't think that would cause any error in real-world usage. On the other hand, having to duplicate the attribute declaration in both the class and an interface is cumbersome.
5. rokoroku commented on Sep 11, 2017

   

   rokoroku

   on Sep 11, 2017

   ContributorAuthor

   More actions

   Though writing same attribute declaration in class and interface is bit cumbersome, it's not a requirement. We can also write a model without generic thanks to the TypeScripts' default type parameters.

   Like @SimonSchick said, generic attributes will provide more help when we write where or attributes clause, also allowing any sequelize.fn type too.

   If you don't mind, I'll try to make a suggestion PR from below scratch.

   type WhereAttributeHash<TAttributes = any> = {
     [K in keyof TAttributes]?: TAttributes[K] | WhereValue<TAttributes[K]>;
   };
   
   type WhereValue<TAttributes = any> =
     TAttributes
     | null
     | WhereOperators<TAttributes>
     | WhereAttributeHash<TAttributes> // for JSON columns
     | Col<TAttributes> // reference another column
     | OrOperator<TAttributes>
     | AndOperator<TAttributes>
     | WhereGeometryOptions<TAttributes>
     | Array<TAttributes | WhereAttributeHash<TAttributes>>; // implicit $or
   
   type WhereOptions<T = any> = WhereAttributeHash<T> | null | T | T[];
6. SimonSchick commented on Sep 24, 2017

   

   SimonSchick

   on Sep 24, 2017

   Last edited by SimonSchick

   Contributor

   More actions

   One way to achieve this without massive duplication would be:

   type Constructor<T> = new (...args: any[]) => T;
   
   function mixin<TI, T extends typeof Mode l= typeof Model>(cls: T): T & Constructor<TI> {
     return <T & Constructor<TI>> <any> cls;
   }
   
   export interface RoleProperties {
     userName: string;
   }
   
   // tslint:disable-next-line:max-classes-per-file
   export class Role extends mixin<RoleProperties>(Model) {
   ...
   }