fix(hooks): don't expose unsupported hooks on models (#10547)

lib/hooks.js

@@ -34,14 +34,14 @@ const hookTypes = {
   beforeFindAfterOptions: { params: 1 },
   beforeFindAfterOptions: { params: 1 },
   afterFind: { params: 2 },
   afterFind: { params: 2 },
   beforeCount: { params: 1 },
   beforeCount: { params: 1 },
-  beforeDefine: { params: 2, sync: true },
-  afterDefine: { params: 1, sync: true },
-  beforeInit: { params: 2, sync: true },
-  afterInit: { params: 1, sync: true },
+  beforeDefine: { params: 2, sync: true, noModel: true },
+  afterDefine: { params: 1, sync: true, noModel: true },
+  beforeInit: { params: 2, sync: true, noModel: true },
+  afterInit: { params: 1, sync: true, noModel: true },
   beforeAssociate: { params: 2, sync: true },
   beforeAssociate: { params: 2, sync: true },
   afterAssociate: { params: 2, sync: true },
   afterAssociate: { params: 2, sync: true },
-  beforeConnect: { params: 1 },
-  afterConnect: { params: 2 },
+  beforeConnect: { params: 1, noModel: true },
+  afterConnect: { params: 2, noModel: true },
   beforeSync: { params: 1 },
   beforeSync: { params: 1 },
   afterSync: { params: 1 },
   afterSync: { params: 1 },
   beforeBulkSync: { params: 1 },
   beforeBulkSync: { params: 1 },
@@ -123,7 +123,7 @@ const Hooks = {
       }
       }
 
 
       debug(`running hook ${hookType}`);
       debug(`running hook ${hookType}`);
-      return Promise.resolve(hook.apply(this, hookArgs));
+      return hook.apply(this, hookArgs);
     }).return();
     }).return();
   },
   },
 
 
@@ -209,10 +209,13 @@ const Hooks = {
 Hooks.hasHooks = Hooks.hasHook;
 Hooks.hasHooks = Hooks.hasHook;
 
 
 
 
-function applyTo(target) {
+function applyTo(target, isModel = false) {
   _.mixin(target, Hooks);
   _.mixin(target, Hooks);
 
 
   for (const hook of Object.keys(hookTypes)) {
   for (const hook of Object.keys(hookTypes)) {
+    if (isModel && hookTypes[hook].noModel) {
+      continue;
+    }
     target[hook] = function(name, callback) {
     target[hook] = function(name, callback) {
       return this.addHook(hook, name, callback);
       return this.addHook(hook, name, callback);
     };
     };

lib/model.js

@@ -4333,6 +4333,6 @@ class Model {
 }
 }
 
 
 Object.assign(Model, associationsMixin);
 Object.assign(Model, associationsMixin);
-Hooks.applyTo(Model);
+Hooks.applyTo(Model, true);
 
 
 module.exports = Model;
 module.exports = Model;

test/unit/hooks.test.js

@@ -14,6 +14,12 @@ describe(Support.getTestDialectTeaser('Hooks'), () => {
     this.Model = current.define('m');
     this.Model = current.define('m');
   });
   });
 
 
+  it('does not expose non-model hooks', function() {
+    for (const badHook of ['beforeDefine', 'afterDefine', 'beforeConnect', 'afterConnect', 'beforeInit', 'afterInit']) {
+      expect(this.Model).to.not.have.property(badHook);
+    }
+  });
+
   describe('arguments', () => {
   describe('arguments', () => {
     it('hooks can modify passed arguments', function() {
     it('hooks can modify passed arguments', function() {
       this.Model.addHook('beforeCreate', options => {
       this.Model.addHook('beforeCreate', options => {

types/lib/hooks.d.ts

@@ -19,7 +19,7 @@ export type HookReturn = Promise<void> | void;
  * Options for Model.init. We mostly duplicate the Hooks here, since there is no way to combine the two
  * Options for Model.init. We mostly duplicate the Hooks here, since there is no way to combine the two
  * interfaces.
  * interfaces.
  */
  */
-export interface ModelHookOptions<M extends Model = Model> {
+export interface ModelHooks<M extends Model = Model> {
   beforeValidate(instance: M, options: ValidationOptions): HookReturn;
   beforeValidate(instance: M, options: ValidationOptions): HookReturn;
   afterValidate(instance: M, options: ValidationOptions): HookReturn;
   afterValidate(instance: M, options: ValidationOptions): HookReturn;
   beforeCreate(attributes: M, options: CreateOptions): HookReturn;
   beforeCreate(attributes: M, options: CreateOptions): HookReturn;
@@ -45,14 +45,11 @@ export interface ModelHookOptions<M extends Model = Model> {
   afterBulkSync(options: SyncOptions): HookReturn;
   afterBulkSync(options: SyncOptions): HookReturn;
 }
 }
 
 
-export interface AllModelHooks extends ModelHookOptions {
+export interface SequelizeHooks extends ModelHooks {
   beforeDefine(attributes: ModelAttributes, options: ModelOptions<Model>): void;
   beforeDefine(attributes: ModelAttributes, options: ModelOptions<Model>): void;
   afterDefine(model: typeof Model): void;
   afterDefine(model: typeof Model): void;
   beforeInit(config: Config, options: Options): void;
   beforeInit(config: Config, options: Options): void;
   afterInit(sequelize: Sequelize): void;
   afterInit(sequelize: Sequelize): void;
-}
-
-export interface SequelizeHooks extends AllModelHooks {
   beforeConnect(config: Config): HookReturn;
   beforeConnect(config: Config): HookReturn;
   afterConnect(connection: unknown, config: Config): HookReturn;
   afterConnect(connection: unknown, config: Config): HookReturn;
 }
 }

types/lib/model.d.ts

@@ -11,7 +11,7 @@ import {
 } from './associations/index';
 } from './associations/index';
 import { DataType } from './data-types';
 import { DataType } from './data-types';
 import { Deferrable } from './deferrable';
 import { Deferrable } from './deferrable';
-import { AllModelHooks, HookReturn, Hooks, ModelHookOptions } from './hooks';
+import { HookReturn, Hooks, ModelHooks } from './hooks';
 import { ValidationOptions } from './instance-validator';
 import { ValidationOptions } from './instance-validator';
 import { ModelManager } from './model-manager';
 import { ModelManager } from './model-manager';
 import Op = require('./operators');
 import Op = require('./operators');
@@ -1348,7 +1348,7 @@ export interface ModelOptions<M extends Model = Model> {
    * See Hooks for more information about hook
    * See Hooks for more information about hook
    * functions and their signatures. Each property can either be a function, or an array of functions.
    * functions and their signatures. Each property can either be a function, or an array of functions.
    */
    */
-  hooks?: Partial<ModelHookOptions<M>>;
+  hooks?: Partial<ModelHooks<M>>;
 
 
   /**
   /**
    * An object of model wide validations. Validations have access to all model values via `this`. If the
    * An object of model wide validations. Validations have access to all model values via `this`. If the
@@ -2156,49 +2156,6 @@ export abstract class Model<T = any, T2 = any> extends Hooks {
   ): void;
   ): void;
 
 
   /**
   /**
-   * A hook that is run before a define call
-   *
-   * @param name
-   * @param fn   A callback function that is called with attributes, options
-   */
-  public static beforeDefine<M extends Model>(
-    this: { new (): M } & typeof Model,
-    name: string,
-    fn: (attributes: ModelAttributes, options: ModelOptions<M>) => void
-  ): void;
-  public static beforeDefine<M extends Model>(
-    this: { new (): M } & typeof Model,
-    fn: (attributes: ModelAttributes, options: ModelOptions<M>) => void
-  ): void;
-
-  /**
-   * A hook that is run after a define call
-   *
-   * @param name
-   * @param fn   A callback function that is called with factory
-   */
-  public static afterDefine(name: string, fn: (model: typeof Model) => void): void;
-  public static afterDefine(fn: (model: typeof Model) => void): void;
-
-  /**
-   * A hook that is run before Sequelize() call
-   *
-   * @param name
-   * @param fn   A callback function that is called with config, options
-   */
-  public static beforeInit(name: string, fn: (config: Config, options: Options) => void): void;
-  public static beforeInit(fn: (config: Config, options: Options) => void): void;
-
-  /**
-   * A hook that is run after Sequelize() call
-   *
-   * @param name
-   * @param fn   A callback function that is called with sequelize
-   */
-  public static afterInit(name: string, fn: (sequelize: Sequelize) => void): void;
-  public static afterInit(fn: (sequelize: Sequelize) => void): void;
-
-  /**
    * A hook that is run before sequelize.sync call
    * A hook that is run before sequelize.sync call
    * @param fn   A callback function that is called with options passed to sequelize.sync
    * @param fn   A callback function that is called with options passed to sequelize.sync
    */
    */

types/lib/sequelize.d.ts

@@ -555,6 +555,24 @@ export class Sequelize extends Hooks {
   public static beforeFind(fn: (options: FindOptions) => void): void;
   public static beforeFind(fn: (options: FindOptions) => void): void;
 
 
   /**
   /**
+   * A hook that is run before a connection is established
+   *
+   * @param name
+   * @param fn   A callback function that is called with options
+   */
+  public static beforeConnect(name: string, fn: (options: Config) => void): void;
+  public static beforeConnect(fn: (options: Config) => void): void;
+
+  /**
+   * A hook that is run after a connection is established
+   *
+   * @param name
+   * @param fn   A callback function that is called with options
+   */
+  public static afterConnect(name: string, fn: (connection: unknown, options: Config) => void): void;
+  public static afterConnect(fn: (connection: unknown, options: Config) => void): void;
+
+  /**
    * A hook that is run before a find (select) query, after any { include: {all: ...} } options are expanded
    * A hook that is run before a find (select) query, after any { include: {all: ...} } options are expanded
    *
    *
    * @param name
    * @param name

types/test/sequelize.ts

@@ -33,3 +33,12 @@ Sequelize.addHook('beforeCreate', () => {
 }).addHook('beforeBulkCreate', () => {
 }).addHook('beforeBulkCreate', () => {
     // noop
     // noop
 });
 });
+
+Sequelize.beforeConnect(() => {
+
+});
+
+Sequelize.afterConnect(() => {
+
+});
+