Skip to content

Commit

Permalink
Merge pull request #11812 from martndemus/stable-get-deprecations
Browse files Browse the repository at this point in the history
[BUGFIX release] Add deprecation messages when using get/set in a deprecated way
  • Loading branch information
rwjblue committed Jul 19, 2015
2 parents cc99723 + aa7e1c7 commit 6a6aa8d
Show file tree
Hide file tree
Showing 8 changed files with 43 additions and 3 deletions.
8 changes: 7 additions & 1 deletion packages/ember-metal/lib/property_get.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,12 +51,15 @@ export let UNHANDLED_GET = symbol("UNHANDLED_GET");
@public
*/
export function get(obj, keyName) {
Ember.deprecate(`Get must be called with two arguments; an object and a property key`,
arguments.length === 2);
// Helpers that operate with 'this' within an #each
if (keyName === '') {
return obj;
}

if (!keyName && 'string' === typeof obj) {
Ember.deprecate('Calling Ember.get with only a property key has been deprecated, please also specify a target object.');
keyName = obj;
obj = Ember.lookup;
}
Expand All @@ -65,6 +68,7 @@ export function get(obj, keyName) {
Ember.assert(`Cannot call get with '${keyName}' on an undefined object.`, obj !== undefined);

if (isNone(obj)) {
Ember.deprecate('Calling Ember.get without a target object has been deprecated, please specify a target object.');
return _getPath(obj, keyName);
}

Expand Down Expand Up @@ -159,6 +163,8 @@ export function _getPath(root, path) {
// detect complicated paths and normalize them
hasThis = pathHasThis(path);

Ember.deprecate(`Ember.get with 'this' in the path has been deprecated. Please use the same path without 'this'.`, !hasThis);

if (!root || hasThis) {
tuple = normalizeTuple(root, path);
root = tuple[0];
Expand All @@ -169,7 +175,7 @@ export function _getPath(root, path) {
parts = path.split(".");
len = parts.length;
for (idx = 0; root != null && idx < len; idx++) {
root = get(root, parts[idx], true);
root = get(root, parts[idx]);
if (root && root.isDestroyed) { return undefined; }
}
return root;
Expand Down
7 changes: 7 additions & 0 deletions packages/ember-metal/lib/property_set.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,17 @@ export let UNHANDLED_SET = symbol("UNHANDLED_SET");
export function set(obj, keyName, value, tolerant) {
if (typeof obj === 'string') {
Ember.assert(`Path '${obj}' must be global if no obj is given.`, isGlobalPath(obj));
Ember.deprecate('Calling Ember.set with only a property key and a value has been deprecated, please also specify a target object.');
value = keyName;
keyName = obj;
obj = Ember.lookup;
}

Ember.deprecate(`Set must be called with tree or four arguments; an object, a property key, a value and tolerant true/false`,
arguments.length === 3 || arguments.length === 4);
Ember.assert(`Cannot call set with '${keyName}' key.`, !!keyName);


if (obj === Ember.lookup) {
return setPath(obj, keyName, value, tolerant);
}
Expand All @@ -62,6 +66,7 @@ export function set(obj, keyName, value, tolerant) {

var isUnknown, currentValue;
if ((!obj || desc === undefined) && isPath(keyName)) {
Ember.deprecate('Calling Ember.set without a target object has been deprecated, please specify a target object.', !!obj);
return setPath(obj, keyName, value, tolerant);
}

Expand Down Expand Up @@ -139,6 +144,8 @@ function setPath(root, path, value, tolerant) {
// get the root
if (path !== 'this') {
root = getPath(root, path);
} else {
Ember.deprecate(`Ember.set with 'this' in the path has been deprecated. Please use the same path without 'this'.`);
}

if (!keyName || keyName.length === 0) {
Expand Down
10 changes: 10 additions & 0 deletions packages/ember-metal/tests/accessors/get_path_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,17 @@ QUnit.test('[obj, foothis.bar] -> obj.foothis.bar', function() {
});

QUnit.test('[obj, this.foo] -> obj.foo', function() {
expectDeprecation(/Ember.get with 'this' in the path has been deprecated. Please use the same path without 'this'./);
deepEqual(get(obj, 'this.foo'), obj.foo);
});

QUnit.test('[obj, this.foo.bar] -> obj.foo.bar', function() {
expectDeprecation(/Ember.get with 'this' in the path has been deprecated. Please use the same path without 'this'./);
deepEqual(get(obj, 'this.foo.bar'), obj.foo.bar);
});

QUnit.test('[obj, this.Foo.bar] -> (undefined)', function() {
expectDeprecation(/Ember.get with 'this' in the path has been deprecated. Please use the same path without 'this'./);
equal(get(obj, 'this.Foo.bar'), undefined);
});

Expand Down Expand Up @@ -109,18 +112,22 @@ QUnit.test('[obj, Foo.bar] -> (undefined)', function() {
//

QUnit.test('[null, Foo] -> Foo', function() {
expectDeprecation(/Calling Ember.get without a target object has been deprecated, please specify a target object./);
equal(get(null, 'Foo'), Foo);
});

QUnit.test('[null, Foo.bar] -> Foo.bar', function() {
expectDeprecation(/Calling Ember.get without a target object has been deprecated, please specify a target object./);
deepEqual(get(null, 'Foo.bar'), Foo.bar);
});

QUnit.test('[null, $foo] -> $foo', function() {
expectDeprecation(/Calling Ember.get without a target object has been deprecated, please specify a target object./);
equal(get(null, '$foo'), window.$foo);
});

QUnit.test('[null, aProp] -> null', function() {
expectDeprecation(/Calling Ember.get without a target object has been deprecated, please specify a target object./);
equal(get(null, 'aProp'), null);
});

Expand All @@ -129,13 +136,16 @@ QUnit.test('[null, aProp] -> null', function() {
//

QUnit.test('[Foo] -> Foo', function() {
expectDeprecation(/Calling Ember.get with only a property key has been deprecated, please also specify a target object/);
deepEqual(get('Foo'), Foo);
});

QUnit.test('[aProp] -> aProp', function() {
expectDeprecation(/Calling Ember.get with only a property key has been deprecated, please also specify a target object/);
deepEqual(get('aProp'), window.aProp);
});

QUnit.test('[Foo.bar] -> Foo.bar', function() {
expectDeprecation(/Calling Ember.get with only a property key has been deprecated, please also specify a target object/);
deepEqual(get('Foo.bar'), Foo.bar);
});
2 changes: 2 additions & 0 deletions packages/ember-metal/tests/accessors/get_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,12 @@ QUnit.test('warn on attempts to get a property path of undefined', function() {
});

QUnit.test('returns null when fetching a complex local path on a null context', function() {
expectDeprecation(/Calling Ember.get without a target object has been deprecated, please specify a target object./);
equal(get(null, 'aProperty.on.aPath'), null);
});

QUnit.test('returns null when fetching a simple local path on a null context', function() {
expectDeprecation(/Calling Ember.get without a target object has been deprecated, please specify a target object./);
equal(get(null, 'aProperty'), null);
});

Expand Down
9 changes: 9 additions & 0 deletions packages/ember-metal/tests/accessors/set_path_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,11 +63,13 @@ QUnit.test('[obj, foo.bar] -> obj.foo.bar', function() {
});

QUnit.test('[obj, this.foo] -> obj.foo', function() {
expectDeprecation(/Ember.set with 'this' in the path has been deprecated. Please use the same path without 'this'./);
set(obj, 'this.foo', "BAM");
equal(get(obj, 'foo'), "BAM");
});

QUnit.test('[obj, this.foo.bar] -> obj.foo.bar', function() {
expectDeprecation(/Ember.get with 'this' in the path has been deprecated. Please use the same path without 'this'./);
set(obj, 'this.foo.bar', "BAM");
equal(get(obj, 'foo.bar'), "BAM");
});
Expand All @@ -77,10 +79,17 @@ QUnit.test('[obj, this.foo.bar] -> obj.foo.bar', function() {
//

QUnit.test('[null, Foo.bar] -> Foo.bar', function() {
expectDeprecation(/Calling Ember.set without a target object has been deprecated, please specify a target object./);
set(null, 'Foo.bar', "BAM");
equal(get(Ember.lookup.Foo, 'bar'), "BAM");
});

QUnit.test('[Foo.bar] -> Foo.bar', function() {
expectDeprecation(/Calling Ember.set with only a property key and a value has been deprecated, please also specify a target object./);
set('Foo.bar', "BAM");
equal(get(Ember.lookup.Foo, 'bar'), "BAM");
});

// ..........................................................
// DEPRECATED
//
Expand Down
1 change: 1 addition & 0 deletions packages/ember-metal/tests/computed_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -581,6 +581,7 @@ testBoth('depending on simple chain', function(get, set) {
});

testBoth('depending on Global chain', function(get, set) {
expectDeprecation(/Calling Ember.get with only a property key has been deprecated, please also specify a target object/);

// assign computed property
defineProperty(obj, 'prop', computed(function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,7 @@ QUnit.test("raise if the provided object is undefined", function() {
});

QUnit.test("should work when object is Ember (used in Ember.get)", function() {
expectDeprecation(/Calling Ember.get with only a property key has been deprecated, please also specify a target object/);
equal(get('Ember.RunLoop'), Ember.RunLoop, 'Ember.get');
equal(get(Ember, 'RunLoop'), Ember.RunLoop, 'Ember.get(Ember, RunLoop)');
});
Expand All @@ -173,6 +174,7 @@ QUnit.module("Ember.get() with paths", {
});

QUnit.test('should return a property at a given path relative to the lookup', function() {
expectDeprecation(/Calling Ember.get with only a property key has been deprecated, please also specify a target object/);
lookup.Foo = ObservableObject.extend({
Bar: ObservableObject.extend({
Baz: computed(function() { return 'blargh'; }).volatile()
Expand All @@ -193,6 +195,7 @@ QUnit.test("should return a property at a given path relative to the passed obje
});

QUnit.test("should return a property at a given path relative to the lookup - JavaScript hash", function() {
expectDeprecation(/Calling Ember.get with only a property key has been deprecated, please also specify a target object/);
lookup.Foo = {
Bar: {
Baz: "blargh"
Expand Down
6 changes: 4 additions & 2 deletions packages/ember-runtime/tests/mixins/deferred_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,9 @@ if (!EmberDev.runningProdBuild) {
var obj = {};

Ember.deprecate = function(message) {
deprecationMade = message;
if (message === 'Usage of Ember.DeferredMixin or Ember.Deferred is deprecated.') {
deprecationMade = true;
}
};

deferred = EmberObject.extend(Deferred).create();
Expand All @@ -360,7 +362,7 @@ if (!EmberDev.runningProdBuild) {
deferred.then(function(value) {
equal(value, obj, "successfully resolved to given value");
});
equal(deprecationMade, 'Usage of Ember.DeferredMixin or Ember.Deferred is deprecated.');
equal(deprecationMade, true, 'the deprecation was made');

run(deferred, 'resolve', obj);
});
Expand Down

0 comments on commit 6a6aa8d

Please sign in to comment.