Integration for ES6 classes #4668

kherock · 2016-10-30T09:41:06Z

The reason for this PR is that I noticed that mongoose under the hood is essentially acting as a mixin to a class, and I saw room for improvement with the syntax for defining models. Generally static and instance methods have to be inconveniently defined through helpers on a Schema before they are copied onto the model, and Model.compile is also being inefficient by manually setting the prototype. So essentially, mongoose is doing work that ES6 already natively supports.

With these changes mongoose.model can support taking an ES6 class where name is normally provided:

const mongoose = require('mongoose');
const { Model, Schema } = mongoose;

const schema = new Schema({
  _id: Schema.ObjectId,
  field1: Number,
  field2: String
});
class SomeThing extends Model {
  static explode() {
    return SomeThing.explode();
  }

  get numString() {
    return this.field1 + this.field2;
  }

  increment() {
    return SomeThing.update({ _id: this._id }, {
      $inc: { 'field1': 1 }
    }, { new: true }).exec();
  }
}

module.exports = mongoose.model(SomeThing, schema, 'some_things');

I've also included a loadClass instance method for schemas that accomplishes some of what mongoose-class-wrapper can do. It allows similar functionality as the above, but it's useful for sub-schema definitions.

This should be fully backward compatible and shouldn't introduce any breaking changes.

…S6 classes

kherock · 2016-11-04T03:33:35Z

Just looking ahead even further, eventually the Schema constructor could be an @model class decorator if we opt to support Babel. This would mean we don't have to manually call mongoose.model at the end and we could have something like

@model({
  _id: Schema.ObjectId,
  field1: Number,
  field2: String
}, { collection: 'some_things' })
export class SomeThing extends Model { ... }

which IMO looks very nice.

vkarpov15 · 2016-11-05T03:02:46Z

lib/schema.js

+  }
+
+  // Add static methods
+  if (!virtualsOnly) {


Hmm what's the point of this virtualsOnly option? Looks like it's always true anyway...

I suppose it's probably unnecessary. Its only use is mainly internal by Model.compile. That's also why I didn't add JSDoc for the parameter.

It seems like Schemas need the virtuals defined on them directly for them to function properly, e.g. in toObject and toJSON. It just felt wasteful copying all of the statics and instance methods from the class onto the schema when applyMethods(model, schema) and applyStatics(model, schema) are being called immediately afterward.

To clarify, loadClass is intended to be public, used generally when someone might want to use a class to define virtuals, etc. on a subschema, like

const subSchema = new Schema({ ... }); subSchema.loadClass(class { ... }); const schema = new Schema({ field1: subSchema }); class MyModel extends Model { ... }

In this case virtualsOnly is undefined.

mleanos · 2016-11-21T00:43:32Z

lib/index.js

+  if (typeof name === 'function') {
+    model = name;
+    name = model.name;
+    if (!(model.prototype instanceof Model)) {


Correct me if I'm wrong.. I don't think this would work in the case when using a Base class on an individual class.

export class Foo extends MyBase {...}

No, instanceof looks through the whole prototype chain. So as long as MyBase extends Model, it's fine. I'm actually currently using this use of extends for some of the models in my app.

Thank you for clarifying that. I didn't know that about instanceof; obviously :)

mleanos · 2016-11-21T00:46:02Z

lib/index.js

@@ -393,7 +401,7 @@ Mongoose.prototype.model = function(name, schema, collection, skipInit) {
  }

  var connection = options.connection || this.connection;
-  model = this.Model.compile(name, schema, collection, connection, this);
+  model = this.Model.compile(model || name, schema, collection, connection, this);


Is this change needed? name is being set above when model is set.

Not sure where the problem you see is, can you elaborate? model is unset if the name passed in is a string, which means that Model.compile receives a string name rather than an ES6 class, and everything proceeds normally.

You're right. I just saw the diff and thought it wasn't necessary based on my misunderstanding of what the compile method was doing.

When I first looked at this, I thought this bit was still inside the if (typeof name === 'function').. scope.

mleanos · 2016-11-21T00:51:24Z

I'm very interested in having this feature. I would like to decouple my application's classes from my data access layer. If I can provide an es6 class in lieu of the Mongoose Schema definition, then I can effectively use the same classes with a different database technology.

Without this PR's implementation, is there a recommend way of achieving this?

kherock · 2016-11-21T01:11:16Z

@mleanos Yeah, that is the direction I'd like to see mongoose go.

This is mostly attainable using the mongoose-class-wrapper plugin, but obviously it's less convenient and uglier, and IDEs that support ES6 don't aren't aware of the methods provided by Model.

malixsys · 2016-11-24T14:54:19Z

@mleanos this is now merged and released in 4.7

malixsys · 2016-11-24T14:55:00Z

@rockmacaca 👏 👏 👏 Thank you for this!

TrejGun · 2016-11-25T02:24:36Z

🎉

vkarpov15 · 2016-11-27T00:37:42Z

🎉

klimashkin · 2017-01-06T01:03:05Z

Is there docs for this feature?
How to deal with path, pre, etc?

kherock · 2017-01-06T02:58:20Z

@klimashkin I suppose there probably should be an example like the one I initially gave in the API docs. This shouldn't affect anything with path or pre, since those are for schemas.

However, depending on how you're set up, you might be able to natively add pre and post hooks like this:

class MyClass extends Model {
  async save() {
    doSomethingPreSave();
    const newDoc = await super.save();
    doSomethingPostSave();
    return newDoc;
  }
}

aksyonov · 2017-01-17T11:08:32Z

Awesome 🎉

alexis-y · 2017-01-24T16:41:04Z

This is great, I've been looking how to do this for ages!

However I can't get it to work with discriminators/schema extension.

Here's what I have (roughly):

mongoose.model(
  class LoginBase extends Model {}, 
  new Schema({ enabled: Boolean }, { collection: 'logins', discriminatorKey: '_type' })
);

LoginBase.discriminator(
  class LocalLogin extends LoginBase {},
  new Schema({ email: String, password: String })
);

This throws Virtual path "__v" conflicts with a real path in the schema. If I disable versioning, then it's "_id" that fails. At that point it's clear either I've got something backwards, or this use case isn't supported (in which case I'd be glad to help out)

vkarpov15 · 2017-01-27T22:46:44Z

@alexis-y can you please open up a separate issue? Hard to track issues when new issues are nested underneath old ones :)

Re: #4668

malcommac · 2017-04-19T17:36:16Z

Is there a doc to see how it should work?

vkarpov15 · 2017-04-30T02:48:28Z

http://mongoosejs.com/docs/advanced_schemas.html

NickBolles · 2017-09-02T21:17:09Z

I assume this will work with typescript classes, but I don't see any mention of it. Will this work? Also as a side question, can you define the constructor in the class to extend the base Model constructor? Again it would seem so.

everythingspirals · 2018-03-26T18:36:26Z

When I use the syntax from the original post, i.e:

class UserSession extends mongoose.Model {
    static get MAX_LOGIN_ATTEMPTS() {
        return 5;
    }

    static get TOKEN_EXPIRING_PERIOD() {
        return 172800;
    }
}

mongoose.model(UserSession, schema, 'user.session');

I get the following error:

MissingSchemaError: Schema hasn't been registered for model "user.session".

However, if I revert to:

mongoose.model('user.session', schema, 'user.session');

I no longer get this error.

HRK44 · 2018-03-26T18:49:38Z

lib/model.js

+  var model;
+  if (typeof name === 'function' && name.prototype instanceof Model) {
+    model = name;
+    name = model.name;


How is the '.name' defined?

Because type of name=== 'function' and model = name model is a function. This function.prototype.name is what it's calling. So it would be "myFunction" in the below function function my function() {}

With a class this is the name of the class.

What if we want our model to have a name different than the class name, i.e if our model is name is 'my.function'?

We have to override it? i.e

class myFunction { static get name(){ return 'my.function'; } }

name is a read-only property, so you can't change it
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/name

I suggest you look at the link that vkarpov15 commented to use .LoadClass into the schema you want

benjamin-sweney · 2018-05-01T05:36:20Z

@everythingspirals problem lies in /lib/index.js:390. Specifically:

if (options.cache === false) {
    return model;
}

Removing this allow it to work as expected. Not 100% positive on submitting a pull request as I'm sure there's a deeper issue but I myself do not have time to look into it. Perhaps a contributor like @vkarpov15 could look into this?

vkarpov15 · 2018-05-02T23:30:15Z

@benjamin-halter can you provide a script that demonstrates the issue you're seeing? That'll make it much easier for me to look into this.

abettke · 2019-03-22T04:15:30Z

http://mongoosejs.com/docs/advanced_schemas.html

In regards to this in case anyone else runs into this. If you are using a model class to define document methods, use the schema.loadClass() method to correctly integrate it into the schema. Not doing so creates some weird behavior with schemas hooks being registered multiple times.

class Test extends mongoose.Model {
    ....
}
const schema = new mongoose.Schema('testschema', {
	...
})
  /** Middleware */
  .pre('save', async function () {
     ... some stuff
  })
  .loadClass(Test); // GOOD

-----------

mongoose.model(Test, schema); // BAD, pre-save middleware calls twice

kherock added 2 commits October 30, 2016 05:46

feat(schema): added loadClass for importing methods + virtuals from E…

141e81c

…S6 classes

feat(model): support passing an ES6 class to compile

27d2342

vkarpov15 reviewed Nov 5, 2016

View reviewed changes

mleanos reviewed Nov 21, 2016

View reviewed changes

vkarpov15 added this to the 4.7.0 milestone Nov 23, 2016

vkarpov15 merged commit abd58d8 into Automattic:master Nov 23, 2016

alexis-y mentioned this pull request Jan 30, 2017

Exception subclassing a Model #4942

Closed

vkarpov15 added a commit that referenced this pull request Apr 16, 2017

docs(schema): add basic docs for loadClass()

06099f8

Re: #4668

HRK44 reviewed Mar 26, 2018

View reviewed changes

vkarpov15 mentioned this pull request Apr 3, 2018

Docs for schema.loadClass() #6308

Closed

Automattic locked as resolved and limited conversation to collaborators Mar 24, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Integration for ES6 classes #4668

Integration for ES6 classes #4668

kherock commented Oct 30, 2016 •

edited

Loading

kherock commented Nov 4, 2016 •

edited

Loading

vkarpov15 Nov 5, 2016

kherock Nov 5, 2016 •

edited

Loading

kherock Nov 5, 2016

mleanos Nov 21, 2016

kherock Nov 21, 2016

mleanos Nov 21, 2016

mleanos Nov 21, 2016 •

edited

Loading

kherock Nov 21, 2016

mleanos Nov 21, 2016

mleanos Nov 21, 2016

mleanos commented Nov 21, 2016

kherock commented Nov 21, 2016

malixsys commented Nov 24, 2016

malixsys commented Nov 24, 2016

TrejGun commented Nov 25, 2016

vkarpov15 commented Nov 27, 2016

klimashkin commented Jan 6, 2017

kherock commented Jan 6, 2017 •

edited

Loading

aksyonov commented Jan 17, 2017

alexis-y commented Jan 24, 2017

vkarpov15 commented Jan 27, 2017

malcommac commented Apr 19, 2017

vkarpov15 commented Apr 30, 2017

NickBolles commented Sep 2, 2017

everythingspirals commented Mar 26, 2018 •

edited

Loading

HRK44 Mar 26, 2018

NickBolles Mar 26, 2018

everythingspirals Mar 26, 2018

NickBolles Mar 26, 2018

benjamin-sweney commented May 1, 2018 •

edited

Loading

vkarpov15 commented May 2, 2018

abettke commented Mar 22, 2019

Integration for ES6 classes #4668

Integration for ES6 classes #4668

Conversation

kherock commented Oct 30, 2016 • edited Loading

kherock commented Nov 4, 2016 • edited Loading

Choose a reason for hiding this comment

kherock Nov 5, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mleanos Nov 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mleanos commented Nov 21, 2016

kherock commented Nov 21, 2016

malixsys commented Nov 24, 2016

malixsys commented Nov 24, 2016

TrejGun commented Nov 25, 2016

vkarpov15 commented Nov 27, 2016

klimashkin commented Jan 6, 2017

kherock commented Jan 6, 2017 • edited Loading

aksyonov commented Jan 17, 2017

alexis-y commented Jan 24, 2017

vkarpov15 commented Jan 27, 2017

malcommac commented Apr 19, 2017

vkarpov15 commented Apr 30, 2017

NickBolles commented Sep 2, 2017

everythingspirals commented Mar 26, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjamin-sweney commented May 1, 2018 • edited Loading

vkarpov15 commented May 2, 2018

abettke commented Mar 22, 2019

kherock commented Oct 30, 2016 •

edited

Loading

kherock commented Nov 4, 2016 •

edited

Loading

kherock Nov 5, 2016 •

edited

Loading

mleanos Nov 21, 2016 •

edited

Loading

kherock commented Jan 6, 2017 •

edited

Loading

everythingspirals commented Mar 26, 2018 •

edited

Loading

benjamin-sweney commented May 1, 2018 •

edited

Loading