Skip to content

Commit

Permalink
Merge pull request #163 from canjs/162-value-not-observable
Browse files Browse the repository at this point in the history
#162 .value functions should not be observable.
  • Loading branch information
justinbmeyer authored Mar 28, 2017
2 parents 97ceb28 + 9141f62 commit 043753e
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 15 deletions.
26 changes: 17 additions & 9 deletions can-define.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,7 @@ define.property = function(objPrototype, prop, definition, dataInitializers, com

// Determine a function that will provide the initial property value.
if ((definition.value !== undefined || definition.Value !== undefined)) {
getInitialValue = make.get.defaultValue(prop, definition, typeConvert, eventsSetter);
getInitialValue = Observation.ignore(make.get.defaultValue(prop, definition, typeConvert, eventsSetter));
}

// If property has a getter, create the compute that stores its data.
Expand Down Expand Up @@ -277,14 +277,19 @@ make = {
},
events: function(prop, getCurrent, setData, eventType) {
return function(newVal) {
var current = getCurrent.call(this);
if (newVal !== current) {
if (this.__inSetup) {
setData.call(this, newVal);

canEvent.dispatch.call(this, {
type: prop,
target: this
}, [newVal, current]);
}
else {
var current = getCurrent.call(this);
if (newVal !== current) {
setData.call(this, newVal);

canEvent.dispatch.call(this, {
type: prop,
target: this
}, [newVal, current]);
}
}
};
},
Expand Down Expand Up @@ -500,7 +505,10 @@ make = {
},
data: function(prop) {
return function() {
Observation.add(this, prop);
if (!this.__inSetup) {
Observation.add(this, prop);
}

return this._data[prop];
};
},
Expand Down
44 changes: 38 additions & 6 deletions map/map-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ var DefineMap = require("can-define/map/map");
var Observation = require("can-observation");
var canTypes = require("can-types");
var each = require("can-util/js/each/each");
var compute = require("can-compute");
var sealWorks = (function() {
try {
var o = {};
Expand Down Expand Up @@ -31,7 +32,7 @@ QUnit.test("creating an instance", function(){
QUnit.equal(oldVal, "foo");
});

map.prop ="BAR";
map.prop = "BAR";
});

QUnit.test("creating an instance with nested prop", function(){
Expand All @@ -43,7 +44,7 @@ QUnit.test("creating an instance with nested prop", function(){
QUnit.equal(oldVal, "Justin");
});

map.name.first ="David";
map.name.first = "David";
});


Expand Down Expand Up @@ -173,15 +174,15 @@ QUnit.test("get with dynamically added properties", function(){
var map = new DefineMap();
map.set("a",1);
map.set("b",2);
QUnit.deepEqual(map.get(), {a: 1, b:2});
QUnit.deepEqual(map.get(), {a: 1, b: 2});
});


QUnit.test("set multiple props", function(){
var map = new DefineMap();
map.set({a: 0, b: 2});

QUnit.deepEqual(map.get(), {a: 0, b:2});
QUnit.deepEqual(map.get(), {a: 0, b: 2});

map.set({a: 2}, true);

Expand All @@ -208,7 +209,7 @@ QUnit.test("serialize responds to added props", function(){

QUnit.test("initialize an undefined property", function(){
var MyMap = DefineMap.extend({seal: false},{});
var instance = new MyMap({foo:"bar"});
var instance = new MyMap({foo: "bar"});

equal(instance.foo, "bar");
});
Expand Down Expand Up @@ -282,7 +283,7 @@ QUnit.test("serialize: function works (#38)", function(){
var MyMap2 = DefineMap.extend({
"*": {
serialize: function(value){
return ""+value;
return "" + value;
}
}
});
Expand Down Expand Up @@ -534,3 +535,34 @@ QUnit.test(".extend errors when re-defining a property (#117)", function(){
});
QUnit.ok(true, "extended without errors");
});

QUnit.test(".value functions should not be observable", function(){
var outer = new DefineMap({
bam: "baz"
});

var ItemsVM = DefineMap.extend({
item: {
value: function(){
(function(){})(this.zed, outer.bam);
return new DefineMap({ foo: "bar" });
}
},
zed: "string"
});

var items = new ItemsVM();

var count = 0;
var itemsList = compute(function(){
count++;
return items.item;
});

itemsList.on('change', function() {});

items.item.foo = "changed";
items.zed = "changed";

equal(count, 1);
});

0 comments on commit 043753e

Please sign in to comment.