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

Option to disable applyPlugins for discriminator function #12604

Closed
2 tasks done
hasezoey opened this issue Oct 27, 2022 · 0 comments · Fixed by #12613
Closed
2 tasks done

Option to disable applyPlugins for discriminator function #12604

hasezoey opened this issue Oct 27, 2022 · 0 comments · Fixed by #12613
Labels
enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature
Milestone

Comments

@hasezoey
Copy link
Collaborator

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 #12472)

I had now tried #12542 in the project (typegoose) where i had first noticed the problem of hooks being duplicated (from plugins) which got resolved, but now i had noticed that the plugins are still force-fully copied over from the base schema instead of merging or being able to use the plugins applied to the discriminator model.

This is a proposal to add a option similar to mergeHooks to disable applying plugins from the base model (ie disabling the always true of applyPlugins), for example this new options could be named mergePlugins or applyPlugins (like the parameter name) and default to true

schema = discriminator(this, name, schema, value, true, options.mergeHooks);

Example

Current Behavior:

it('supports `mergeHooks` option to use the discriminator schema\'s hooks over the base schema\'s (gh-12472)', function() {
  let pluginTimes = 0;
  const shapeDef = { name: String };
  const shapeSchema = Schema(shapeDef, { discriminatorKey: 'kind' });
  shapeSchema.plugin(myPlugin, { opts1: true });

  const Shape = db.model('Test', shapeSchema);

  const triangleSchema = Schema({ ...shapeDef, sides: { type: Number, enum: [3] } });
  triangleSchema.plugin(myPlugin, { opts2: true });
  const Triangle = Shape.discriminator(
    'Triangle',
    triangleSchema
  );
  const squareSchema = Schema({ ...shapeDef, sides: { type: Number, enum: [4] } });
  squareSchema.plugin(myPlugin, { opts3: true });
  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); // correct
  assert.equal(Square.schema.s.hooks._pres.get('save').filter(hook => hook.fn.name === 'testHook12472').length, 1); // correct

  const squareFilteredPlugins = Square.schema.plugins.filter((obj) => obj.fn.name === 'myPlugin');
  assert.equal(squareFilteredPlugins.length, 1); // correct, should only have been applied once
  assert.equal(squareFilteredPlugins[0].opts['opts3'], true); // FAIL: because it force-applies plugins from the base, instead of the re-applied

  assert.equal(pluginTimes, 3); // correct, 3 times the plugin function is called

  function myPlugin(schema, opts) {
    pluginTimes += 1;
    schema.pre('save', function testHook12472() {});
  }
});

Example new Behavior:

it('supports `mergeHooks` option to use the discriminator schema\'s hooks over the base schema\'s (gh-12472)', function() {
  let pluginTimes = 0;
  const shapeDef = { name: String };
  const shapeSchema = Schema(shapeDef, { discriminatorKey: 'kind' });
  shapeSchema.plugin(myPlugin, { opts1: true });

  const Shape = db.model('Test', shapeSchema);

  const triangleSchema = Schema({ ...shapeDef, sides: { type: Number, enum: [3] } });
  triangleSchema.plugin(myPlugin, { opts2: true });
  const Triangle = Shape.discriminator(
    'Triangle',
    triangleSchema
  );
  const squareSchema = Schema({ ...shapeDef, sides: { type: Number, enum: [4] } });
  squareSchema.plugin(myPlugin, { opts3: true });
  const Square = Shape.discriminator(
    'Square',
    squareSchema,
    { mergeHooks: false, mergePlugins: false } // new option "mergePlugins", could also be named as "applyPlugins"
  );

  assert.equal(Triangle.schema.s.hooks._pres.get('save').filter(hook => hook.fn.name === 'testHook12472').length, 2); // correct
  assert.equal(Square.schema.s.hooks._pres.get('save').filter(hook => hook.fn.name === 'testHook12472').length, 1); // correct

  const squareFilteredPlugins = Square.schema.plugins.filter((obj) => obj.fn.name === 'myPlugin');
  assert.equal(squareFilteredPlugins.length, 1); // correct, should only have been applied once
  assert.equal(squareFilteredPlugins[0].opts['opts3'], true); // pass, because of using the discriminator's plugins instead of force-applied base plugins

  assert.equal(pluginTimes, 3); // correct, 3 times the plugin function is called

  function myPlugin(schema, opts) {
    pluginTimes += 1;
    schema.pre('save', function testHook12472() {});
  }
});

re #12473

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants