backbone icon indicating copy to clipboard operation
backbone copied to clipboard

Support ES6 method definitions for constructors

Open sebamarynissen opened this issue 5 years ago • 3 comments

Using ES6, it becomes possible to extend Backbone objects using

const MyModel = Backbone.Model.extend({
  foo() {
    return 'bar';
  }
});

This syntax is a nice shortcut for the more old-fashioned

const MyModel = Backbone.Model.extend({
  foo: function() {
    return 'bar';
  }
});

The problem is though that if you want to specify a custom constructor

const MyModel = Backbone.Model.extend({
  constructor() {
    Backbone.Model.apply(this, arguments);
  }
});

it turns out that

let model = new MyModel();

throws an Uncaught TypeError: MyModel is not a constructor.

The reason behind this behavior is that the ES6 method definition name() is not completely equivalent to the old name: function() {} because the ES6 methods are labelled internally as non-constructable.

A way to solve this is to change the extend method from

if (protoProps && _.has(protoProps, 'constructor')) {
  child = protoProps.constructor;
} else {
  child = function(){ return parent.apply(this, arguments); };
}

to

if (protoProps && _.has(protoProps, 'constructor')) {
  var ctor = protoProps.constructor;
  child = function(){ return ctor.apply(this, arguments); };
} else {
  child = function(){ return parent.apply(this, arguments); };
}

There is one minor drawback to this approach which is that if someone ever did something like

const proto = {
  constructor: function() {}
};
const Model = Backbone.Model.extend(proto);
if (Model === proto.constructor) { ... }

this would no longer work. I tried to find whether it is possible to check whether a method is constructable or not in which case something could be done like

if (protoProps && _.has(protoProps, 'constructor')) {
  var ctor = protoProps.constructor;
  if (!isConstructable(ctor)) {
    child = function(){ return ctor.apply(this, arguments); };
  } else {
    child = ctor;
  }
} else {
  child = function(){ return parent.apply(this, arguments); };
}

but it seems that there's no elegant way to check this.

Hence it's a trade-off between 100% backwards compatability for a rather rare edge case, or to be able to get rid of the requirement to specify the constructor as a non-ES6 method definition, but this leads to some awkward code like

const Model = Backbone.Model.extend({
  constructor: function() { ... },
  method() { ... }
});

I would love to hear some thoughts on this subject. I'd be willing to create a pull request if required.

sebamarynissen avatar May 03 '19 08:05 sebamarynissen

i have to say, that babel converts constructor() to constructor: function() also, microsoft edge understands this correctly constructor() and there is no errors if you are in edge check thisout: https://codepen.io/dimatabu/pen/ZZgdxj?editors=0010

personally, i am always using babel and faced with this issue only when i create something in test purposes and i am always treat this as bad es6 implementation in browser

taburetkin avatar May 03 '19 10:05 taburetkin

I know that using Babel solves the issue, but still I think that there's a place for this, because:

  • When using platforms like electron, you don't need to use babel because you know what version of node you're running and hence there's no need to transpile ES6 functionality.
  • I don't really know the ES spec in detail, but if I'm not mistaken the fact that object methods are not constructable is in the spec, which only means that Edge doesn't follow the spec appropriately.

sebamarynissen avatar May 03 '19 11:05 sebamarynissen

Related to #3560 and #4245.

jgonggrijp avatar Jan 05 '22 03:01 jgonggrijp