From a9f7fd97f8e7b7a418fb6f3c35be833cc082d2a6 Mon Sep 17 00:00:00 2001 From: Mike Macaulay Date: Sat, 29 Nov 2014 12:20:16 -0600 Subject: [PATCH 1/5] implementation of onSet --- ampersand-state.js | 32 ++++++++++++++++++++------------ test/full.js | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/ampersand-state.js b/ampersand-state.js index 67cf0f9..fe75a0a 100644 --- a/ampersand-state.js +++ b/ampersand-state.js @@ -20,6 +20,7 @@ var Events = require('ampersand-events'); var KeyTree = require('key-tree-store'); var arrayNext = require('array-next'); var changeRE = /^change:/; +var noop = function () {}; function Base(attrs, options) { options || (options = {}); @@ -117,7 +118,7 @@ assign(Base.prototype, Events, { var self = this; var extraProperties = this.extraProperties; var changing, changes, newType, newVal, def, cast, err, attr, - attrs, dataType, silent, unset, currentVal, initial, hasChanged, isEqual; + attrs, dataType, silent, unset, currentVal, initial, hasChanged, isEqual, onSet; // Handle both `"key", value` and `{key: value}` -style arguments. if (isObject(key) || key === null) { @@ -171,11 +172,12 @@ assign(Base.prototype, Events, { } isEqual = this._getCompareForType(def.type); + onSet = this._getOnSetForType(def.type); dataType = this._dataTypes[def.type]; // check type if we have one if (dataType && dataType.set) { - cast = dataType.set(newVal); + cast = dataType.set.call(this, newVal, options); newVal = cast.val; newType = cast.type; } @@ -217,6 +219,7 @@ assign(Base.prototype, Events, { if (hasChanged) { changes.push({prev: currentVal, val: newVal, key: attr}); self._changed[attr] = newVal; + onSet(newVal, currentVal, attr); } else { delete self._changed[attr]; } @@ -353,6 +356,12 @@ assign(Base.prototype, Events, { return _isEqual; // if no compare function is defined, use _.isEqual }, + _getOnSetForType : function(type){ + var dataType = this._dataTypes[type]; + if (dataType && dataType.onSet) return bind(dataType.onSet, this); + return noop; + }, + // Run validation against the next complete set of model attributes, // returning `true` if all is well. Otherwise, fire an `"invalid"` event. _validate: function (attrs, options) { @@ -699,22 +708,21 @@ var dataTypes = { }; } }, - compare: function (currentVal, newVal, attributeName) { - var isSame = currentVal === newVal; + compare: function (currentVal, newVal) { + return currentVal === newVal; + }, + onSet : function(newVal, currentVal, attributeName){ // if this has changed we want to also handle // event propagation - if (!isSame) { - if (currentVal) { - this.stopListening(currentVal); - } - if (newVal != null) { - this.listenTo(newVal, 'all', this._getEventBubblingHandler(attributeName)); - } + if (currentVal) { + this.stopListening(currentVal); } - return isSame; + if (newVal != null) { + this.listenTo(newVal, 'all', this._getEventBubblingHandler(attributeName)); + } } } }; diff --git a/test/full.js b/test/full.js index e021b63..b704234 100644 --- a/test/full.js +++ b/test/full.js @@ -1814,3 +1814,38 @@ test('toJSON should serialize customType props - issue #197', function(t) { t.end(); }); + +test("#112 - should not set up events on child state if setOnce check fails", function(t){ + var Person = State.extend({ + props : { + birthday : { + type : 'state', + setOnce : true + } + } + }); + var Birthday = State.extend({ + props : { + day : 'date' + } + }); + + var p = new Person(); + var bday = new Birthday({day : new Date()}); + p.once('change:birthday', function() { + t.pass('birthday can change once'); + }); + p.birthday = bday; + var newBday = new Birthday({day : new Date()}); + t.throws(function() { + p.birthday = newBday; + }, TypeError, 'Throws error on change of setOnce'); + + p.on('change:birthday.day', function() { + t.fail('should not trigger change event on old one'); + }); + + newBday.day = new Date(1); + + t.end(); +}); From 35405f9816eb2a3d8fc5570988a09d09d59c646f Mon Sep 17 00:00:00 2001 From: John Chesley Date: Wed, 21 Oct 2015 23:15:43 -0400 Subject: [PATCH 2/5] call onSet callback after initializing default value --- ampersand-state.js | 8 +++++--- test/full.js | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 3 deletions(-) diff --git a/ampersand-state.js b/ampersand-state.js index fe75a0a..8bf6a1f 100644 --- a/ampersand-state.js +++ b/ampersand-state.js @@ -580,15 +580,17 @@ function createPropertyDefinition(object, name, desc, isSession) { } var value = this._values[name]; var typeDef = this._dataTypes[def.type]; + var onSet = this._getOnSetForType(def.type); if (typeof value !== 'undefined') { if (typeDef && typeDef.get) { value = typeDef.get(value); } return value; } - value = result(def, 'default'); - this._values[name] = value; - return value; + var defaultValue = result(def, 'default'); + this._values[name] = defaultValue; + onSet(defaultValue, value, name); + return defaultValue; } }); diff --git a/test/full.js b/test/full.js index b704234..a3d598c 100644 --- a/test/full.js +++ b/test/full.js @@ -1849,3 +1849,39 @@ test("#112 - should not set up events on child state if setOnce check fails", fu t.end(); }); + +test('#112 - onSet should be called for default values', function (t) { + var Person = State.extend({ + dataTypes: { + 'custom-type': { + set: function (newVal) { + return { + type: 'custom-type', + val: newVal + }; + }, + onSet: function (newVal, curVal, name) { + t.equal(newVal.value, 100, 'should get the default value as newVal'); + t.equal(curVal, undefined, 'should get undefined as current value'); + t.equal(name, 'strength', 'should get the attribute name'); + t.pass('onSet was called'); + } + } + }, + props: { + strength: { + type: 'custom-type', + default: function () { + t.pass('default function should be called'); + return { + value: 100 + }; + } + } + } + }); + + t.plan(6); + var p = new Person(); + t.equal(p.strength.value, 100); +}); From f0fe21b1c307281b39eac237e7fe202a78caebd4 Mon Sep 17 00:00:00 2001 From: John Chesley Date: Wed, 21 Oct 2015 23:27:14 -0400 Subject: [PATCH 3/5] rename onSet to onChange and only call when value changes Since the `dataType.set` function is called on every set, the name `dataType.onSet` was a little confusing. Since the function is only called when the value changes, `dataType.onChange` seemed more appropriate. --- ampersand-state.js | 22 ++++++++++++---------- test/full.js | 6 +++--- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/ampersand-state.js b/ampersand-state.js index 8bf6a1f..a2dd10b 100644 --- a/ampersand-state.js +++ b/ampersand-state.js @@ -118,7 +118,7 @@ assign(Base.prototype, Events, { var self = this; var extraProperties = this.extraProperties; var changing, changes, newType, newVal, def, cast, err, attr, - attrs, dataType, silent, unset, currentVal, initial, hasChanged, isEqual, onSet; + attrs, dataType, silent, unset, currentVal, initial, hasChanged, isEqual, onChange; // Handle both `"key", value` and `{key: value}` -style arguments. if (isObject(key) || key === null) { @@ -172,7 +172,7 @@ assign(Base.prototype, Events, { } isEqual = this._getCompareForType(def.type); - onSet = this._getOnSetForType(def.type); + onChange = this._getOnChangeForType(def.type); dataType = this._dataTypes[def.type]; // check type if we have one @@ -219,7 +219,7 @@ assign(Base.prototype, Events, { if (hasChanged) { changes.push({prev: currentVal, val: newVal, key: attr}); self._changed[attr] = newVal; - onSet(newVal, currentVal, attr); + onChange(newVal, currentVal, attr); } else { delete self._changed[attr]; } @@ -356,9 +356,9 @@ assign(Base.prototype, Events, { return _isEqual; // if no compare function is defined, use _.isEqual }, - _getOnSetForType : function(type){ + _getOnChangeForType : function(type){ var dataType = this._dataTypes[type]; - if (dataType && dataType.onSet) return bind(dataType.onSet, this); + if (dataType && dataType.onChange) return bind(dataType.onChange, this); return noop; }, @@ -580,7 +580,6 @@ function createPropertyDefinition(object, name, desc, isSession) { } var value = this._values[name]; var typeDef = this._dataTypes[def.type]; - var onSet = this._getOnSetForType(def.type); if (typeof value !== 'undefined') { if (typeDef && typeDef.get) { value = typeDef.get(value); @@ -589,7 +588,10 @@ function createPropertyDefinition(object, name, desc, isSession) { } var defaultValue = result(def, 'default'); this._values[name] = defaultValue; - onSet(defaultValue, value, name); + if (typeof defaultValue !== 'undefined') { + var onChange = this._getOnChangeForType(def.type); + onChange(defaultValue, value, name); + } return defaultValue; } }); @@ -714,12 +716,12 @@ var dataTypes = { return currentVal === newVal; }, - onSet : function(newVal, currentVal, attributeName){ + onChange : function(newVal, previousVal, attributeName){ // if this has changed we want to also handle // event propagation - if (currentVal) { - this.stopListening(currentVal); + if (previousVal) { + this.stopListening(previousVal); } if (newVal != null) { diff --git a/test/full.js b/test/full.js index a3d598c..9f2b8b0 100644 --- a/test/full.js +++ b/test/full.js @@ -1850,7 +1850,7 @@ test("#112 - should not set up events on child state if setOnce check fails", fu t.end(); }); -test('#112 - onSet should be called for default values', function (t) { +test('#112 - onChange should be called for default values', function (t) { var Person = State.extend({ dataTypes: { 'custom-type': { @@ -1860,11 +1860,11 @@ test('#112 - onSet should be called for default values', function (t) { val: newVal }; }, - onSet: function (newVal, curVal, name) { + onChange: function (newVal, curVal, name) { t.equal(newVal.value, 100, 'should get the default value as newVal'); t.equal(curVal, undefined, 'should get undefined as current value'); t.equal(name, 'strength', 'should get the attribute name'); - t.pass('onSet was called'); + t.pass('onChange was called'); } } }, From 2466cee2d7f1b0da1c0f72dbd0a33e8700e43544 Mon Sep 17 00:00:00 2001 From: John Chesley Date: Wed, 21 Oct 2015 23:33:53 -0400 Subject: [PATCH 4/5] add onChange to README.md --- README.md | 1 + 1 file changed, 1 insertion(+) diff --git a/README.md b/README.md index 4b1a9cf..cb55a2b 100644 --- a/README.md +++ b/README.md @@ -221,6 +221,7 @@ To define a type, you generally will provide an object with 4 member functions ( * `set : function(newVal){}; returns {type : type, val : newVal};`: Called on every set. Should return an object with two members: `val` and `type`. If the `type` value does not equal the name of the dataType you defined, a `TypeError` should be thrown. * `compare : function(currentVal, newVal, attributeName){}; returns boolean`: Called on every `set`. Should return `true` if `oldVal` and `newVal` are equal. Non-equal values will eventually trigger `change` events, unless the state's `set` (not the dataTypes's!) is called with the option `{silent : true}`. +* `onChange : function (value, previousValue, attributeName){};`: Called after the value changes. Useful for automatically setting up or tearing down listeners on properties. * `get : function(val){} returns val;`: Overrides the default getter of this type. Useful if you want to make defensive copies. For example, the `date` dataType returns a clone of the internally saved `date` to keep the internal state consistent. * `default : function(){} returns val;`: Returns the default value for this type. From 89b01fd26946923fbf87613abb92c74a832bff1b Mon Sep 17 00:00:00 2001 From: John Chesley Date: Thu, 22 Oct 2015 15:24:05 -0400 Subject: [PATCH 5/5] don't bind this context when calling dataType.set --- ampersand-state.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ampersand-state.js b/ampersand-state.js index a2dd10b..33a9ae6 100644 --- a/ampersand-state.js +++ b/ampersand-state.js @@ -177,7 +177,7 @@ assign(Base.prototype, Events, { // check type if we have one if (dataType && dataType.set) { - cast = dataType.set.call(this, newVal, options); + cast = dataType.set(newVal); newVal = cast.val; newType = cast.type; }