Merge pull request #12542 from Automattic/vkarpov15/gh-12472

feat(model): add `mergeHooks` option to `Model.discriminator()` to avoid duplicate hooks
Automattic · Oct 17, 2022 · 8c84754 · 8c84754
2 parents bed6d81 + 2a9208b
commit 8c84754
Show file tree

Hide file tree

Showing 5 changed files with 52 additions and 6 deletions.
diff --git a/lib/helpers/model/discriminator.js b/lib/helpers/model/discriminator.js
@@ -19,11 +19,13 @@ const CUSTOMIZABLE_DISCRIMINATOR_OPTIONS = {
  * ignore
  */
 
-module.exports = function discriminator(model, name, schema, tiedValue, applyPlugins) {
+module.exports = function discriminator(model, name, schema, tiedValue, applyPlugins, mergeHooks) {
   if (!(schema && schema.instanceOfSchema)) {
     throw new Error('You must pass a valid discriminator Schema');
   }
 
+  mergeHooks = mergeHooks == null ? true : mergeHooks;
+
   if (model.schema.discriminatorMapping &&
       !model.schema.discriminatorMapping.isRoot) {
     throw new Error('Discriminator "' + name +
@@ -32,7 +34,7 @@ module.exports = function discriminator(model, name, schema, tiedValue, applyPlu
 
   if (applyPlugins) {
     const applyPluginsToDiscriminators = get(model.base,
-      'options.applyPluginsToDiscriminators', false);
+      'options.applyPluginsToDiscriminators', false) || !mergeHooks;
     // Even if `applyPluginsToDiscriminators` isn't set, we should still apply
     // global plugins to schemas embedded in the discriminator schema (gh-7370)
     model.base._applyPlugins(schema, {
@@ -179,7 +181,9 @@ module.exports = function discriminator(model, name, schema, tiedValue, applyPlu
       schema.options._id = _id;
     }
     schema.options.id = id;
-    schema.s.hooks = model.schema.s.hooks.merge(schema.s.hooks);
+    if (mergeHooks) {
+      schema.s.hooks = model.schema.s.hooks.merge(schema.s.hooks);
+    }
 
     schema.plugins = Array.prototype.slice.call(baseSchema.plugins);
     schema.callQueue = baseSchema.callQueue.concat(schema.callQueue);

diff --git a/lib/model.js b/lib/model.js
@@ -1214,6 +1214,7 @@ Model.exists = function exists(filter, options, callback) {
  * @param {String} [options.value] the string stored in the `discriminatorKey` property. If not specified, Mongoose uses the `name` parameter.
  * @param {Boolean} [options.clone=true] By default, `discriminator()` clones the given `schema`. Set to `false` to skip cloning.
  * @param {Boolean} [options.overwriteModels=false] by default, Mongoose does not allow you to define a discriminator with the same name as another discriminator. Set this to allow overwriting discriminators with the same name.
+ * @param {Boolean} [options.mergeHooks=true] By default, Mongoose merges the base schema's hooks with the discriminator schema's hooks. Set this option to `false` to make Mongoose use the discriminator schema's hooks instead.
  * @return {Model} The newly created discriminator model
  * @api public
  */
@@ -1241,7 +1242,7 @@ Model.discriminator = function(name, schema, options) {
     schema = schema.clone();
   }
 
-  schema = discriminator(this, name, schema, value, true);
+  schema = discriminator(this, name, schema, value, true, options.mergeHooks);
   if (this.db.models[name] && !schema.options.overwriteModels) {
     throw new OverwriteModelError(name);
   }

diff --git a/test/model.discriminator.test.js b/test/model.discriminator.test.js
@@ -2011,4 +2011,32 @@ describe('model', function() {
     assert.strictEqual(doc.shape.get('a').radius, '5');
     assert.strictEqual(doc.shape.get('b').side, 10);
   });
+
+  it('supports `mergeHooks` option to use the discriminator schema\'s hooks over the base schema\'s (gh-12472)', function() {
+    const shapeSchema = Schema({ name: String }, { discriminatorKey: 'kind' });
+    shapeSchema.plugin(myPlugin);
+
+    const Shape = db.model('Test', shapeSchema);
+
+    const triangleSchema = Schema({ sides: { type: Number, enum: [3] } });
+    triangleSchema.plugin(myPlugin);
+    const Triangle = Shape.discriminator(
+      'Triangle',
+      triangleSchema
+    );
+    const squareSchema = Schema({ sides: { type: Number, enum: [4] } });
+    squareSchema.plugin(myPlugin);
+    const Square = Shape.discriminator(
+      'Square',
+      squareSchema,
+      { mergeHooks: false }
+    );
+
+    assert.equal(Triangle.schema.s.hooks._pres.get('save').filter(hook => hook.fn.name === 'testHook12472').length, 2);
+    assert.equal(Square.schema.s.hooks._pres.get('save').filter(hook => hook.fn.name === 'testHook12472').length, 1);
+
+    function myPlugin(schema) {
+      schema.pre('save', function testHook12472() {});
+    }
+  });
 });
diff --git a/test/types/discriminator.test.ts b/test/types/discriminator.test.ts
@@ -18,6 +18,12 @@ const doc: IDiscriminatorTest = new Disc({ name: 'foo', email: 'hi' });
 doc.name = 'bar';
 doc.email = 'hello';
 
+const Disc2 = Base.discriminator<IDiscriminatorTest>(
+  'Disc2',
+  new Schema({ email: { type: String } }),
+  { value: 'test', mergeHooks: false }
+);
+
 function test(): void {
   enum CardType {
     Artifact = 'artifact',

diff --git a/types/models.d.ts b/types/models.d.ts
@@ -1,10 +1,17 @@
 declare module 'mongoose' {
   import mongodb = require('mongodb');
 
+  export interface DiscriminatorOptions {
+    value?: string | number | ObjectId;
+    clone?: boolean;
+    overwriteModels?: boolean;
+    mergeHooks?: boolean;
+  }
+
   export interface AcceptsDiscriminator {
     /** Adds a discriminator type. */
-    discriminator<D>(name: string | number, schema: Schema, value?: string | number | ObjectId): Model<D>;
-    discriminator<T, U>(name: string | number, schema: Schema<T, U>, value?: string | number | ObjectId): U;
+    discriminator<D>(name: string | number, schema: Schema, value?: string | number | ObjectId | DiscriminatorOptions): Model<D>;
+    discriminator<T, U>(name: string | number, schema: Schema<T, U>, value?: string | number | ObjectId | DiscriminatorOptions): U;
   }
 
   interface MongooseBulkWriteOptions {