Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Ensure options object is cloned when used, instead of attached directly. #425

Merged
merged 1 commit into from

2 participants

@STRML
Collaborator

This prevents modifications on the passed-in options object from changing options on the view.

Without it, the attached test case will fail, leading to some pretty bizarre and hard-to-find bugs.

@tbranyen
Owner

Build is failing because of #426

backbone.layoutmanager.js
@@ -732,7 +732,7 @@ var LayoutManager = Backbone.View.extend({
// Configure a View to work with the LayoutManager plugin.
setupView: function(views, options) {
// Don't break the options object (passed into Backbone.View#initialize).
- options = options || {};
+ options = _.clone(options) || {};
@tbranyen Owner

Maybe: _.extend({}, options); as an added benefit this would ensure options is always an object.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@STRML STRML Ensure options object is cloned when used, instead of attached directly.
This prevents modifications on the passed-in options object from changing
options on the view.
0f12148
@tbranyen
Owner

Looks good to me. Thanks!

@tbranyen tbranyen merged commit a5f865d into tbranyen:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Feb 28, 2014
  1. @STRML

    Ensure options object is cloned when used, instead of attached directly.

    STRML authored
    This prevents modifications on the passed-in options object from changing
    options on the view.
This page is out of date. Refresh to see the latest.
Showing with 12 additions and 2 deletions.
  1. +3 −2 backbone.layoutmanager.js
  2. +9 −0 test/spec/views.js
View
5 backbone.layoutmanager.js
@@ -731,8 +731,9 @@ var LayoutManager = Backbone.View.extend({
// Configure a View to work with the LayoutManager plugin.
setupView: function(views, options) {
- // Don't break the options object (passed into Backbone.View#initialize).
- options = options || {};
+ // Ensure that options is always an object, and clone it so that
+ // changes to the original object don't screw up this view.
+ options = _.extend({}, options);
// Set up all Views passed.
_.each(aConcat.call([], views), function(view) {
View
9 test/spec/views.js
@@ -2308,3 +2308,12 @@ test("not attached even if already rendered", 1, function() {
ok(!view.contains(layout.el, view.el), "View should not exist inside Layout");
});
+
+test("Modifications to options after initialization should not modify a view", 1, function() {
+ var options = {
+ option: "value"
+ };
+ var layout = new Backbone.Layout(options);
+ options.option = "changedValue";
+ equal(layout.options.option, "value");
+});
Something went wrong with that request. Please try again.