A @Hook decorator would be great

[natesilva]

There are several ways to add hooks, but they need your Model to be added to a Sequelize instance first. It isn’t very declarative.

It would be great if there was a @Hook decorator. Something like:

@Table
class Book extends Model<Book> {
  // (bunch of @Columns etc.)

  @Hook('beforeCreate')
  static reticulateSplines(instance: Book) {
    // do beforeCreate hook stuff here
  }

  @Hook('afterDestroy')
  static herdLlamas(instance: Book) {
    // do afterDestroy hook stuff here
  }
}

Is this in the works? I could try to write something. The decorator would store information using the reflect-metadata system and then something like initializeHooks would be called from the addModels method.

[RobinBuschmann]

@natesilva Currently there is no implementation of hook decorators in progress from my side. But yeah, sounds great. I would be pleased to see this implemented :)
But I would suggest to use @BeforeCreateHook or simply @BeforeCreate (and so on) instead of or at least in addition to the @Hook(...) decorator. Passing strings could lead to typos. @BeforeCreate is more type safe. What do you think?

[natesilva]

I was thinking the argument to Hook(…) would be a string literal type, so it would be safe from typos — and I wouldn’t have to create ~20 named decorators. But having individual decorators is more in keeping with the existing style, especially the Hooks class functions. So let’s do a named decorator for each type of hook.

[RobinBuschmann]

Perfect :) Can I assign this feature to you?

[natesilva]

Yes.

[natesilva]

This works, but there are no tests yet. (yeah I know) Typical usage would be:

@Table
class Person extends Model<Person> {
  @Column
  name: string;

  @BeforeUpdate
  @BeforeCreate
  static capitalizeName(instance: Person) {
    instance.name = instance.name.toLocaleUpperCase();
  }

  @BeforeCreate
  static addUnicorn(instance: Person) {
    instance.name += ' 🦄';
  }
}

If a hook decorator is applied to a non-static method, an informative error will be thrown.

It’s possible to name a hook: @AfterDestroy({ name: 'myHook' }) and then you can remove it: Person.removeHook('afterDestroy', 'myHook'). However this doesn’t work correctly in all cases, still working on it.

[natesilva]

The removeHook issue was a bug in Sequelize. sequelize/sequelize#8095

After tests are written I’ll submit this PR.

[RobinBuschmann]

Looks great! 🎉

[chanlito]

Hi do we really need to pass the instance? Can't it be just "this"?

[RobinBuschmann]

The context of a static function is the reference to the class itself. Therefore this shouldn't be manipulated to be something else than Person in natesilva's example.

[natesilva]

The arguments depend on which hook you are implementing. For some hooks Sequelize passes an instance, for others an array of instances (beforeBulkCreate), or no instance (beforeBulkDestroy). So it can’t be an instance method.

I can imagine a future enhancement where the subset of hooks that receive a single instance create a wrapper function to be the actual hook, which then calls a method on the instance -- allowing you to use this. But that’s well beyond what I’m doing here.