Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dispense with View#options merging. #2461

Merged
merged 1 commit into from
Apr 9, 2013
Merged

Dispense with View#options merging. #2461

merged 1 commit into from
Apr 9, 2013

Conversation

braddunbar
Copy link
Collaborator

As discussed in #2458, using defaults is unnecessary when attaching options directly to new instances.

For example, this code

var View = Backbone.View.extend({
  options: {
    foo: 15,
    bar: false
  },
  initialize: function(options) {
    _.extend(this, _.pick(options, 'foo', 'bar'));
  }
});

is better written as and functionally equivalent to

var View = Backbone.View.extend({
  foo: 15,
  bar: false,
  initialize: function(options) {
    _.extend(this, _.pick(options, 'foo', 'bar'));
  }
});

With that in mind, there is really no need for merging options with View#options.

@gsamokovarov
Copy link
Contributor

:shipit:

@tgriesser
Copy link
Collaborator

Yeah, I like this change - objects or arrays directly on the prototype should generally be discouraged anyway.

@joeybaker
Copy link

It seems like the reason for dropping _configure and views not automatically assigning passed in options to this.options was to make view instantiation behave in the same way as model and collection instantiation?

If I have that correct, the desire for a consistent API makes a lot of sense. However, I think there are two points that are worth bringing up that weren't considered:

  1. _configure This is actually a very convenient "pre-initialization hook" that's now gone. I've used it to override the default viewOptions in a "base view" object which allows me to not override initalize – giving views based on this "base view" a very backbone-esque API. With the change, the only option left is to force users of this "base view", to use a custom method (e.g. init).

    suggestion: re-implement the _config method, even if it's just a no-op by default.

  2. views are different When creating a model or a collection, they infrequently need to communicate either additional options or pass additional configuration off to other models or collections. On the other hand, views – which are constantly nested – frequently have this problem. Creating this.options from the constructor's options argument is a great default that does not harm to those that don't need it, and hugely benefits a common use case.

    suggestion: re-implement the _config method, so that users can easily re-add this default to all their views at the same time if they want to.

I'm happy to create a pull request that does this.

@eth0lo
Copy link

eth0lo commented Oct 12, 2013

The init method you mention is not need it at all; problem is that your base view is overwriting the initialize method, instead I would suggest you to rename it as constructor and do all your initialization there.

Also that way you can revert the this.options = options; if you really need it.

Don't forget to add at the end of your constructor Backbone.View.prototype.constructor.apply(this, args); so it initialize the Backbone.View as usual.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants