Automattic / mongoose

MongoDB object modeling designed to work in an asynchronous environment.
https://mongoosejs.com
MIT License
26.8k stars 3.81k forks source link

insertMany middleware #11531

Open jayantasamaddar opened 2 years ago

jayantasamaddar commented 2 years ago

Dear Mongoose Team,

The post and pre insertMany middleware have the arguments order reversed.

Pre takes in (next, docs), Post takes in (docs, next)

Please follow standardized API design or it gets confusing.

IslandRhythms commented 2 years ago
const mongoose = require('mongoose');

const testSchema = new mongoose.Schema({
    name: String
});

testSchema.pre('insertMany', (docs, next) => {
    console.log('==========================PRE===========================')
    console.log('first', docs);
    console.log('second', next);
    if(Array.isArray(docs)) {
        next();
    } else {
        docs();
    }
});

testSchema.post('insertMany', (docs, next) => {
    console.log('==========================POST==========================');
    console.log('first', docs);
    console.log('second', next);
    if(Array.isArray(docs)) {
        next();
    } else {
        docs();
    }
});

const Test = mongoose.model('Test', testSchema);

async function run() {
    await mongoose.connect('mongodb://localhost:27017');
    await mongoose.connection.dropDatabase();

    await Test.insertMany([{name: 'Test'}, {name: 'Testerson'}]);
}

run();
vkarpov15 commented 2 years ago

Something to consider changing for Mongoose 7. I agree this is very confusing.

assetcorp commented 1 year ago

This is especially confusing in TypeScript code. For example, I have the following code:

Schema.pre('insertMany', async function (results: ISchema[]) {
  if (Array.isArray(results)) {
    for (const item of results) {
      const finder = await Model.findOne({ someId: item.someId })
      if (!finder) {
        throw new Error("Schema must exist before AnotherSchema can be created")
      }
    }
  }
})

Becausepre takes in (next, docs), TypeScript warns that Types of parameters 'results' and 'next' are incompatible.. Thus, I am forced to do Schema.pre('insertMany', async function (next, results: ISchema[]) { for everything to work correctly.

I assume correcting the order might be a breaking change, but I agree with @vkarpov15 that it should be considered for version 7.