fix(types): improve ModelCtor / ModelStatic typing (#13890)

Co-authored-by: Rik Smale <13023439+WikiRik@users.noreply.github.com>

types/lib/model.d.ts

@@ -1612,7 +1612,7 @@ export abstract class Model<TModelAttributes extends {} = any, TCreationAttribut
   /**
    * Returns the attributes of the model
    */
-  public static  getAttributes(): { [attribute: string]: ModelAttributeColumnOptions }; 
+  public static  getAttributes(): { [attribute: string]: ModelAttributeColumnOptions };
 
   /**
    * Reference to the sequelize instance the model was initialized with
@@ -2911,15 +2911,18 @@ export abstract class Model<TModelAttributes extends {} = any, TCreationAttribut
   public isSoftDeleted(): boolean;
 }
 
+/** @deprecated use ModelStatic */
 export type ModelType<TModelAttributes = any, TCreationAttributes = TModelAttributes> = new () => Model<TModelAttributes, TCreationAttributes>;
 
-// Do not switch the order of `typeof Model` and `{ new(): M }`. For
-// instances created by `sequelize.define` to typecheck well, `typeof Model`
-// must come first for unknown reasons.
-export type ModelCtor<M extends Model> = typeof Model & { new(): M };
+type NonConstructorKeys<T> = ({[P in keyof T]: T[P] extends new () => any ? never : P })[keyof T];
+type NonConstructor<T> = Pick<T, NonConstructorKeys<T>>;
 
-export type ModelDefined<S, T> = ModelCtor<Model<S, T>>;
+/** @deprecated use ModelStatic */
+export type ModelCtor<M extends Model> = ModelStatic<M>;
 
-export type ModelStatic<M extends Model> = { new(): M };
+export type ModelDefined<S, T> = ModelStatic<Model<S, T>>;
+
+// remove the existing constructor that tries to return `Model<{},{}>` which would be incompatible with models that have typing defined & replace with proper constructor.
+export type ModelStatic<M extends Model> = NonConstructor<typeof Model> & { new(): M };
 
 export default Model;

types/test/models/User.ts

@@ -60,9 +60,9 @@ User.init(
   {
     version: true,
     getterMethods: {
-    a() {
-      return 1;
-    },
+      a() {
+        return 1;
+      },
     },
     setterMethods: {
       b(val: string) {
@@ -93,10 +93,10 @@ User.init(
 
 User.afterSync(() => {
   sequelize.getQueryInterface().addIndex(User.tableName, {
-      fields: ['lastName'],
-      using: 'BTREE',
-      name: 'lastNameIdx',
-      concurrently: true,
+    fields: ['lastName'],
+    using: 'BTREE',
+    name: 'lastNameIdx',
+    concurrently: true,
   })
 })
 
@@ -134,12 +134,26 @@ User.addScope(
 // associate
 // it is important to import _after_ the model above is already exported so the circular reference works.
 import { UserGroup } from './UserGroup';
+import { UserPost } from "./UserPost";
+
+// associate with a class-based model
 export const Group = User.belongsTo(UserGroup, { as: 'group', foreignKey: 'groupId' });
+// associate with a sequelize.define model
+User.hasMany(UserPost, { as: 'posts', foreignKey: 'userId' });
+UserPost.belongsTo(User, {
+  foreignKey: 'userId',
+  targetKey: 'id',
+  as: 'user',
+});
 
 // associations refer to their Model
 const userType: ModelCtor<User> = User.associations.group.source;
 const groupType: ModelCtor<UserGroup> = User.associations.group.target;
 
+// should associate correctly with both sequelize.define and class-based models
+User.findOne({ include: [{ model: UserGroup }]});
+User.findOne({ include: [{ model: UserPost }]});
+
 User.scope([
   'custom2',
   { method: [ 'custom', 32 ] }

types/test/models/UserPost.ts

@@ -0,0 +1,49 @@
+import { Model, Optional, DataTypes } from 'sequelize';
+import { sequelize } from '../connection';
+
+export interface UserPostAttributes {
+  id: number;
+  userId: number;
+  text: string;
+}
+
+export interface UserPostCreationAttributes
+  extends Optional<UserPostAttributes, 'id'> {}
+
+export interface UserPostInstance
+  extends Model<UserPostAttributes, UserPostCreationAttributes>,
+    UserPostAttributes {}
+
+/**
+ * This is a component defined using `sequelize.define` to ensure that various
+ * functions also work with non-class models, which were the default before
+ * Sequelize v5.
+ */
+export const UserPost = sequelize.define<UserPostInstance>(
+  'UserPost',
+  {
+    id: {
+      type: DataTypes.INTEGER.UNSIGNED,
+      primaryKey: true,
+      autoIncrement: true,
+    },
+    userId: {
+      type: DataTypes.INTEGER.UNSIGNED,
+      allowNull: false,
+    },
+    text: {
+      type: DataTypes.STRING(255),
+      allowNull: false,
+    },
+  },
+  {
+    indexes: [
+      {
+        name: 'userId',
+        fields: ['userId'],
+      },
+    ],
+  },
+);
+
+UserPost.findOne({ where: { id: 1 }});

types/test/type-helpers/deep-writable.ts

@@ -6,10 +6,34 @@
  * Thank you!
  */
 
-import { Model, Sequelize, ModelCtor, ModelType, ModelDefined, ModelStatic } from "sequelize";
+import {
+  Model,
+  Sequelize,
+  ModelCtor,
+  ModelDefined,
+  ModelStatic,
+} from 'sequelize';
 
-type Builtin = string | number | boolean | bigint | symbol | undefined | null | Function | Date | Error | RegExp;
-type SequelizeBasic = Builtin | Sequelize | Model | ModelCtor<Model> | ModelType | ModelDefined<any, any> | ModelStatic<Model>;
+type Builtin =
+  | string
+  | number
+  | boolean
+  | bigint
+  | symbol
+  | undefined
+  | null
+  | Function
+  | Date
+  | Error
+  | RegExp;
+
+type SequelizeBasic =
+  | Builtin
+  | Sequelize
+  | Model
+  | ModelCtor<Model>
+  | ModelDefined<any, any>
+  | ModelStatic<Model>;
 
 // type ToMutableArrayIfNeeded<T> = T extends readonly any[]
 //   ? { -readonly [K in keyof T]: ToMutableArrayIfNeeded<T[K]> }