Add getter/setter support to extend method

[jamiebuilds]

Since getters/setters are supported IE9+ and will be much more prevalent with ES6 it'd be nice if Backbone's extend method supported them.

var Person = Backbone.Model.extend({
  get fullName() {
    return this.get('firstName') + ' ' + this.get('lastName');
  }
});

var elonMusk = new Person({
  firstName: 'Elon', 
  lastName: 'Musk'
});

console.log(person.fullName); // >> Elon Musk

Right now this would result in undefined is not a function because using _.extend is calling the getter.

[akre54]

Is there any reason that couldn't be better written as person.fullName() so you know it's a function call?

Using getters and setters are for the most part a bad idea in JS, and they go against one of the core ideas of Backbone Models. I doubt you'll see support added anytime soon.

[jamiebuilds]

More than anything this would be partially aligned with the new class syntax. There are no property initializers in the class syntax or even any spec for them yet, so interop will be an issue.

they go against one of the core ideas of Backbone Models.

What core idea is that?

Also getters are a partial solution to an existing problem in Backbone.Collection with model.idAttribute.

[jamiebuilds]

Spent a few minutes trying to make a jsperf with different solutions: http://jsperf.com/backbone-extend-with-define-property

[akre54]

What core idea is that?

The idea that setting and retrieving a property on an object should be transparent and logical to reason about. You should always know that obj.prop = val never does anything other than setting that property value (ditto for retrieval), while we should use functions to handle things like computed properties and change events. That's the main reason for the get and set wrapper methods around attributes in the first place.

Also getters are a partial solution to an existing problem in Backbone.Collection with model.idAttribute.

Any reason the current _.result wrapper doesn't work for this? idAttribute can either be a primitive or a function. This is also being fixed with modelId.

[jamiebuilds]

Sorry I should've linked to the issue I was speaking about: https://github.com/jashkenas/backbone/issues/3408

I'm bringing this up mainly because Backbone's class syntax is largely already compatible with the ES class syntax, and it seems only reasonable they be compatible considering the added support for coffeescript.

[akre54]

CoffeeScript doesn't support setters and getters largely for the same reasons (look into issues there for some background, starting with https://github.com/jashkenas/coffeescript/pull/2902). They shouldn't be a roadblock to supporting Backbone objects as classes.

Sorry I should've linked to the issue I was speaking about: #3408

Remind me? I'm straining to see the connection off the bat.

Besides, any changes to extend would have to be done in _.extend, not here. Closing as a wontfix.

[jamiebuilds]

CoffeeScript doesn't support setters and getters largely for the same reasons

I wasn't speaking about getters/setters, I was speaking about how Backbone added __super__ for interop with CoffeeScript, and how it only makes sense for it to support ES6 classes in the same manner.

Remind me? I'm straining to see the connection off the bat.

Collection.extend({
  model: function() { return Model }
});
//
collection.modelId(model); // 'id' regardless if that's correct or not.

Collection.extend({
  get model() { return Model }
});
//
collection.modelId(model); // correct for *some* of the common cases.

Besides, any changes to extend would have to be done in _.extend, not here. Closing as a wontfix.

That's definitely not the right place for this change, if anything it would be in the new _.assign method. But it makes much more sense for it to be supported here IMO.

Also, before this issue gets closed and forgotten about I would like to address why it's okay for Backbone's extend to have the interop issues with ES classes but not CoffeeScript classes.

[akre54]

Well seeing as IE Tech Preview is the only browser with even remotely decent support of class (even Traceur and 6to5 don't yet convert extends), we're a ways off from this even being necessary. That aside Backbone isn't going to go out of its way to break backwards compatibility just to support a feature that isn't a great idea to begin with.

[jamiebuilds]

even Traceur and 6to5 don't yet convert extends

They both do actually, they just require support for __proto__ for static properties which is widely supported.

Also there is no reason backwords-compatibility needs to be broken, it could even be made faster if it selectively used Object.setPrototypeOf where available.

[jashkenas]

@thejameskyle Want to turn this into a PR instead of an issue? It's worth looking at.

[jamiebuilds]

Sure, I'll do that

[nicolas-zozol]

Is it on the way ? Have you a gist or something about it ? I'd like to take a shot.