Skip to content

Commit

Permalink
Merge pull request #11213 from cibernox/remove-set-chaining
Browse files Browse the repository at this point in the history
[BUGFIX beta] Remove chaining in Observable.set
  • Loading branch information
mixonic committed Jun 15, 2015
2 parents 9c97c78 + dfc31df commit 8f53f8e
Show file tree
Hide file tree
Showing 4 changed files with 25 additions and 31 deletions.
6 changes: 3 additions & 3 deletions packages/ember-metal/lib/set_properties.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,11 @@ import keys from "ember-metal/keys";
@method setProperties
@param obj
@param {Object} properties
@return obj
@return properties
@public
*/
export default function setProperties(obj, properties) {
if (!properties || typeof properties !== "object") { return obj; }
if (!properties || typeof properties !== "object") { return properties; }
changeProperties(function() {
var props = keys(properties);
var propertyName;
Expand All @@ -35,5 +35,5 @@ export default function setProperties(obj, properties) {
set(obj, propertyName, properties[propertyName]);
}
});
return obj;
return properties;
}
14 changes: 6 additions & 8 deletions packages/ember-metal/tests/set_properties_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,21 +6,19 @@ QUnit.test("supports setting multiple attributes at once", function() {
deepEqual(setProperties(null, null), null, 'noop for null properties and null object');
deepEqual(setProperties(undefined, undefined), undefined, 'noop for undefined properties and undefined object');

deepEqual(setProperties({}), {}, 'noop for no properties');
deepEqual(setProperties({}, undefined), {}, 'noop for undefined');
deepEqual(setProperties({}, null), {}, 'noop for null');
deepEqual(setProperties({}, NaN), {}, 'noop for NaN');
deepEqual(setProperties({}), undefined, 'noop for no properties');
deepEqual(setProperties({}, undefined), undefined, 'noop for undefined');
deepEqual(setProperties({}, null), null, 'noop for null');
deepEqual(setProperties({}, NaN), NaN, 'noop for NaN');
deepEqual(setProperties({}, {}), {}, 'meh');

deepEqual(setProperties({}, { foo: 1 }), { foo: 1 }, 'Set a single property');

deepEqual(setProperties({}, { foo: 1, bar: 1 }), { foo: 1, bar: 1 }, 'Set multiple properties');

deepEqual(setProperties({ foo: 2, baz: 2 }, { foo: 1 }), { foo: 1, baz: 2 }, 'Set one of multiple properties');
deepEqual(setProperties({ foo: 2, baz: 2 }, { foo: 1 }), { foo: 1 }, 'Set one of multiple properties');

deepEqual(setProperties({ foo: 2, baz: 2 }, { bar: 2 }), {
bar: 2,
foo: 2,
baz: 2
bar: 2
}, 'Set an additional, previously unset property');
});
12 changes: 4 additions & 8 deletions packages/ember-runtime/lib/mixins/observable.js
Original file line number Diff line number Diff line change
Expand Up @@ -214,8 +214,7 @@ export default Mixin.create({
@public
*/
set(keyName, value) {
set(this, keyName, value);
return this;
return set(this, keyName, value);
},


Expand Down Expand Up @@ -461,8 +460,7 @@ export default Mixin.create({
incrementProperty(keyName, increment) {
if (isNone(increment)) { increment = 1; }
Ember.assert("Must pass a numeric value to incrementProperty", (!isNaN(parseFloat(increment)) && isFinite(increment)));
set(this, keyName, (parseFloat(get(this, keyName)) || 0) + increment);
return get(this, keyName);
return set(this, keyName, (parseFloat(get(this, keyName)) || 0) + increment);
},

/**
Expand All @@ -482,8 +480,7 @@ export default Mixin.create({
decrementProperty(keyName, decrement) {
if (isNone(decrement)) { decrement = 1; }
Ember.assert("Must pass a numeric value to decrementProperty", (!isNaN(parseFloat(decrement)) && isFinite(decrement)));
set(this, keyName, (get(this, keyName) || 0) - decrement);
return get(this, keyName);
return set(this, keyName, (get(this, keyName) || 0) - decrement);
},

/**
Expand All @@ -500,8 +497,7 @@ export default Mixin.create({
@public
*/
toggleProperty(keyName) {
set(this, keyName, !get(this, keyName));
return get(this, keyName);
return set(this, keyName, !get(this, keyName));
},

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -274,41 +274,41 @@ QUnit.module("object.set()", {

});

QUnit.test("should change normal properties and return this", function() {
QUnit.test("should change normal properties and return the value", function() {
var ret = object.set("normal", "changed");
equal(object.normal, "changed");
equal(ret, object);
equal(ret, "changed");
});

QUnit.test("should call computed properties passing value and return this", function() {
QUnit.test("should call computed properties passing value and return the value", function() {
var ret = object.set("computed", "changed");
equal(object._computed, "changed");
equal(ret, object);
equal(ret, "changed");
});

QUnit.test("should change normal properties when passing undefined", function() {
var ret = object.set('normal', undefined);
equal(object.normal, undefined);
equal(ret, object);
equal(ret, undefined);
});

QUnit.test("should replace the function for a non-computed property and return this", function() {
QUnit.test("should replace the function for a non-computed property and return the value", function() {
var ret = object.set("method", "changed");
equal(object._method, "method"); // make sure this was NOT run
ok(typeof object.method !== 'function');
equal(ret, object);
equal(ret, "changed");
});

QUnit.test("should replace prover when property value is null", function() {
var ret = object.set("nullProperty", "changed");
equal(object.nullProperty, "changed");
equal(ret, object);
equal(ret, "changed");
});

QUnit.test("should call unknownProperty with value when property is undefined", function() {
var ret = object.set("unknown", "changed");
equal(object._unknown, "changed");
equal(ret, object);
equal(ret, "changed");
});

// ..........................................................
Expand Down Expand Up @@ -453,11 +453,11 @@ QUnit.test("setting values should call function return value", function() {

forEach(keys, function(key) {

equal(object.set(key, values[0]), object, fmt('Try #1: object.set(%@, %@) should run function', [key, values[0]]));
equal(object.set(key, values[0]), values[0], fmt('Try #1: object.set(%@, %@) should run function', [key, values[0]]));

equal(object.set(key, values[1]), object, fmt('Try #2: object.set(%@, %@) should run function', [key, values[1]]));
equal(object.set(key, values[1]), values[1], fmt('Try #2: object.set(%@, %@) should run function', [key, values[1]]));

equal(object.set(key, values[1]), object, fmt('Try #3: object.set(%@, %@) should not run function since it is setting same value as before', [key, values[1]]));
equal(object.set(key, values[1]), values[1], fmt('Try #3: object.set(%@, %@) should not run function since it is setting same value as before', [key, values[1]]));

});

Expand Down

0 comments on commit 8f53f8e

Please sign in to comment.