Sails: Model Instance Methods in v1.0

[magiksd]

Sails version: 1.0
Node version: 7.4.0
NPM version: 4.1.1
Operating system: Linux Debian 8

---

Hello guys,

I was reading the Sails.js v1.0 changelog by curiosity and then read the following line :

Model Instance Methods are no longer supported. This allows records returned from find queries to be plain javascript objects instead of model record instances.

The first thing I had in mind was ... why ?

Maybe I'm wrong, but I think a lot of people use model instance methods, it's very useful for interacting on model's data, and having object methods is an essential feature of OOP programming that everybody is used to work with.

Can you explain why are you removing this feature ? For me, it's like killing the ORM.

For example, I have set a middleware that inject a "req.user" containing the logged user requested from the database via session ID. After that, I can get properties of my logged user and call some useful methods like "req.user.canRead(item)" to know is my user can access a given item by checking its parents.

if (!req.user.canRead(item)) {
  return res.forbidden();
}

How will I achieve this with Sails.js v1.0 ? Do I have to transform every model instance methods to static model methods ? In my opinion, this will be a pain to use models that way, I have so many model instance methods that help me for a lot of stuff.

if (!User.canRead(req.user, Item.getParents(item))) {
  return res.forbidden();
}

I understand that, for performance reason, it could be better to return plain javascript object, but can you at least provide an option to enable/disable model instantiation for "find" results ? It would be very nice :)

[skleeschulte]

Can you explain why are you removing this feature ?

I found a (somewhat weak) explanation in the new docs: What about instance methods? I'd also like to know a bit more about the reasoning behind this...

[sgress454]

Hi @magiksd, @skleeschulte; thanks for reaching out. When we publish 1.0, we'll put out a blog post with some more explanation of some of the decisions that went into it, like this one.

For this particular case, I can say personally that in real projects we've come against the following issues with model methods, among other things:

1. The situation noted in the new docs, namely that if (for example) all you want to do is find out if a particular user canRead an item, but you don't actually need to retrieve the user, you either have to do a User.find() just to get the user instance so you can run .canRead() on it, or you end up having to create the User.canRead() class method anyway -- and the class method can be made to work in _any_ situation, by having the first argument accept either a record ID or a full record.
2. Using class methods makes for more maintainable code. It's a lot easier to scan the code and see User.canRead(), and know you're dealing with the User model, than to see theLoggedInUser.canRead() and have to check back to see exactly what kind of thing theLoggedInUser is. This is especially important when different models might implement instance methods with the same name.

We really appreciate everyone's feedback on all this, and we realize that not every decision will be popular with all Sails users. We're doing our best to make the framework accessible and easy to use for as many developers as we can!

[magiksd]

Hi @sgress454, thanks for your response.

It seems that you have a strong reason to remove model instance methods, but I still don't understand why, and in my opinion, it's a regression because it's less flexible than before and force developers to code without a feature present in all ORM.

Anyway, while returning a plain object by default, could you at least provide an option to return an instance, for example by setting a meta :

User.find({
  name: 'John'
})
.meta({
  instantiate: true
})

Alternatively, you could introduce lifecycle callbacks on find like "afterFetch" to inject methods in the object (or modifying properties).

Finally, it would be very useful to know from which model the object came from. For example, by doing myObject instanceof MyModel or, better, myObject.constructor.name.

[luislobo]

There's also the overhead of adding those methods to all your objects even though you might not use them, making queries take longer since you have to inject those methods.

Also, if you are caching your models, then you also "loose" your methods, anyway, unless you re-hydrate them manually.

I always prefer to have Services/Helpers that interact with our Plan Old Javascript Objects. And Controllers that just talk to services and provide view code (if you use server side generated views). In my case we use Angular so, not even that, our Controllers just call services.

http://stackoverflow.com/questions/20735380/sails-js-access-controller-method-from-controller-method/25171973#25171973

[greggreenhaw]

Removing instance methods is absurd. Say you wanted a quick way to merge first and last name as a fullname method. I guess the real issue is that sails does not use proper instances with prototypes but rather simple objects?

[mattsears18]

Literally just had this problem. How might I modify this model to combine the first and last name into a fullname?

// api/models/User.js

module.exports = {
  attributes: {
    first_name: {type: 'string'},
    last_name: {type: 'string'},
  }
};

[magiksd]

Why not just modify the file lib/waterline/utils/query/process-all-records.js`to replace this...

if (WLModel.customToJSON) {
  Object.defineProperty(record, 'toJSON', {
    value: WLModel.customToJSON
  });
}

...by this...

if (WLModel.methods) {
  Object.assign(record, WLModel.methods);
}

...to allow developers to define instance methods easily like this...

module.exports = {
  attributes: { ... },
  methods: {
    toJSON: () => _.pick(this, ['firstname', 'lastname']),
    getName: () => [this.firstname, this.lastname].join(' ')
  }
};

[greggreenhaw]

I'm curious why sails doesn't use the conventional prototype style for classes with inheritance
Can you even extend models in sails?

For Example:
function Mammal(name){
this.name=name;
this.offspring=[];
}
Mammal.prototype.haveABaby=function(){
var newBaby=new Mammal("Baby "+this.name);
this.offspring.push(newBaby);
return newBaby;
}
Mammal.prototype.toString=function(){
return '[Mammal "'+this.name+'"]';
}

Cat.prototype = new Mammal(); // Here's where the inheritance occurs
Cat.prototype.constructor=Cat; // Otherwise instances of Cat would have a constructor of Mammal
function Cat(name){
this.name=name;
}
Cat.prototype.toString=function(){
return '[Cat "'+this.name+'"]';
}

[sayjeyhi]

I am curious how to use this is model methods with sails version 1 ?