Changed type of return value of Model<TInstance, TAttributes>.scope

[Thylossus]

Changed type of return value of Model<TInstance, TAttributes>.scope from this to Model<TInstance, TAttributes>. The following example in https://github.com/Thylossus/sequelize-pull-request-justification-demo/tree/master tries to mimic the example from https://github.com/louy/typed-sequelize/blob/master/README.md.

However, I cannot compile this example due to a type error:

index.ts(16,16): error TS2322: Type 'sequelize.Model<Thing, ThingInstance>' is not assignable to type 'sequelize.SequelizeStaticAndInstance.Model<Thing, ThingInstance>'.
  Types of property 'scope' are incompatible.
    Type '(options?: string | string[] | ScopeOptions | WhereOptions) => Model<Thing, ThingInstance>' is not assignable to type '(options?: string | string[] | ScopeOptions | WhereOptions) => this'.
      Type 'Model<Thing, ThingInstance>' is not assignable to type 'this'.

This is probably related to microsoft/TypeScript#5863. Changing the return type of scope solves the error for me.
Of course, this is not a sufficient criterion for changing your code, but I believe that isn't working for anyone. Feedback about the correctness of this assumption is very welcome.

Thank you for providing the typings!

[felixfbecker]

Your type arguments are switched, it's sequelize.define<TInstance, TAttributes>

[louy]

This is a typo in the readme file. it should be Model<ThingInstance, Thing>. I'll update it.

[felixfbecker]

But that doesn't change the error though. It can't be related to the issue you linked as scope() is not static. I actually think this is a bug in TypeScript, it doesn't get that this in that case is equivalent to Model<ThingInstance, Thing>... But I think changing the return type will not do any harm.

[felixfbecker]

@louy Reopening because that doesn't solve this issue ;)

[Thylossus]

Thank you for your responses. I've updated my code in https://github.com/Thylossus/sequelize-pull-request-justification-demo/tree/master. Nonetheless, the issue remains. In accordance with https://github.com/louy/typed-sequelize/blob/master/index.d.ts#L5296, https://github.com/louy/typed-sequelize/blob/master/index.d.ts#L2578, and https://github.com/louy/typed-sequelize/blob/master/index.d.ts#L3468, I swapped Thing and ThingInstance for Instance, Model, and define.

The error is now

index.ts(19,16): error TS2322: Type 'sequelize.Model<ThingInstance, Thing>' is not assignable to type 'sequelize.SequelizeStaticAndInstance.Model<ThingInstance, Thing>'.
  Types of property 'scope' are incompatible.
    Type '(options?: string | string[] | ScopeOptions | WhereOptions) => Model<ThingInstance, Thing>' is not assignable to type '(options?: string | string[] | ScopeOptions | WhereOptions) => this'.
      Type 'Model<ThingInstance, Thing>' is not assignable to type 'this'.

(basically the same error with Thing and ThingInstance swapped).

[louy]

Right @felixfbecker.
Thanks for taking the time to demonstrate this @Thylossus.

So for some reason the return type of Connection#define is not of type Sequelize.Model.

I'll take a look at it later today. It might be better to have this as an issue rather than a PR as this clearly doesn't fix it.

[Thylossus]

@felixfbecker Oh, you are right, of course. scope isn't static. It is probably rather related to microsoft/TypeScript#7818 and/or microsoft/TypeScript#6223 and thus should be fixed in TypeScript version 2.0 (its part of the 2.0 milestone).

[Thylossus]

@louy Okay, so should I close this PR and open an issue referencing this PR?