Mongoose: [Feature] Add hooks methods to ES6 class models (advanced usage with `loadClass`)

Created on 5 Sep 2017  路  6Comments  路  Source: Automattic/mongoose

First of all, I want to say that ES6 class models is the amazing thing, which allows covering mongoose models with static analysis and auto-completion with Flowtype/Typescript. An example with flowtype. Thanks a lot for this ability!

ES6 class models covers static and regular methods, virtuals fields, BUT do not provide any ability to define pre & post hooks.

@vkarpov15 how do you think is there a way to add hooks to the class models somehow like this?

1. Document hooks via instance methods:

These pre/post hooks in models do not require parallel execution or several serial. It can be done via Promise.all or several functions calls inside this methods. Main aim to provide access to hooks via model class definition with static analysis.

class User {
  preSave() {
    this.touchedAt = new Date();
  }
  postSave() { 
    // In `post` hooks changed access to document via `this` 
    // instance scope. (was `doc` arg)
    // It's done due better static analysis.
    console.log(this.id);
  }
  preInit() {}
  postInit() {}
  preValidate() {}
  postValidate() {}
  preRemove() {}
  postRemove() {}
}

2. Model hooks via static methods:

class User {
  static preInsertMany() {
     // `this.` is binded to model context and methods.
  }
  static postInsertMany() {};
}

3. Query and model hooks via static properties:

Via static properties, cause methods have query context in this. So better do not use regular instance/static method for this kind of hooks. For them can be used preQueryHooks and postQueryHooks static properties.

class User {
  static preQueryHooks = {
    count: function() {
      // `this.` is binded to query context
    },
    find: function() {},
    findOne: function() {},
    findOneAndRemove: function() {},
    findOneAndUpdate: function() {},
    update: function() {},
  };

  static postQueryHooks = {
    count: function() {},
    find: function() {},
    findOne: function() {},
    findOneAndRemove: function() {},
    findOneAndUpdate: function() {},
    update: function() {},
  };


  // -----  OR -------- (suppose that this is better)

  static queryHooks = {
    preCount: function() {},
    postCount: function() {},
    // etc.
  }
}

Thoughts?

new feature

Most helpful comment

This can already mostly be achieved with real class inheritance. For example:

/* @flow */
import mongoose, { Model, Schema } from 'mongoose';
import argon2 from 'argon2';

const schema = new Schema(
  {
    email: {
      type: String,
      required: true,
      index: true,
    },

    password: {
      type: String,
      required: true,
      hidden: true,
    },
  }
);

// real inheritance
class UserModel extends Model {
  email: string;
  password: string;
  createdAt: string;
  updatedAt: string;

// ...

  async encryptPassword(password: string): Promise<string> {
    return argon2.hash(password);
  }

  // pre/post-save hook works
  async save(options, next) {
    // Hash the password
    if (this.isModified('password')) {
      this.password = await this.encryptPassword(this.password);
    }

    // call actual save
    await super.save(next).then(doc => { console.log(doc); /* post hook */ });
  }
}

export default mongoose.model(UserModel, schema);

@nodkz btw, it would be nice, if this syntax could be added to the flow-types. I didn't find time to prepare a pull-request.

I like this syntax a lot better and I'm unsure why the hacky loadClass-syntax is promoted via the docs. Are there any reasons for this?

All 6 comments

This can already mostly be achieved with real class inheritance. For example:

/* @flow */
import mongoose, { Model, Schema } from 'mongoose';
import argon2 from 'argon2';

const schema = new Schema(
  {
    email: {
      type: String,
      required: true,
      index: true,
    },

    password: {
      type: String,
      required: true,
      hidden: true,
    },
  }
);

// real inheritance
class UserModel extends Model {
  email: string;
  password: string;
  createdAt: string;
  updatedAt: string;

// ...

  async encryptPassword(password: string): Promise<string> {
    return argon2.hash(password);
  }

  // pre/post-save hook works
  async save(options, next) {
    // Hash the password
    if (this.isModified('password')) {
      this.password = await this.encryptPassword(this.password);
    }

    // call actual save
    await super.save(next).then(doc => { console.log(doc); /* post hook */ });
  }
}

export default mongoose.model(UserModel, schema);

@nodkz btw, it would be nice, if this syntax could be added to the flow-types. I didn't find time to prepare a pull-request.

I like this syntax a lot better and I'm unsure why the hacky loadClass-syntax is promoted via the docs. Are there any reasons for this?

@benbender that syntax feels cleaner, but both of these syntaxes still make it unintuitive to add multiple save hooks (like for plugin support). Any suggestions on how we might do that with a class-based syntax?

@vkarpov15 I don't think there is a clean solution. If you go for class-inheritance, you have a hierarchical model where posthooks (worse: multiple ones!) don't quite fit.

If I had to decide, I would probably go for a clean hook-api outside of the model-class hierarchy, which is exposed by the model, to register those hooks. In this case, the execution of those hooks would be the job of the mongoose core and not the model. I think this would be the only way of a clean separation of concerns - at least as far as I've digged into mongoose.

quick example:

// real inheritance
class UserModel extends Model {
  email: string;
  password: string;

  constructor() {
    this.registerHook('preSave', async () => {
      // Hash the password
      if (this.isModified('password')) {
        this.password = await this.encryptPassword(this.password);
      }
    });

    this.registerHook('postSave', () => { ... });
    // ...
  }

  // ...

  async encryptPassword(password: string) {
    return argon2.hash(password);
  }
}

@benbender Thanks for your solution (first one), it looks very promising 鉂わ笍. But I tried, and it does not work for me with post hooks:

  async save(options, next) {
    // never finished, and `super.save` does not return promise
    const res = super.save(next); 

    // or with `options` save record properly, but again does not return promise
    const res = super.save(options, next);
    console.log(options, next, res); // [Function: fnWrapper] undefined undefined 
  }

Any advise how force super.save to return promise?


but both of these syntaxes still make it unintuitive to add multiple save hooks (like for plugin support). Any suggestions on how we might do that with a class-based syntax?

@vkarpov15 how I know plugins attach to Schema, not Model. Here in this issue we discuss definition of model via class. If I miss, that model also can have its own plugins, maybe we can use something like this:

class User extends Model {
  static plugins = [Plugin1, Plugin2];
}

But it's ugly, better to keep plugins only on schema level. And this.registerHook in constructor (second @benbende example) also not good solution. I think if a developer wants to apply several plugins on model level, I suppose, better somehow to call them explicitly in save method (first @benbende example):

class User extends Model {
    async save(opts) {
       // pre hooks code
       const res = await super.save(opts);
       // post hooks code
       return res;
    }
}

Or with inheritance:

class ModelExtended extends Model {
    async save(opts) {
       // pre hooks code 1
       const res = await super.save(opts);
       // post hooks code 1
       return res;
    }
}
class User extends ModelExtended {
    async save(opts) {
       // pre hooks code 2
       const res = await super.save(opts);
       // post hooks code 2
       return res;
    }
}

// on User.save() we get following calls:
//    pre hooks code 2
//    pre hooks code 1
//    save
//    post hooks code 1
//    post hooks code 2

If somebody wants to call some hooks in parallel, it can use Promise.all syntax:

class User extends Model {
    async save(opts) {
       await Promise.all([ asyncHook1, asyncHook2, ... ]);
       const res = await super.save(opts);
       // post hooks code
       return res;
    }
}

Honestly current synchronious/parallel hooks syntax scary me 馃

Anyway while super.save does not return Promise we cannot keep going with real class inheritance.

Really it will be very cool if pre/post hooks will be removed via class inheritance.

class MyModel extends Model {
  await save() {
    await preSaveHooks;
    return super.save().then(() => postSaveHooks);
  }
  await init() {
    await preInitHooks;
    return super.init().then(() => postInitHooks);
  }
  await validate() {
    await preValidateHooks;
    return super.validate().then(() => postValidateHooks);
  }
  await remove() {
    await preRemoveHooks;
    return super.remove().then(() => postRemoveHooks);
  }

  static async insertMany() {
    await preInsertManyHooks;
    return super.insertMany().then(() => postInsertManyHooks);
  }
  // and so on
}

That unfortunately won't work because of plugins. Feels too much like fitting a square peg into a round hole, OOP isn't the right solution for everything. For one thing, we keep hooks in schemas rather than models for performance reasons, so we can build the function once when the model is compiled.

Was this page helpful?
0 / 5 - 0 ratings