Add a preinitialize method to allow for true instance properties and ES6 classes

[benmccormick]

This is my attempt to allow for better support for ES6 classes through a new method, localProps.

localProps allows you to define a function (or an object), that is evaluated in the constructor for Models/Collections/Views/Routers, with the result being mixed in to each local object.

As a happy side effect this allows a declarative format for instance properties on an object (instead of using this.foo = 'bar' statements in the constructor).

This allows us to write something like

class DocumentRow extends Backbone.View {

    localProps() {
        return {
          tagName:  "li",
          className: "document-row",
          events: {
            "click .icon":          "open",
            "click .button.edit":   "openEditDialog",
            "click .button.delete": "destroy"
          }
        };
    }

    initialize() {
        this.listenTo(this.model, "change", this.render);
    }

    render() {
        //...
    }
}

instead of adding properties to the prototype separate from the class declaration or using non-standard features like decorators.

See #3560 for more background on why this is useful/necessary

[jridgewell]

I'm not sure I like this any better than the property methods approach...

[benmccormick]

@jridgewell hey Justin. I get it because I don't think its perfect/beautiful either.

This way has a bit less boilerplate than methods for everything, and works for Backbone libs/extensions that rely on properties without using _.result (admittedly they probably should be doing that)

I'd love it if somebody came along and tossed out a better solution for a clean syntax with pure ES6 classes. If this PR is a start for something better then that's a win in my mind. But if there isn't something better, I think this is an improvement.

[akre54]

I'm not sure I like this any better than the property methods approach...

Agreed. And this has the added disadvantage of not sharing objects across prototypes. An app with a lot of objects is going to balloon in memory very quickly.

I've been declaring ES2016 classes broken until they fix prototypes and super in the constructor. Backbone's approach (and the long history of others that came before) is much more flexible and sensible.

[benmccormick]

An app with a lot of objects is going to balloon in memory very quickly.

I think this is overstated How much additional memory overhead is this really going to add in a normal case, if this is for configuration props?. Again though, is there a better way?

I've been declaring ES2016 classes broken until they fix prototypes and super in the constructor. Backbone's approach (and the long history of others that came before) is much more flexible and sensible.

I understand this mindset. But, I'm in favor of allowing devs to use the spec'd classes here for the sake of standardization. A common class standard allows better tooling (allow things like http://ternjs.net/ to better interpret Backbone constructs for instance) as well as lowering the learning curve when moving between Backbone/Angular 2/ React / etc. So while I'm very willing to accept that this isn't the best way of doing this (I hope somebody has something better), I don't like the idea of giving up on allowing ES6 classes as a usable option.

[jridgewell]

This way has a bit less boilerplate than methods for everything, and works for Backbone libs/extensions that rely on properties without using _.result (admittedly they probably should be doing that)

It's not much boilerplate. And for things like Model#defaults and View#attributes it'll actually save developers from themselves.

Agreed. And this has the added disadvantage of not sharing objects across prototypes. An app with a lot of objects is going to balloon in memory very quickly.

I'm wary about this argument. Yes they'll be bigger, but I don't think it'll cause huge problems.

So while I'm very willing to accept that this isn't the best way of doing this (I hope somebody has something better), I don't like the idea of giving up on allowing ES6 classes as a usable option.

I'm actually coming around to this. If we update the code to pass in options, I'd be in favor. We can use this to as the sanctioned method to attach properties to the instance before the other methods are called on creation. Kind of like a #preinitialize.

I know there are issues requesting this, and that can be solved with this, but my issue searching skills are failing me. Something along the lines of changing the tagName based on input options.

Also, it'd help since ~75% of my own code does _.extend(this, _.pick(options, ['option1', ...]) in my constructor or #initialize (depending on where in the creation process I need it).

---

So, add options, and 👍.

[akre54]

We shouldn't need result for this, no? There's never a case localProps would be an object over just sticking them on the prototype, yeah?

[jridgewell]

I think forcing it to be a method is the right choice.

[benmccormick]

My thought was around the secondary use case (in my mind) of adding properties to the instance. I was thinking a user might want to do that with a few static properties (using the traditional Backbone class style). But I'm realizing now that the reference would be shared anyway and that would undermine the purpose. So I'm on board with that.

[blikblum]

How about change the name from localProps to instanceProps ?

[benmccormick]

@jridgewell just making sure I'm understanding you correctly, you want the options passed in to the constructor passed to the localProps/instanceProps function?

@blikblum I'd be ok with instanceProps. It would also be an opportunity to clarify the current docs. Right now the model extend docs start with this:

To create a Model class of your own, you extend Backbone.Model and provide instance properties, as well as optional classProperties to be attached directly to the constructor function.

It's referring to the fact that the first argument to extend is added as instance properties on the prototype, but thats super confusing I think to most people. Naming this instanceProps and clarifying that extends is mixing data onto the prototype and not an instance would likely be helpful.

[jridgewell]

just making sure I'm understanding you correctly, you want the options passed in to the constructor passed to the localProps/instanceProps function?

Yup.

It would also be an opportunity to clarify the current docs

Separate PR?

I'd be ok with instanceProps

I think this is going in the wrong direction. Defining a more generic #preinitialize would cover this use case and avoid semantic arguments about what it should and shouldn't do. That'll solve issues like #3620, #3685, and #3060 (comment).

var Model = function(data, options) {
  var attrs = attributes || {};
  options || (options = {});
  this.preinitialize.apply(this, arguments);
  // ...
  this.initialize.apply(this, arguments);
};

[benmccormick]

@jridgewell I'm on board with that. Generalizing it makes sense to me. Thanks for the wider context.

Ok I'm going to edit the PR to rename this to preinitialize across the board, make it method only, and change it to applying with the constructor options as opposed to extending the prototype.

So my example above will look like:

class DocumentRow extends Backbone.View {

    preinitialize() {
        _.extend(this, {
          tagName:  "li",
          className: "document-row",
          events: {
            "click .icon":          "open",
            "click .button.edit":   "openEditDialog",
            "click .button.delete": "destroy"
          }
        });
    }

    initialize() {
        this.listenTo(this.model, "change", this.render);
    }

    render() {
        //...
    }
}

Which is more explicit for readers anyway. I like it.

[braddunbar]

So, I don't quite understand how this is different than the following.

class MyView extends Backbone.View {
  constructor (attributes, options) {
    _.extend(this, …)
    super(attributes, options)
    this.listenTo(…)
  }
}

I find initialize to be problematic as a replacement for calling super, with or without class sugar. Adding another lifecycle method as an alternative to calling methods when necessary seems like a step in the wrong direction to me.

[jridgewell]

So, I don't quite understand how this is different than the following.

You can't use this before super() in ES6 classes. 😞

class MyView extends Backbone.View {
  constructor (attributes, options) {
    _.extend(this, …) // throws error
    super(attributes, options)
    this.listenTo(…)
  }
}

class MyView extends Backbone.View {
  constructor (attributes, options) {
    super(attributes, options)
    _.extend(this, …) // too late, dom element has already been created and events bound.
    this.listenTo(…)
  }
}

I find initialize to be problematic as a replacement for calling super

I actually agree for the current case: #initialize should be deprecated when ES6 classes become the norm. But for this case, we can't do a "before constructor" (#preinitialize), only an "after" (#initialize).

[braddunbar]

You can't use this before super() in ES6 classes.

Well that's…surprising. I don't understand why since one can already do Super.call(this), but thanks for telling me! 😸

In that case we're left with getters and setters I suppose? I haven't seen them mentioned yet in this thread or #3560.

class MyView extends Backbone.View {
  get tagName () { return '…' }
  …
}

I understand it's preferable to declare these things directly on the prototype (I would rather that as well) but I don't know that preinitialize is an improvement in that regard.

[jridgewell]

In that case we're left with getters and setters I suppose? I haven't seen them mentioned yet in this thread or #3560.

#3560 (comment) 😛

But that won't work for runtime options:

class View extends Backbone.View {
  constructor(options) {
    super(options);
    _.extend(this, _.pick(options, 'type'));
  }

  tagName() {
    return this.type === 'emote' ? 'em' : 'span';
  }
}

Yes, you could pass in tagName in options, but then your view's logic isn't self contained.

[braddunbar]

Thanks @jridgewell! (That's a long thread.) I guess I'll leave this alone then, as I don't have any better ideas. ¯_(ツ)_/¯

[jridgewell]

I was being cheeky, sorry.

I guess I'll leave this alone then, as I don't have any better ideas.

I don't mean to force my way here, I just think it's a pretty simple solution and it'll work well with code I usually write.

[benmccormick]

@jridgewell well I proposed an idea that I wasn't completely happy with in the hopes that it would produce a better idea, and I feel like that happened, so its all good from my POV

[jridgewell]

Random newline?

[benmccormick]

@jridgewell oops. Fixed.

[akre54]

(replying to @jridgewell) My objection is that with this pull classes now have three places to put create-time logic: the constructor, preinitialize, and initialize. This feels like a hacky solution that is a failure of ES6 classes, and adds extra confusion for those who just want to use normal extend.

[benmccormick]

@akre54 theres default constructor behavior, and hooks to run code before and after the default behavior. Or you can override the default behavior.

Thats a reasonable and coherent set of options, and its a common pattern to provide a component that has life cycle functions/events, and hooks to add behavior before or after those events.

Marionette Views allow you to override render, or put stuff in onBeforeRender or onRender.

React has similar before and after hooks for the life cycle events of their components (componentWillUpdate/componentDidUpdate).

Ember does similar stuff, components have willInsertElement and didInsertElement events.

I think people will be able to grasp the difference here if it is documented.

[jridgewell]

My objection is that with this pull classes now have three places to put create-time logic: the constructor, preinitialize, and initialize.

With this, there's never a reason to override constructor. Anything you would put there, put in either preinit or init.

This feels like a hacky solution that is a failure of ES6 classes, and adds extra confusion for those who just want to use normal extend.

I'm not happy with the ES6 class spec either, but I don't think that should stop us from adding support for it. And this solves issues like #3060 (comment).

[megawac]

After thinking about it some more, I like preinitialize a lot as a way to avoid having to manage constructor call order.

Can you add a couple tests @benmccormick

[benmccormick]

@megawac Tests added, and passing. Sorry for the delay, I missed your original message somehow.

[megawac]

@jridgewell @akre54 I'm very happy with this PR. Any more criticisms or is it good to merge? When merged can we try to get a 1.4 out asap?

[jridgewell]

LGTM, let's merge at the end of the day if no one chimes in.

[megawac]

[akre54]

Now that static property support has landed in Chrome, do we need this anymore?

[jridgewell]

Yup. Static properties are set after the super() call, which means our view has already been created.

[akre54]

Right right. I'm referencing the comment above it. (By static properties I mean the static keyword, not property initializers. Those haven't, to my knowledge, landed in the spec yet).

Example.

[jridgewell]

Ohh. I'm kinda partial to this solution over the instOrStaticVar. @megawac?

[benmccormick]

@akre54 to be clear, in your example, the static events part isn't currently part of the spec right? Thats combining the static keyword with the property initializer proposal.

If that is right, I'm not sure what has changed here? You can use methods to define the various things here with or without the static keyword.

[typhonrt]

Yes the class properties / stage-2 proposal. Check out the example @akre54 posted with just the ES2015 preset; things fail. Now click the presets section to the right of the evaluate checkbox and add stage-2.

[benmccormick]

@typhonrt I don't think we're saying different things.

If I understand correctly static methods are currently part of the spec, are supported in the latest version of every major environment and will be supported everywhere eventually.

As far as I can tell (and it seems you agree) static properties are a proposal that may or may not eventually end up in the spec, and can currently be used through Babel.

I understand how static properties potentially solve the problem of this PR. I don't understand how static methods do, and that was what I was trying to ask about

Edit: I initially read that as you trying to disagree with me saying that static events wasn't part of the spec, looks like you were actually just trying to answer my first question by pointing to the proposal. Reading comprehension fail. Thanks!

[typhonrt]

Indeed, though at stage-2 / draft it's quite likely to proceed onward and is a fairly popular feature. An article to peruse on ES6+ with React; see the "property initializers" / "arrow functions" sections.

[davidmpaz]

Hi,

is there any ETA to get this PR via #4028 merged? This is more than one year old already and from what I read from related threads and posts, most people agreed on while not the perfect solution provides necessary support for current specifications.

I would like to start using Backbone since it fits better than a full bloated framework like Angular or Vue.js but the fact that I can not have members (properties) initialized in my constructors, because of problems stated already, is hindering me severely.

Could you please decide on adding it?

Thanks in advance
David

[benmccormick]

@davidmpaz I've resigned myself to this possibly never getting released. I don't think Backbone is getting significant attention from any of the maintainers these days (see recent issue activity). I'd recommend working off a fork if this is super important to you, or if you just need the data modeling, using a different data library like MobX.

[davidmpaz]

I thought on introduce it bit by bit to have time to check how is accepted, starting with Views using it also with TypeScript by the way. Just wanted to have goodies (bit more structure, events, jQuery scoped access to DOM) from Backbone.View.

But having your comments in mind as well as issues activity I will probably quit, I can not risk on a fork for this. I really don't like one man orchestra shows :) and I also don't want to rely on decorators as long as they don't became standard, which is the alternative I like the most for this situation.

I will discuss this with colleagues and see what we can do. I was strongly hoping to not end re-implementing wheels by my self ;) Backbone was looking like a perfect match for our needs.

Thanks for the hints