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.

Sign up for GitHub

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

[BUGFIX beta] Remove chaining in Observable.set #11213

Merged
merged 3 commits into from
Jun 15, 2015
Merged

[BUGFIX beta] Remove chaining in Observable.set #11213

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e633c52
Remove chaining in set and see what happens
cibernox May 18, 2015
796aff2
Return the setter values instead of this
cibernox May 19, 2015
dfc31df
Apply commit 48aad1d with a patch
cibernox May 19, 2015
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
6 changes: 3 additions & 3 deletions packages/ember-metal/lib/set_properties.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
24 changes: 12 additions & 12 deletions packages/ember-runtime/tests/legacy_1x/mixins/observable/observable_test.js
View file
Open in desktop
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