From d07d2680f3483ffef1f7f6e403c6cbfc093059e7 Mon Sep 17 00:00:00 2001 From: Alma Madsen Date: Wed, 10 Dec 2014 15:23:20 -0700 Subject: [PATCH 1/3] children should not have changes when data is set during parent instantiation --- ampersand-state.js | 4 +++- test/full.js | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/ampersand-state.js b/ampersand-state.js index 79a742e..d14e68f 100644 --- a/ampersand-state.js +++ b/ampersand-state.js @@ -201,7 +201,9 @@ _.extend(Base.prototype, BBEvents, { // and push to changes array if (hasChanged) { changes.push({prev: currentVal, val: newVal, key: attr}); - self._changed[attr] = newVal; + if (!initial) { + self._changed[attr] = newVal; + } } else { delete self._changed[attr]; } diff --git a/test/full.js b/test/full.js index e6eaab2..f70fbe5 100644 --- a/test/full.js +++ b/test/full.js @@ -940,6 +940,23 @@ test('children and collections should be instantiated', function (t) { t.end(); }); +test('children should not have changes when data is set during parent instantiation', function (t) { + var Widget = State.extend({ + props: { + title: 'string' + } + }); + var Parent = State.extend({ + children: { + widget: Widget + } + }); + var parent = new Parent({ widget: { title: 'foo' } }); + + t.equal(parent.widget.hasChanged(), false, 'should not have changed'); + t.end(); +}); + test('issue #82, child collections should not be cleared if they add data to themselves when instantiated', function (t) { var Widget = State.extend({ props: { From 7406a6e5950155a0f3eb2a5e6fcb6a96593553d9 Mon Sep 17 00:00:00 2001 From: Alma Madsen Date: Thu, 11 Dec 2014 11:15:42 -0700 Subject: [PATCH 2/3] undo previous change --- ampersand-state.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/ampersand-state.js b/ampersand-state.js index d14e68f..79a742e 100644 --- a/ampersand-state.js +++ b/ampersand-state.js @@ -201,9 +201,7 @@ _.extend(Base.prototype, BBEvents, { // and push to changes array if (hasChanged) { changes.push({prev: currentVal, val: newVal, key: attr}); - if (!initial) { - self._changed[attr] = newVal; - } + self._changed[attr] = newVal; } else { delete self._changed[attr]; } From 9c9e0a5ab8eaeffdba60060b261a67f2a8532579 Mon Sep 17 00:00:00 2001 From: Alma Madsen Date: Thu, 11 Dec 2014 11:18:07 -0700 Subject: [PATCH 3/3] more robust solution that also passes option from parent to children and collections during instantiation --- ampersand-state.js | 21 ++++++++++++++------- test/full.js | 44 ++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 54 insertions(+), 11 deletions(-) diff --git a/ampersand-state.js b/ampersand-state.js index 79a742e..a9f5a69 100644 --- a/ampersand-state.js +++ b/ampersand-state.js @@ -15,11 +15,16 @@ function Base(attrs, options) { this.parent = options.parent; this.collection = options.collection; this._keyTree = new KeyTree(); - this._initCollections(); - this._initChildren(); + this._initCollections(attrs, _.pick(options, 'parse')); + this._initChildren(attrs, _.pick(options, 'parse')); this._cache = {}; this._previousAttributes = {}; - if (attrs) this.set(attrs, _.extend({silent: true, initial: true}, options)); + if (attrs) { + // omit children and collection attributes since we instantiate + // them with attrs during _initCollections and _initChildren + attrs = _.omit(attrs, function(attr, name) { return (name in this._collections) || (name in this._children); }, this); + this.set(attrs, _.extend({silent: true, initial: true}, options)); + } this._changed = {}; if (this._derived) this._initDerived(); if (options.init !== false) this.initialize.apply(this, arguments); @@ -427,19 +432,21 @@ _.extend(Base.prototype, BBEvents, { } }, - _initCollections: function () { + _initCollections: function (attrs, options) { var coll; if (!this._collections) return; + options || (options = {}); for (coll in this._collections) { - this[coll] = new this._collections[coll](null, {parent: this}); + this[coll] = new this._collections[coll](attrs ? attrs[coll] : null, _.extend({parent: this}, options)); } }, - _initChildren: function () { + _initChildren: function (attrs, options) { var child; if (!this._children) return; + options || (options = {}); for (child in this._children) { - this[child] = new this._children[child]({}, {parent: this}); + this[child] = new this._children[child](attrs ? attrs[child] : null, _.extend({parent: this}, options)); this.listenTo(this[child], 'all', this._getEventBubblingHandler(child)); } }, diff --git a/test/full.js b/test/full.js index f70fbe5..86418cb 100644 --- a/test/full.js +++ b/test/full.js @@ -940,10 +940,16 @@ test('children and collections should be instantiated', function (t) { t.end(); }); -test('children should not have changes when data is set during parent instantiation', function (t) { +test('parent instantiation with children', function (t) { var Widget = State.extend({ props: { - title: 'string' + title: 'string', + size: 'string' + }, + parse: function(attrs) { + attrs.title = attrs.serverTitle; + delete attrs.serverTitle; + return attrs; } }); var Parent = State.extend({ @@ -951,9 +957,39 @@ test('children should not have changes when data is set during parent instantiat widget: Widget } }); - var parent = new Parent({ widget: { title: 'foo' } }); + var parent = new Parent({ widget: { serverTitle: 'foo', size: 'lg' } }, { parse: true }); + + t.equal(parent.hasChanged(), false, 'parent should not register as having changes'); + t.equal(parent.widget.hasChanged(), false, 'child should not register as having changes'); + t.equal(parent.widget.title, 'foo', 'child should be instantiated with parse option same as parent'); + t.end(); +}); + +test('parent instantiation with collections', function (t) { + var Widget = State.extend({ + props: { + title: 'string', + size: 'string' + }, + parse: function(attrs) { + attrs.title = attrs.serverTitle; + delete attrs.serverTitle; + return attrs; + } + }); + var WidgetCollection = Collection.extend({ + model: Widget + }); + var Parent = State.extend({ + collections: { + widgets: WidgetCollection + } + }); + var parent = new Parent({ widgets: [{ serverTitle: 'foo', size: 'lg' }] }, { parse: true }); - t.equal(parent.widget.hasChanged(), false, 'should not have changed'); + t.equal(parent.hasChanged(), false, 'parent should not register as having changes'); + t.equal(parent.widgets.at(0).hasChanged(), false, 'collection models should not register as having changes'); + t.equal(parent.widgets.at(0).title, 'foo', 'collection models should be instantiated with same parse option as parent'); t.end(); });