lack of documentation of middleware callback

[adelbke]

Prerequisites

• I have written a descriptive issue title
• I have searched existing issues to ensure the issue has not already been raised

Issue

Middleware callback functions can have varying parameters; sometimes, pre-hooks have different structures than post-hooks, and query middleware is different than document middleware Yet the structure of the callback lacks clear documentation. I humbly believe it needs to be more evident for newcomers and documented.

The only way to notice this is by inspecting the typescript files on my project, which is slow and time-consuming.
I am ready to contribute to making this improvement in the documentation.

[vkarpov15]

What specifically about the middleware callback is confusing? Our middleware docs show examples of using next vs using async functions, which seems to cover most of what I can think of with the middleware callback.

[adelbke]

I'll explain it to you by giving examples of situations I found myself in when working on a task related to this.

Goal:
I am trying to propagate Mongoose schema events to an event-emitter. A one-to-one mapping would be straightforward. Of each event in a given schema to an event name in the event emitter.

Situations:

• Post Hook of save events (a document middleware) fire twice on each save, and in the first instance, when I access the "this" keyword, it is not a proper document instance as this.constructor.modelName is undefined.
• in the post Hook of save events, I expect to pass a function that looks like the following

function (doc, next) { }

This is expected and documented, but there's no indication of what happens when I pass an async function; it seems to be working, but I still have to call next somehow (which is not bad; I would prefer if I could have the choice not to do see and return a promise).

• In Query Middleware, the "pre" function passes everything regarding the query in the this keyword. But Queries are different; sometimes, there can be an update object, a filter Object, or even an Options Object. They are accessible through functions that may or may not be defined depending on the event. If I try to do what I am doing and react to many different events, I would need to do investigative work to determine which type of query this is.

In this case, I think it would be better if the this keyword were more strictly typed to understand what type of object I am manipulating (is this an update or insertMany ? ). If there is already a way for me to type the Query strictly (I believe there is) that I do not understand, then it means it's either hard to find in the documentation or undocumented.
As to my knowledge, the blog post linked in the docs explains clearly that Query is "ambiguous" in middleware and I think that should be updated

• I also noticed something weird in insertMany mentioned in this issue insertMany middleware #11531. The pre and post middleware functions have different orders of parameters.

• In all the other document events, I had to add an if statement checking if this.constructor.modelName is defined because of occasional double event firing like the save case mentioned above.

I hope this was clear; thank you and apologies for the delayed response.

Edit: updated paragraph structure

[vkarpov15]

Post Hook of save events (a document middleware) fire twice on each save, and in the first instance, when I access the "this" keyword, it is not a proper document instance as this.constructor.modelName is undefined. This is incorrect. The following script shows that post save hooks only execute once, there's only one "post save" printed to the console:

'use strict';

const mongoose = require('mongoose');

mongoose.connect('mongodb://127.0.0.1:27017/mongoose_test');

const schema = mongoose.Schema({ name: String });
schema.post('save', function() {
  console.log('post save');
});
const TestModel = mongoose.model('Test', schema);
const doc = new TestModel({ name: 'test' });
doc.save().then(() => console.log('done'));

This is expected and documented, but there's no indication of what happens when I pass an async function; it seems to be working, but I still have to call next somehow (which is not bad; I would prefer if I could have the choice not to do see and return a promise).

That's fair, we don't have an example in our docs of using async functions with post hooks, just with pre hooks. But post hooks work the same way, and you don't need to call next if you have an async function. For example, the following script shows a post save hook with an async function that waits for 1 sec before completing

'use strict';

const mongoose = require('mongoose');

mongoose.connect('mongodb://127.0.0.1:27017/mongoose_test');

const schema = mongoose.Schema({ name: String });
schema.post('save', async function() {
  console.log('Post save start');
  const start = Date.now();
  await new Promise(resolve => setTimeout(resolve, 1000));
  console.log('post save end', Date.now() - start);
});
const TestModel = mongoose.model('Test', schema);
const doc = new TestModel({ name: 'test' });
doc.save().then(() => console.log('done'));

In this case, I think it would be better if the this keyword were more strictly typed to understand what type of object I am manipulating (is this an update or insertMany ? ). If there is already a way for me to type the Query strictly (I believe there is) that I do not understand, then it means it's either hard to find in the documentation or undocumented.
As to my knowledge, the blog post linked in the docs explains clearly that Query is "ambiguous" in middleware and I think that should be updated

That's why we have schema.pre('updateOne') and schema.pre('insertMany') instead of just schema.pre(/.*/). insertMany isn't even query middleware. I guess you mean just for TypeScript types, not runtime behavior?

[adelbke]

Hi @vkarpov15, thanks for responding.

Post Hook of save events (a document middleware) fire twice on each save, and in the first instance, when I access the "this" keyword, it is not a proper document instance as this.constructor.modelName is undefined. This is incorrect. The following script shows that post save hooks only execute once, there's only one "post save" printed to the console:

I just ran that script and it adds up, thank you for pointing that out. I'll check the other dependencies I had... I may have made a mistake

That's why we have schema.pre('updateOne') and schema.pre('insertMany') instead of just schema.pre(/.*/). insertMany isn't even a query middleware. I guess you mean just for TypeScript types, not runtime behavior?

I was referring to typescript types, not runtime behavior. Especially the meaning and behavior of generics in mongoose that I have a hard time grasping (I don't know if this is my fault or the docs tbh).
at the time of writing the issue I tried to set up a single hook for all queries and set up separation logic myself. But resorted to defining a hook for each operation. As it made more sense. Therefore this comment is moot

[vkarpov15]

Yeah I would recommend defining a hook for each operation, or at least a hook for each general class of operations like pre(['find', 'findOne']), pre(['updateOne', 'updateMany']), etc.

However, Query instances are the same for all operations, so there isn't anything wrong with a pre('find') hook that adds details to this.getUpdate() other than the trivial perf overhead of updating an object. find() just won't use the update.

Which generics are you specifically confused about? With automatic schema type inference, you frequently don't need generics at all.