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

Avoid id alias globally #10701

Open
chumager opened this issue Sep 9, 2021 · 9 comments
Open

Avoid id alias globally #10701

chumager opened this issue Sep 9, 2021 · 9 comments
Labels
new feature This change adds new functionality, like a new method or class

Comments

@chumager
Copy link

chumager commented Sep 9, 2021

Do you want to request a feature or report a bug?
bug
What is the current behavior?
In one of my plugins I've set id false to avoid id in all my models, it worked on 5.13.x, but now it's not working in 6.0.x
If the current behavior is a bug, please provide the steps to reproduce.

"use strict";
const mongoose = require("mongoose");
const {Schema, model} = mongoose;
const test = new Schema({
  test: {
    type: Number
  }
});
const test2 = new Schema(
  {
    test: {
      type: Number
    }
  },
  {id: false}
);
mongoose.plugin((schema) => schema.set("id", false));
const Test = model("Test", test);
const newTest = new Test();
console.log(newTest.id);
const Test2 = model("Test2", test2);
const newTest2 = new Test2();
console.log(newTest2.id);

What is the expected behavior?
I'd like to be able to set id false globally, instead of using a plugin. For the moment I need a solution to avoid id alias in my models as Schema.set("id", false); is not working in a plugin.
What are the versions of Node.js, Mongoose and MongoDB you are using? Note that "latest" is not a version.
Node 16.8.0
Mongoose 6.0.5
Mongodb atlas 4.4.8

@IslandRhythms IslandRhythms added the confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. label Sep 9, 2021
@IslandRhythms
Copy link
Collaborator

First console log prints an id when it should print undefined

@vkarpov15 vkarpov15 modified the milestones: 6.0.8, 6.0.7 Sep 13, 2021
vkarpov15 added a commit that referenced this issue Sep 18, 2021
@vkarpov15
Copy link
Collaborator

I took a closer look and you're right, this is a bug. We tried to refactor id to be a plugin in #3936 for 6.0, made a note to undo that refactor, but didn't get to it. Lessons learned: make a separate issue instead of treating the issue as "done" but relying on a comment as a mental note to self.

We'll ship 6.0.7 with this fix on Monday 👍

vkarpov15 added a commit that referenced this issue Sep 18, 2021
@chumager
Copy link
Author

Hi, thanks for the solution, but IMHO I think there should be some kind of order to load plugins, instead of other solutions, at least three sections, something like before, middle (or nothing) and after.

Some examples:

  • Get to know if a schema have children or is a child, other plugins could need to know this so this plugin should be loaded at first.
  • Modify indexes, a plugin could want to change the indexes and it should run after all others that can change or create indexes.
  • Define if not exist, a plugin could want to add a helper (method, static or virtual) only if it not exists, but other plugin may define the helper. this should run after others.
  • id alias, this issue, should run at last.

For now I've to use the old school filename method to sort the way the plugins are loaded, but I've several layers, so at first my FW plugins are loaded, then the default apps plugins and finally the app plugins, so this kind of sort it's not perfect.

Regards.

@vkarpov15
Copy link
Collaborator

The intention of plugins is that order is up to the end developer. The order you call .plugin() is the order that the plugins are applied. Things just get a bit tricky with Mongoose's built-in plugins, because those are always defined first.

Another way is to modify mongoose.plugins directly. plugins is just an array of [function, PluginOptions], you can shuffle that around as you need to.

@chumager
Copy link
Author

chumager commented Oct 1, 2021

Hi, thanks for the answer, but I'm not able to "sort" manually the plugins, because I develop a framework, so the "plugin" stage is static and independent the framework user, maybe a could add a "position" to the plugin object definition.

Regards.

@vkarpov15 vkarpov15 reopened this Oct 2, 2021
@vkarpov15 vkarpov15 modified the milestones: 6.0.7, 6.0.9 Oct 2, 2021
@vkarpov15 vkarpov15 added enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature and removed confirmed-bug We've confirmed this is a bug in Mongoose and will fix it. labels Oct 2, 2021
@vkarpov15
Copy link
Collaborator

@chumager so something like mongoose.plugin(myPlugin, { position: 0 }) // <-- force this plugin to be first, no matter what ? Or are you looking for something more sophisticated, like support for a plugin dependency graph?

@vkarpov15 vkarpov15 removed this from the 6.0.9 milestone Oct 2, 2021
@vkarpov15 vkarpov15 added discussion If you have any thoughts or comments on this issue, please share them! and removed enhancement This issue is a user-facing general improvement that doesn't fix a bug or add a new feature labels Oct 2, 2021
@chumager
Copy link
Author

chumager commented Oct 4, 2021

Hi @vkarpov15, I already thought about that kind of solution as I used in other parts of my code, but the meaning of using inside Mongoose is to help other developers.

My plugin objects looks like this:

{
  plugin(schema, options) {//do something},
  schemas: [ ],//optional, could be global
  position: x// new, position
}

But yes... that's what I'm developing right now.

Best regards.

@vkarpov15
Copy link
Collaborator

@chumager how would that work if two plugins have the same position? Or would that throw an error?

@chumager
Copy link
Author

Hi @vkarpov15
Just like any other sort system, FIFO...

@vkarpov15 vkarpov15 added this to the 6.x Unprioritized milestone Oct 14, 2021
@vkarpov15 vkarpov15 added new feature This change adds new functionality, like a new method or class and removed discussion If you have any thoughts or comments on this issue, please share them! labels Oct 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature This change adds new functionality, like a new method or class
Projects
None yet
Development

No branches or pull requests

3 participants