feat: fix non-nested discriminator hooks & plugins

also add more tests regading this implementation to not break again unnoticed

re mongoose #12472
re mongoose #12604
re mongoose #12613
re mongoose #12696
fixes #768

docs/api/decorators/modelOptions.md

@@ -133,16 +133,75 @@ Only run this while in development. It could cause race-conditions because `getM
 
 #### disablePluginsOnDiscriminator
 
+<span class="badge badge--warning">Deprecated</span><span class="badge badge--warning">No Effect</span>
+
 Default: `false`
 
+This Option has been deprecated (and has no use) since typegoose 9.13.0
+
 Disables adding plugins to a discriminator model.  
 This may be necessary to be set to `true` to not have duplicate plugins or plugin hooks, see [Mongoose#12472](https://github.com/Automattic/mongoose/issues/12472).  
 Only has a effect when [`$isDiscriminator`](#isdiscriminator) is `true`.
 
 #### $isDiscriminator
 
-<span class="badge badge--warning">Internal</span>
+<span class="badge badge--warning">Deprecated</span><span class="badge badge--warning">No Effect</span><span class="badge badge--warning">Internal</span>
+
+This Option has been deprecated (and has no use) since typegoose 9.13.0
 
 Internal function, is only set to `true` for [`buildSchema`](../functions/buildSchema.md) calls through [`getDiscriminatorModelForClass`](../functions/getDiscriminatorModelForClass.md).  
 
 This should not be set manually
+
+#### enableMergePlugins
+
+Default: `false`
+
+Enable Overwriting of the plugins on the "to-be" discriminator schema with the base schema's.
+
+:::caution
+This does not actually "merge plugins", it will overwrite the "to-be" discriminator's plugins with the base schema's!
+:::
+
+:::note
+If [`enableMergePlugins`](#enablemergeplugins) and [`enableMergeHooks`](#enablemergehooks) are both `false`, then the global plugins will be automatically applied by typegoose, see [Mongoose Issue #12696](https://github.com/Automattic/mongoose/issues/12696).
+:::
+
+#### enableMergeHooks
+
+Default: `false`
+
+Enable Merging of Hooks.
+
+Example of what can be deduplicated:
+
+```ts
+// this is a global function and can be de-duplicated, because they are the same reference
+function hookTestTimesGlobal() {}
+
+function pluginTestTimes(schema) {
+  pluginCount += 1;
+  // the following function cannot be de-duplicated, because they are a new reference each time the plugin gets called
+  schema.pre('save', function hookTestTimesNonGlobal() {});
+  schema.pre('save', hookTestTimesGlobal);
+}
+
+@plugin(pluginTestTimes)
+@modelOptions({
+  options: {
+    enableMergeHooks: true, // needs to be set, because by default typegoose does not need de-duplication
+  },
+})
+class MergeHooks {
+  @prop()
+  public dummy?: string;
+}
+```
+
+:::caution
+Only hooks that can be matched against each-other can be de-duplicated.
+:::
+
+:::note
+If [`enableMergePlugins`](#enablemergeplugins) and [`enableMergeHooks`](#enablemergehooks) are both `false`, then the global plugins will be automatically applied by typegoose, see [Mongoose Issue #12696](https://github.com/Automattic/mongoose/issues/12696).
+:::

docs/api/functions/getDiscriminatorModelForClass.md

@@ -94,6 +94,8 @@ const ClickEventModel = getDiscriminatorModelForClass(EventModel, ClickEvent);
 
 ModelOption [`disablePluginsOnDiscriminator`](../decorators/modelOptions.md#disablepluginsondiscriminator) many need to be set to not get duplicate plugins / plugin hooks.
 
+This will not be neccessary for typegoose 9.13.0 and higher.
+
 Example:
 
 ```ts

src/internal/schema.ts

@@ -159,15 +159,9 @@ export function _buildSchema<U extends AnyParamConstructor<any>>(
     const plugins: IPluginsArray[] = Reflect.getMetadata(DecoratorKeys.Plugins, cl);
 
     if (Array.isArray(plugins)) {
-      const mergedOptions = { ...ropt.options, ...overwriteOptions?.options };
-
-      if (!(mergedOptions.$isDiscriminator && mergedOptions.disablePluginsOnDiscriminator)) {
-        for (const plugin of plugins) {
-          logger.debug('Applying Plugin:', plugin);
-          sch.plugin(plugin.mongoosePlugin, plugin.options);
-        }
-      } else {
-        logger.info('Ignoring plugins because it is a discriminator and "disablePluginsOnDiscriminator" is set');
+      for (const plugin of plugins) {
+        logger.debug('Applying Plugin:', plugin);
+        sch.plugin(plugin.mongoosePlugin, plugin.options);
       }
     }
 

src/typegoose.ts

@@ -403,18 +403,29 @@ export function getDiscriminatorModelForClass<U extends AnyParamConstructor<any>
     return models.get(name) as ReturnModelType<U, QueryHelpers>;
   }
 
-  const sch: mongoose.Schema<any> = buildSchema(cl, mergedOptions.schemaOptions, {
-    ...rawOptions,
-    options: { ...rawOptions?.options, $isDiscriminator: true },
-  });
+  const sch: mongoose.Schema<any> = buildSchema(cl, mergedOptions.schemaOptions, rawOptions);
+
+  const mergeHooks = mergedOptions.options?.enableMergeHooks ?? false;
+  // Note: this option is not actually for "merging plugins", but if "true" it will *overwrite* all plugins with the base-schema's
+  const mergePlugins = mergedOptions.options?.enableMergePlugins ?? false;
+
+  if (!mergeHooks && !mergePlugins) {
+    logger.debug('Manually applying global plugins');
+    // apply global plugins to the schema, see https://github.com/Automattic/mongoose/issues/12696
+    ((mergedOptions.existingMongoose ?? mongoose) as any)._applyPlugins(sch);
+  }
 
   const discriminatorKey = sch.get('discriminatorKey');
 
   if (!!discriminatorKey && sch.path(discriminatorKey)) {
     (sch.paths[discriminatorKey] as any).options.$skipDiscriminatorCheck = true;
   }
 
-  const model = from.discriminator(name, sch, value ? value : name);
+  const model = from.discriminator(name, sch, {
+    value: value ? value : name,
+    mergeHooks,
+    mergePlugins,
+  });
 
   return addModelToTypegoose<U, QueryHelpers>(model, cl);
 }

src/types.ts

@@ -455,14 +455,30 @@ export interface ICustomOptions {
    * which will result in the plugins being overwritten and hooks may be duplicated.
    * Only applies to discriminator schemas, not the base for the discriminators themself
    * @see {@link https://github.com/Automattic/mongoose/issues/12472}
+   * @deprecated Not used anymore since version 9.13.0
    * @default false
    */
-  disablePluginsOnDiscriminator?: boolean;
+  disablePluginsOnDiscriminator?: never;
   /**
    * Option if the current class is meant to be a discriminator
+   * @deprecated Not used anymore since version 9.13.0
    * @internal
    */
-  $isDiscriminator?: boolean;
+  $isDiscriminator?: never;
+  /**
+   * Enable Overwriting of the plugins on the "to-be" discriminator schema with the base schema's
+   * Note: this does not actually "merge plugins", it will overwrite the "to-be" discriminator's plugins with the base schema's
+   * If {@link ICustomOptions.enableMergePlugins} and {@link ICustomOptions.enableMergeHooks} are both "false", then the global plugins will be automatically applied by typegoose, see https://github.com/Automattic/mongoose/issues/12696
+   * @default false
+   */
+  enableMergePlugins?: boolean;
+  /**
+   * Enable Merging of Hooks
+   * Note: only hooks that can be matched against each-other can be de-duplicated
+   * If {@link ICustomOptions.enableMergePlugins} and {@link ICustomOptions.enableMergeHooks} are both "false", then the global plugins will be automatically applied by typegoose, see https://github.com/Automattic/mongoose/issues/12696
+   * @default false
+   */
+  enableMergeHooks?: boolean;
 }
 
 /** Type for the Values stored in the Reflection for Properties */

test/tests/discriminators.test.ts

@@ -317,7 +317,7 @@ it('should allow passing ModelOptions through getDiscriminatorModelForClass [typ
   expect(ExtendedWithoutOptionsModel.schema['discriminatorMapping']).toHaveProperty('value', 'ExtendedWithoutOptions');
 });
 
-it('should only apply plugins once', () => {
+it('should only apply plugins and hooks once and still have global plugins', () => {
   let pluginCount = 0;
 
   function hookTestTimesGlobal() {}
@@ -329,7 +329,6 @@ it('should only apply plugins once', () => {
   }
 
   @plugin(pluginTestTimes)
-  @modelOptions({ options: { disablePluginsOnDiscriminator: true } })
   class DisBase {
     @prop()
     public dummy?: string;
@@ -349,7 +348,8 @@ it('should only apply plugins once', () => {
     return v.fn.name === pluginTestTimes.name;
   };
 
-  expect(pluginCount).toStrictEqual(1);
+  // a plugin gets from the ground-up re-applied to each class
+  expect(pluginCount).toStrictEqual(2);
   // test that the plugin only gets applied once in the base model
   {
     const pluginFunctions = (DisBaseModel.schema as any).plugins.filter(pluginFunctionsFilter);
@@ -383,17 +383,172 @@ it('should only apply plugins once', () => {
   const hooksFunctionsFilter2 = (v) => {
     return v.fn.name === hookTestTimesGlobal.name;
   };
-  // test that the plugin's hook(non-global) only gets applied once in the base model
+  // test that the plugin's hook(global) only gets applied once in the base model
   {
     const hookFunctions = (DisBaseModel.schema as any).s.hooks._pres.get('save').filter(hooksFunctionsFilter2);
 
     expect(hookFunctions.length).toStrictEqual(1);
   }
 
-  // test that the plugin's hook(non-global) only gets applied once in the discriminated model
+  // test that the plugin's hook(global) only gets applied once in the discriminated model
   {
     const hookFunctions = (Dis1Model.schema as any).s.hooks._pres.get('save').filter(hooksFunctionsFilter2);
 
     expect(hookFunctions.length).toStrictEqual(1);
   }
 });
+
+it('if "mergePlugins" is "true", it should not contain any new plugins', () => {
+  let pluginCount = 0;
+  let newPluginCount = 0;
+
+  function pluginTestTimes(/* schema */) {
+    pluginCount += 1;
+  }
+
+  @plugin(pluginTestTimes)
+  @modelOptions({
+    options: {
+      enableMergePlugins: true,
+    },
+  })
+  class MergePluginsTrueBase {
+    @prop()
+    public dummy?: string;
+  }
+
+  const MergePluginsTrueBaseModel = getModelForClass(MergePluginsTrueBase);
+
+  function newPlugin() {
+    newPluginCount += 1;
+  }
+
+  @plugin(newPlugin)
+  class MergePluginsTrue1 extends MergePluginsTrueBase {
+    @prop()
+    public dummy2?: string;
+  }
+
+  const MergePluginsTrue1Model = getDiscriminatorModelForClass(MergePluginsTrueBaseModel, MergePluginsTrue1);
+
+  // common filter function
+  const pluginFunctionsFilterTestTimes = (v) => {
+    return v.fn.name === pluginTestTimes.name;
+  };
+  const pluginFunctionsFilterNewPlugin = (v) => {
+    return v.fn.name === newPlugin.name;
+  };
+
+  // a plugin gets from the ground-up re-applied to each class
+  expect(pluginCount).toStrictEqual(2);
+  // test that the plugin only gets applied once in the base model
+  {
+    const pluginFunctions = (MergePluginsTrueBaseModel.schema as any).plugins.filter(pluginFunctionsFilterTestTimes);
+
+    expect(pluginFunctions.length).toStrictEqual(1);
+
+    // "newPlugin" should not be on the base
+    const pluginFunctionsNewPlugin = (MergePluginsTrueBaseModel.schema as any).plugins.filter(pluginFunctionsFilterNewPlugin);
+
+    expect(pluginFunctionsNewPlugin.length).toStrictEqual(0);
+  }
+  // test that the plugin only gets applied once in the discriminated model
+  {
+    const pluginFunctions = (MergePluginsTrue1Model.schema as any).plugins.filter(pluginFunctionsFilterTestTimes);
+
+    expect(pluginFunctions.length).toStrictEqual(1);
+
+    // "newPlugin" should not be on the discriminated schema, because of "mergePlugins"
+    const pluginFunctionsNewPlugin = (MergePluginsTrueBaseModel.schema as any).plugins.filter(pluginFunctionsFilterNewPlugin);
+
+    expect(pluginFunctionsNewPlugin.length).toStrictEqual(0);
+    // but the plugin still gets executed, but is not listed as a plugin anymore
+    expect(newPluginCount).toStrictEqual(1);
+  }
+});
+
+it('should duplicate non-global hooks if "mergeHooks" is "true"', () => {
+  let pluginCount = 0;
+
+  function hookTestTimesGlobal() {}
+
+  function pluginTestTimes(schema) {
+    pluginCount += 1;
+    schema.pre('save', function hookTestTimesNonGlobal() {});
+    schema.pre('save', hookTestTimesGlobal);
+  }
+
+  @plugin(pluginTestTimes)
+  @modelOptions({
+    options: {
+      enableMergeHooks: true,
+    },
+  })
+  class MergeHooksTrueBase {
+    @prop()
+    public dummy?: string;
+  }
+
+  const MergeHooksTrueBaseModel = getModelForClass(MergeHooksTrueBase);
+
+  class MergeHooksTrue1 extends MergeHooksTrueBase {
+    @prop()
+    public dummy2?: string;
+  }
+
+  const MergeHooksTrue1Model = getDiscriminatorModelForClass(MergeHooksTrueBaseModel, MergeHooksTrue1);
+
+  // common filter function
+  const pluginFunctionsFilter = (v) => {
+    return v.fn.name === pluginTestTimes.name;
+  };
+
+  // a plugin gets from the ground-up re-applied to each class
+  expect(pluginCount).toStrictEqual(2);
+  // test that the plugin only gets applied once in the base model (because "mergePlugins" is "false")
+  {
+    const pluginFunctions = (MergeHooksTrueBaseModel.schema as any).plugins.filter(pluginFunctionsFilter);
+
+    expect(pluginFunctions.length).toStrictEqual(1);
+  }
+  // test that the plugin only gets applied once in the discriminated model (because "mergePlugins" is "false")
+  {
+    const pluginFunctions = (MergeHooksTrue1Model.schema as any).plugins.filter(pluginFunctionsFilter);
+
+    expect(pluginFunctions.length).toStrictEqual(1);
+  }
+
+  const hooksFunctionsFilter1 = (v) => {
+    return v.fn.name === 'hookTestTimesNonGlobal';
+  };
+  // test that the plugin's hook(non-global) only gets applied once in the base model (because for the "Base" it only gets applied once)
+  {
+    const hookFunctions = (MergeHooksTrueBaseModel.schema as any).s.hooks._pres.get('save').filter(hooksFunctionsFilter1);
+
+    expect(hookFunctions.length).toStrictEqual(1);
+  }
+
+  // test that the plugin's hook(non-global) gets applied multiple times in the discriminated model because non-global hooks cannot be de-duplicated
+  {
+    const hookFunctions = (MergeHooksTrue1Model.schema as any).s.hooks._pres.get('save').filter(hooksFunctionsFilter1);
+
+    expect(hookFunctions.length).toStrictEqual(2);
+  }
+
+  const hooksFunctionsFilter2 = (v) => {
+    return v.fn.name === hookTestTimesGlobal.name;
+  };
+  // test that the plugin's hook(global) only gets applied once in the base model (because for the "Base" it only gets applied once)
+  {
+    const hookFunctions = (MergeHooksTrueBaseModel.schema as any).s.hooks._pres.get('save').filter(hooksFunctionsFilter2);
+
+    expect(hookFunctions.length).toStrictEqual(1);
+  }
+
+  // test that the plugin's hook(global) only gets applied once in the discriminated model (because global hooks can be de-duplicated)
+  {
+    const hookFunctions = (MergeHooksTrue1Model.schema as any).s.hooks._pres.get('save').filter(hooksFunctionsFilter2);
+
+    expect(hookFunctions.length).toStrictEqual(1);
+  }
+});