IFindOptions can be generic ?

[bilby91]

Hello,

I was wondering if IFindOptions could be a generic type. Sequelize WhereOptions are defined as:

type WhereOptions<T> = {
        [P in keyof T]?: string | number | WhereLogic | WhereOptions<T[P]> | col | and | or | WhereGeometryOptions | Array<string | number> | null;
    };

If we make IFindOptions generic we can take advantage of typesafety on the where key. Basically, https://github.com/RobinBuschmann/sequelize-typescript/blob/master/lib/interfaces/IFindOptions.ts#L14 would be:

import {WhereOptions} from "sequelize";

export interface IFindOptions<T> extends LoggingOptions, SearchPathOptions {
  where?: WhereOptions<T>
}

[RobinBuschmann]

Yeah, makes perfect sense. So that all the static functions in Model.d.ts needs to be adjusted like findAll, which would become static findAll<T extends Model<T>>(options?: IFindOptions<T>): Promise<T[]>;

[bilby91]

Hey @RobinBuschmann, PR submitted :)

[bilby91]

Awesome!!

Can we get a new beta version ?

Thanks as usual for the super fast response time!

[RobinBuschmann]

Strange, the PR build succeeded but after it was merged, the build fails now. Can see the issue? tsc cannot compile some specs anymore. Can you take care of it? Otherwise I will do it tomorrow.

[bilby91]

@RobinBuschmann Check #163

[RobinBuschmann]

Perfect, thanks! I will publish a new beta-release now

[RobinBuschmann]

Its done! npm install sequelize-typescript@next

[schmod]

I don't quite see how this accomplishes the same thing as WhereOptions<T> in @types/sequelize, given how is typically passed into FindOptions:

findAll<TCustomAttributes>(options?: FindOptions<TAttributes & TCustomAttributes>): Promise<TInstance[]>;

In that case, the user is allowed to generify findAll to search on properties present on the model, or some other field(??) as defined in the TCustomAttributes generic.

@types/sequelize has virtually no test coverage of findAll being used in conjunction with generics, and I suspect that there are a lot of pitfalls here that we're going to be exposing ourselves to, given that their tests are all run against a very forgiving Model<any>.

In particular, I'm also concerned that this may have broken or impaired our ability to use custom operators (ie. the legacy '$and', or the current Op.and) to build complex where queries, and have removed support for passing arrays and/or things like sequelize.and into a where clause.

Admittedly, I have no clue how arrays or sequelize.and were ever intended to work as a root-level where clause. It will also be difficult for us to add typesafety around operators like Op.and until TypeScript figures out whether or not it wants to allow ES6 symbols to be used as literals.

[schmod]

As an aside, I have vague concerns about this library's "drift" away from @types/sequelize, and its heavy reliance on static methods on Model, which has led to verbose code and hard-to-debug errors in our codebase (ie. having to write out User.findAll<User>)