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

ability to specify a type for derived properties #116

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 46 additions & 15 deletions ampersand-state.js
Original file line number Diff line number Diff line change
Expand Up @@ -405,27 +405,44 @@ assign(Base.prototype, Events, {
_initDerived: function () {
var self = this;

forEach(this._derived, function (value, name) {
var def = self._derived[name];
forEach(this._derived, function (def, name) {
def.deps = def.depList;

var update = function (options) {
options = options || {};

var update = function () {
var newVal = def.fn.call(self);

if (self._cache[name] !== newVal || !def.cache) {
var isEqual = self._getCompareForType(def.type);
var dataType = def.type && self._dataTypes[def.type];
var currentVal = self._cache[name];
var hasChanged = !isEqual(currentVal, newVal, name);

if (hasChanged || !def.cache) {
if (def.cache) {
self._previousAttributes[name] = self._cache[name];
self._previousAttributes[name] = currentVal;
}
// cast newVal if there is a type set
if (dataType && dataType.set) {
newVal = dataType.set(newVal).val;
}
self._cache[name] = newVal;
self.trigger('change:' + name, self, self._cache[name]);

// check for default or get for the change event value
if (typeof newVal === 'undefined') {
newVal = result(def, 'default');
}
else if (dataType && dataType.get) {
newVal = dataType.get(newVal);
}
self.trigger('change:' + name, self, newVal);
}
};

def.deps.forEach(function (propString) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason we're getting rid of flushCache?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm guessing cos it's not used. Doesn't appear to be being used by any other modules as per a github search. What do you think about removing it @HenrikJoreteg? Can you remember if it's useful/needed?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know at one point or another i wanted to be able to do this forcibly. But can't remember why. If it's not being used and everything passes, then we're probably good.

self._keyTree.add(propString, update);
});

// init to set any listeners
if (def.init) update();
});

this.on('all', function (eventName) {
Expand All @@ -437,16 +454,17 @@ assign(Base.prototype, Events, {
}, this);
},

_getDerivedProperty: function (name, flushCache) {
_getDerivedProperty: function (name) {
var def = this._derived[name];
// is this a derived property that is cached
if (this._derived[name].cache) {
//set if this is the first time, or flushCache is set
if (flushCache || !this._cache.hasOwnProperty(name)) {
this._cache[name] = this._derived[name].fn.apply(this);
if (def.cache) {
// set if this is the first time
if (!this._cache.hasOwnProperty(name)) {
this._cache[name] = def.fn.apply(this);
}
return this._cache[name];
} else {
return this._derived[name].fn.apply(this);
return def.fn.apply(this);
}
},

Expand Down Expand Up @@ -582,9 +600,14 @@ function createDerivedProperty(modelProto, name, definition) {
var def = modelProto._derived[name] = {
fn: isFunction(definition) ? definition : definition.fn,
cache: (definition.cache !== false),
type: definition.type,
init: definition.init,
default: definition.default,
depList: definition.deps || []
};

if (def.type && isUndefined(def.default))
def.default = modelProto._getDefaultForType(def.type);
// add to our shared dependency list
forEach(def.depList, function (dep) {
modelProto._deps[dep] = union(modelProto._deps[dep] || [], [name]);
Expand All @@ -593,7 +616,15 @@ function createDerivedProperty(modelProto, name, definition) {
// defined a top-level getter for derived names
Object.defineProperty(modelProto, name, {
get: function () {
return this._getDerivedProperty(name);
var value = this._getDerivedProperty(name);
var typeDef = this._dataTypes[def.type];
if (typeof value !== 'undefined') {
if (typeDef && typeDef.get) {
value = typeDef.get(value);
}
return value;
}
return result(def, 'default');
},
set: function () {
throw new TypeError('"' + name + '" is a derived property, it can\'t be set directly.');
Expand Down
43 changes: 43 additions & 0 deletions test/basics.js
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,49 @@ test('uncached derived properties always fire events on dependency change', func
person.name = 'different';
});

test('derived properties with type: `state` will be evented', function (t) {
var ran = 0;
var coolCheck;
var AwesomePerson = Person.extend({
props: {
awesomeness: 'number',
coolness: 'number'
}
});
var friend = new AwesomePerson({name: 'mat', awesomeness: 1, coolness: 1});
var NewPerson = Person.extend({
props: {
friendName: 'string'
},
derived: {
friend: {
deps: ['friendName'],
type: 'state',
init: true,
fn: function () {
ran++;
return this.friendName === 'mat' ? friend : null;
}
}
}
});
var person = new NewPerson({name: 'henrik', friendName: 'mat'});
person.on('change:friend.coolness', function (model, value) {
t.equal(value, 3, "listens to changes on derived property attribute at init");
coolCheck = true;
});
person.on('change:friend.awesomeness', function (model, value) {
t.equal(value, 7, "Fires update for derived property attribute");
t.end();
});
t.equal(ran, 1);
friend.coolness = 3;
t.ok(coolCheck, 'coolCheck');
t.equal(person.friend.awesomeness, 1);
person.friend.awesomeness = 7;
t.equal(ran, 1);
});

test('everything should work with a property called `type`. Issue #6.', function (t) {
var Model = State.extend({
props: {
Expand Down
131 changes: 131 additions & 0 deletions test/full.js
Original file line number Diff line number Diff line change
Expand Up @@ -546,6 +546,137 @@ test('derived properties triggered with multiple instances', function (t) {
t.end();
});

test('derived properties with `type` will use dataType functions', function (t) {
var friendRan = 0;
var crazyRan = 0;
var compareCheck, coolCheck, changeCheck, thingCheck, weirdCheck;

var fooFriends = {};

var Foo = State.extend({
extraProperties: 'allow',
props: {
name: ['string', true],
friendName: 'string',
crazy: 'boolean',
cool: 'boolean'
},
derived: {
friend: {
deps: ['friendName'],
type: 'state',
init: true,
default: 'no friend',
fn: function () {
friendRan++;
return fooFriends[this.friendName];
}
},
crazyFriend: {
deps: ['friend', 'friend.name', 'friend.crazy'],
type: 'crazyType',
fn: function () {
crazyRan++;
if (this.friend.name) {
return this.friend.name + ' is ' + (this.friend.crazy ? '' : 'not ');
}
}
}
},
dataTypes: {
crazyType: {
compare: function (oldVal, newVal, name) {
compareCheck = true;
return false;
},
set: function (newVal) {
return {
val: newVal,
type: 'crazyType'
};
},
get: function (val) {
return val + 'crazy!';
},
default: function () {
return 'crazy!';
}
}
}
});

fooFriends.mat = new Foo({
name: 'mat',
cool: false,
weird: false
});
t.equal(friendRan, 1);
t.equal(fooFriends.mat.friend, 'no friend', 'apply derived default when undefined');
t.equal(fooFriends.mat.crazyFriend, 'crazy!', 'apply dataType default when undefined');
t.equal(crazyRan, 1);
fooFriends.cat = new Foo({
name: 'cat',
cool: false,
weird: false
});
t.equal(friendRan, 3);

var foo = new Foo({
name: 'abe',
friendName: 'mat'
});
t.equal(friendRan, 4);

foo.on('change:friend.weird', function (model, value) {
t.ok(value, "Fires update for derived property attribute");
weirdCheck = true;
});
foo.on('change:friend.cool', function (model, value) {
t.ok(value, "Fires compare for derived state on init");
coolCheck = true;
});

foo.friend.cool = true;
foo.friend.weird = true;
t.ok(foo.friend.weird);
t.equal(friendRan, 4);

var bar = new Foo({
name: 'bob',
friendName: 'nobody'
});

bar.on('change:friend', function (model, value) {
t.ok(value, "Fires on change");
changeCheck = true;
});
bar.on('change:friend:thing', function (model, value) {
t.equal(value, 'thing', "Fires on change of derived child ad hoc properties");
thingCheck = true;
});

bar.friendName = 'cat';
t.equal(friendRan, 6);
t.equal(bar.friend.name, 'cat');
bar.friendName = 'mat';
t.equal(bar.friend.name, 'mat');
t.equal(friendRan, 7);

bar.friend.thing = 'thing';

t.equal(crazyRan, 7);
t.equal(bar.crazyFriend, 'mat is not crazy!', 'derived properties with dataType should use dataType.get');
bar.friend.crazy = true;
t.equal(bar.crazyFriend, 'mat is crazy!', 'derived result for dataType should change');
t.equal(crazyRan, 9);

t.ok(compareCheck, 'passed compareCheck');
t.ok(changeCheck, 'passed changeCheck');
t.ok(coolCheck, 'passed coolCheck');
t.ok(weirdCheck, 'passed weirdCheck');
t.end();
});

test('Calling `previous` during change of derived cached property should work', function (t) {
var foo = new Foo({firstName: 'Henrik', lastName: 'Joreteg'});
var ran = false;
Expand Down