Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

No global plugins applied when using mergePlugins on discriminators #12696

Closed
2 tasks done
hasezoey opened this issue Nov 17, 2022 · 0 comments · Fixed by #12833
Closed
2 tasks done

No global plugins applied when using mergePlugins on discriminators #12696

hasezoey opened this issue Nov 17, 2022 · 0 comments · Fixed by #12833
Labels
discussion If you have any thoughts or comments on this issue, please share them!
Milestone

Comments

@hasezoey
Copy link
Collaborator

hasezoey commented Nov 17, 2022

Prerequisites

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

Issue

(Continuation of #12604)

Since #12613 introduced a way to turn off the "overwrite of the plugins from the base schema", but now i noticed that no global plugins (mongoose._applyPlugins) are applied, which does not make the schemas the same as when calling with mongoose.model.

mongoose/lib/index.js

Lines 570 to 575 in c77dd1b

if (schema) {
if (_mongoose.get('cloneSchemas')) {
schema = schema.clone();
}
_mongoose._applyPlugins(schema);
}


Test Code and Output

Reproduction Repository / Branch: https://github.com/typegoose/typegoose-testing/tree/testHooks672

// NodeJS: 18.10.0
// MongoDB: 5.0 (Docker)
// Typescript 4.8.4
import * as mongoose from 'mongoose'; // mongoose@6.7.2

const eventSchema = new mongoose.Schema(
  {
    time: Date,
  },
  { discriminatorKey: 'kind' }
);
eventSchema.pre('validate', function testv() {
  console.log('validate', this.constructor.name, this.modelName);
});

const clickedLinkEvent = eventSchema.clone();
clickedLinkEvent.add({ url: String });

const signedUpEvent = eventSchema.clone();
signedUpEvent.add({ user: String });

console.log('TEST0', (eventSchema as any).s.hooks._pres);

const EventModel = mongoose.model('Event', eventSchema);
const ClickedLinkEventModel = EventModel.discriminator('ClickedLinkEvent', clickedLinkEvent, { mergeHooks: false, mergePlugins: false });

(async () => {
  await mongoose.connect(`mongodb://localhost:27017/`, {
    dbName: 'verifyMASTER',
  });

  const events = await Promise.all([
    EventModel.create({ time: new Date(Date.now()), url: 'google.com' }),
    ClickedLinkEventModel.create({ time: Date.now(), url: 'google.com' }),
  ]);

  console.log('TEST1', (EventModel.schema as any).s.hooks._pres);
  console.log('TEST2', (ClickedLinkEventModel.schema as any).s.hooks._pres);

  await mongoose.disconnect();
})();

Output with mergeHooks: true or mergePlugins: true:

TEST0 Map(1) { 'validate' => [ { fn: [Function: testv], isAsync: false } ] }
validate model undefined
validate model undefined
TEST1 Map(3) {
  'validate' => [ { fn: [Function: testv], isAsync: false } ],
  'save' => [
    { fn: [Function: validateBeforeSave], isAsync: false },
    { fn: [Function: saveSubdocsPreSave], isAsync: false },
    { fn: [Function: shardingPluginPreSave], isAsync: false },
    { fn: [Function: trackTransactionPreSave], isAsync: false }
  ],
  'remove' => [
    { fn: [Function: removeSubDocsPreRemove], isAsync: false },
    { fn: [Function: shardingPluginPreRemove], isAsync: false }
  ]
}
TEST2 Map(3) {
  'validate' => [ { fn: [Function: testv], isAsync: false }, numAsync: 0 ],
  'save' => [
    { fn: [Function: validateBeforeSave], isAsync: false },
    { fn: [Function: saveSubdocsPreSave], isAsync: false },
    { fn: [Function: shardingPluginPreSave], isAsync: false },
    { fn: [Function: trackTransactionPreSave], isAsync: false },
    numAsync: undefined
  ],
  'remove' => [
    { fn: [Function: removeSubDocsPreRemove], isAsync: false },
    { fn: [Function: shardingPluginPreRemove], isAsync: false },
    numAsync: undefined
  ]
}

Output with mergeHooks: false or mergePlugins: false:

TEST0 Map(1) { 'validate' => [ { fn: [Function: testv], isAsync: false } ] }
validate model undefined
TEST1 Map(3) {
  'validate' => [ { fn: [Function: testv], isAsync: false } ],
  'save' => [
    { fn: [Function: validateBeforeSave], isAsync: false },
    { fn: [Function: saveSubdocsPreSave], isAsync: false },
    { fn: [Function: shardingPluginPreSave], isAsync: false },
    { fn: [Function: trackTransactionPreSave], isAsync: false }
  ],
  'remove' => [
    { fn: [Function: removeSubDocsPreRemove], isAsync: false },
    { fn: [Function: shardingPluginPreRemove], isAsync: false }
  ]
}
TEST2 Map(1) {
  'validate' => [ { fn: [Function: testv], isAsync: false }, numAsync: undefined ]
}

Workaround is to add mongoose._applyPlugins(discriminatorSchema) before calling from.discriminator, like:

mongoose._applyPlugins(clickedLinkEvent);
const ClickedLinkEventModel = EventModel.discriminator('ClickedLinkEvent', clickedLinkEvent, { mergeHooks: false, mergePlugins: false });

I opened this as "Other" with "discussion", because i am not sure if this wanted to be implemented as a "else" case of mergeHooks / mergePlugins, or as a options or at all.


PS: there also seems to be something wrong with this.modelName and this.constructor.name in the validate hook or the types are wrong and this is not actually a full Document.

@hasezoey hasezoey added the discussion If you have any thoughts or comments on this issue, please share them! label Nov 17, 2022
@vkarpov15 vkarpov15 added this to the 6.7.5 milestone Nov 28, 2022
@vkarpov15 vkarpov15 modified the milestones: 6.8.1, 6.8.2 Dec 19, 2022
vkarpov15 added a commit that referenced this issue Dec 24, 2022
…en if `mergeHooks` and `mergePlugins` are both false

Fix #12696
vkarpov15 added a commit that referenced this issue Dec 26, 2022
fix(discriminator): apply built-in plugins to discriminator schema even if `mergeHooks` and `mergePlugins` are both false
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion If you have any thoughts or comments on this issue, please share them!
Projects
None yet
2 participants